From 64eb222bcfd5ce8b6cd7371840450d51e98e7e92 Mon Sep 17 00:00:00 2001 From: Eugeny Kolpakov Date: Wed, 18 Oct 2017 11:53:45 +0800 Subject: [PATCH 1/5] CompletionXBlockMixin + tests --- requirements.txt | 2 + xblock/VERSION.txt | 2 +- xblock/completable.py | 55 ++++++++++++++ xblock/test/test_completable.py | 122 ++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 1 deletion(-) create mode 100644 xblock/completable.py create mode 100644 xblock/test/test_completable.py diff --git a/requirements.txt b/requirements.txt index 87074b664..4cd7f926b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -20,6 +20,8 @@ caniusepython3 diff-cover >= 0.2.1 ddt==0.8.0 tox +hypothesis>=3.33.0,<4.0 + # For docs -r doc/requirements.txt diff --git a/xblock/VERSION.txt b/xblock/VERSION.txt index 7dea76edb..9084fa2f7 100644 --- a/xblock/VERSION.txt +++ b/xblock/VERSION.txt @@ -1 +1 @@ -1.0.1 +1.1.0 diff --git a/xblock/completable.py b/xblock/completable.py new file mode 100644 index 000000000..49aecd0b9 --- /dev/null +++ b/xblock/completable.py @@ -0,0 +1,55 @@ +""" +This module defines CompletableXBlockMixin and completion mode enumeration. +""" +from __future__ import absolute_import, unicode_literals + +class XBlockCompletionMode(object): + """ + Enumeration for completion modes. + """ + COMPLETABLE = "completable" + AGGREGATOR = "aggregator" + EXCLUDED = "excluded" + + +class CompletableXBlockMixin(object): + """ + This mixin sets attributes and provides helper method to integrate XBlock with Completion API. + """ + + has_custom_completion = True + completion_method = XBlockCompletionMode.COMPLETABLE + + # Completion_percent is somewhat misleading name - as it is actually a ratio. But this is how edx-platform calls it, + # so use the same name here for consistency + # https://github.com/edx/XBlock/pull/368#discussion_r146560619 + def emit_completion(self, completion_percent): + """ + Emits completion event through Completion API. + + Unlike grading API, calling this method allows completion to go down - i.e. emitting a value of 0.0 on + a previously completed block indicates that it is no longer considered complete. + + Arguments: + completion_percent (float): Completion in range [0.0; 1.0] (inclusive), where 0.0 means the block + is not completed, 1.0 means the block is fully completed. + + Returns: + None + """ + if not self.has_custom_completion or self.completion_method != 'completable': + raise AttributeError( + "Using `emit_completion` requires `has_custom_completion == True` (was {}) " + "and `completion_method == 'completable'` (was {})".format( + self.has_custom_completion, self.completion_method + ) + ) + + if completion_percent is None or not 0.0 <= completion_percent <= 1.0: + raise ValueError("Completion ratio must be in [0.0; 1.0] interval, {} given".format(completion_percent)) + + self.runtime.publish( + self, + 'completion', + {'completion': completion_percent}, + ) diff --git a/xblock/test/test_completable.py b/xblock/test/test_completable.py new file mode 100644 index 000000000..d2537c981 --- /dev/null +++ b/xblock/test/test_completable.py @@ -0,0 +1,122 @@ +""" +Tests of the CompletableXBlockMixin. +""" +from __future__ import absolute_import, unicode_literals + +import math +from unittest import TestCase + +import mock +from hypothesis import given, example +import hypothesis.strategies as strategies + +from xblock.core import XBlock +from xblock.field_data import DictFieldData +from xblock.fields import ScopeIds +from xblock.runtime import Runtime +from xblock.completable import CompletableXBlockMixin, XBlockCompletionMode + + +class CompletableXBlockMixinTest(TestCase): + """ + Tests for CompletableXBlockMixin. + """ + class TestBuddyXBlock(XBlock, CompletableXBlockMixin): + """ + Simple XBlock extending CompletableXBlockMixin. + """ + + class TestIllegalCustomCompletionAttrXBlock(XBlock, CompletableXBlockMixin): + """ + XBlock extending CompletableXBlockMixin using illegal `has_custom_completion` attribute. + """ + has_custom_completion = False + + class TestIllegalCompletionMethodAttrXBlock(XBlock, CompletableXBlockMixin): + """ + XBlock extending CompletableXBlockMixin using illegal `completion_method` attribute. + """ + completion_method = "something_else" + + def _make_block(self, runtime=None, block_type=None): + """ + Creates a test block. + """ + block_type = block_type if block_type else self.TestBuddyXBlock + runtime = runtime if runtime else mock.Mock(spec=Runtime) + scope_ids = ScopeIds("user_id", "test_buddy", "def_id", "usage_id") + return block_type(runtime=runtime, field_data=DictFieldData({}), scope_ids=scope_ids) + + def test_has_custom_completion_property(self): + """ + Test `has_custom_completion` property is set by mixin. + """ + block = self._make_block() + self.assertTrue(block.has_custom_completion) + self.assertTrue(getattr(block, 'has_custom_completion', False)) + + def test_completion_method_property(self): + """ + Test `completion_method` property is set by mixin. + """ + block = self._make_block() + self.assertEqual(block.completion_method, XBlockCompletionMode.COMPLETABLE) + self.assertEqual(getattr(block, 'completion_method', ""), XBlockCompletionMode.COMPLETABLE) + + @given(strategies.floats()) + def test_emit_completion_illegal_custom_completion(self, any_completion): + """ + Test `emit_completion` raises exception when called on a XBlock with illegal `has_custom_completion` value. + """ + runtime_mock = mock.Mock(spec=Runtime) + illegal_custom_completion_block = self._make_block(runtime_mock, self.TestIllegalCustomCompletionAttrXBlock) + with self.assertRaises(AttributeError): + illegal_custom_completion_block.emit_completion(any_completion) + + @given(strategies.floats()) + def test_emit_completion_completion_method(self, any_completion): + """ + Test `emit_completion` raises exception when called on a XBlock with illegal `completion_method` value. + """ + runtime_mock = mock.Mock(spec=Runtime) + illegal_completion_method_block = self._make_block(runtime_mock, self.TestIllegalCompletionMethodAttrXBlock) + with self.assertRaises(AttributeError): + illegal_completion_method_block.emit_completion(any_completion) + + @given(strategies.floats(min_value=0.0, max_value=1.0)) + @example(1.0) + @example(0.0) + def test_emit_completion_emits_event(self, valid_completion_percent): + """ + Test `emit_completion` emits completion events when passed a valid argument. + + Given a valid completion percent + When emit_completion is called + Then runtime.publish is called with expected arguments + """ + runtime_mock = mock.Mock(spec=Runtime) + block = self._make_block(runtime_mock) + block.emit_completion(valid_completion_percent) + + runtime_mock.publish.assert_called_once_with(block, "completion", {"completion": valid_completion_percent}) + + @given(strategies.floats().filter(lambda x: math.isnan(x) or x < 0.0 or x > 1.0)) + @example(None) + @example(float('+inf')) + @example(float('-inf')) + def test_emit_completion_raises_assertion_error_if_invalid(self, invalid_completion_percent): + """ + Test `emit_completion` raises exception when passed an invalid argument. + + Given an invalid completion percent + * Less than 0.0 + * Greater than 1.0 + * Positive or negative infinity + * NaN + When emit_completion is called + Then value error is thrown + """ + runtime_mock = mock.Mock(spec=Runtime) + block = self._make_block(runtime_mock) + with self.assertRaises(ValueError): + self.assertRaises(block.emit_completion(invalid_completion_percent)) From d3dcf04548e7b404f331fd438e0af2965f957bdf Mon Sep 17 00:00:00 2001 From: Eugeny Kolpakov Date: Wed, 25 Oct 2017 10:10:09 +0800 Subject: [PATCH 2/5] Fixed broken test --- xblock/test/test_completable.py | 3 +-- xblock/test/test_core.py | 7 +++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/xblock/test/test_completable.py b/xblock/test/test_completable.py index d2537c981..9609519b5 100644 --- a/xblock/test/test_completable.py +++ b/xblock/test/test_completable.py @@ -11,7 +11,6 @@ import hypothesis.strategies as strategies from xblock.core import XBlock -from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from xblock.runtime import Runtime from xblock.completable import CompletableXBlockMixin, XBlockCompletionMode @@ -45,7 +44,7 @@ def _make_block(self, runtime=None, block_type=None): block_type = block_type if block_type else self.TestBuddyXBlock runtime = runtime if runtime else mock.Mock(spec=Runtime) scope_ids = ScopeIds("user_id", "test_buddy", "def_id", "usage_id") - return block_type(runtime=runtime, field_data=DictFieldData({}), scope_ids=scope_ids) + return block_type(runtime=runtime, scope_ids=scope_ids) def test_has_custom_completion_property(self): """ diff --git a/xblock/test/test_core.py b/xblock/test/test_core.py index 22a289527..52407d527 100644 --- a/xblock/test/test_core.py +++ b/xblock/test/test_core.py @@ -1039,6 +1039,13 @@ class TestBlock(XBlock): """An empty XBlock for testing""" pass + # FIXME: This test is fragile - fails in python27 if a warning it is expecting is emitted before it is executed + # warnings module in py27 seems to be intelligent enough to deduplicate warnings raised at the same place. So if + # this test is executed _after_ any test that already raised FieldDataDeprecationWarning in XBlock constructor + # (currently raised in `ScopedStorageMixin`) it will not catch the warning and fail. + # Example: XBlock(runtime, field_data=Whatever(), scopeIds=ScopeIds()) + # Note that test_mixins have that pattern in abundance - the only reason it didn't fail earlier is that + # test_mixins are alphabetically preceding def test_field_data_paramater(self): field_data = Mock(spec=FieldData) with self.assertWarns(FieldDataDeprecationWarning): From b2b5d0f4ba2e3acc311be890e5d7ddbb790a218d Mon Sep 17 00:00:00 2001 From: Eugeny Kolpakov Date: Wed, 25 Oct 2017 10:10:40 +0800 Subject: [PATCH 3/5] Added (or rather restored) ability to run single test via tox --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 5c0110e6f..fc499adf4 100644 --- a/tox.ini +++ b/tox.ini @@ -14,7 +14,7 @@ deps = django111: Django>=1.11,<2.0 -rrequirements.txt commands = - coverage run -m nose + coverage run -m nose {posargs} py27-django18: make quality whitelist_externals = make From 74ab8554f3d6913418fcc8d65d801f0bf80fcb89 Mon Sep 17 00:00:00 2001 From: Eugeny Kolpakov Date: Thu, 26 Oct 2017 09:46:53 +0800 Subject: [PATCH 4/5] Fixed one missing ratio ues and made comment about ratio vs. percent more informative --- xblock/completable.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xblock/completable.py b/xblock/completable.py index 49aecd0b9..7e38bf337 100644 --- a/xblock/completable.py +++ b/xblock/completable.py @@ -20,9 +20,8 @@ class CompletableXBlockMixin(object): has_custom_completion = True completion_method = XBlockCompletionMode.COMPLETABLE - # Completion_percent is somewhat misleading name - as it is actually a ratio. But this is how edx-platform calls it, - # so use the same name here for consistency - # https://github.com/edx/XBlock/pull/368#discussion_r146560619 + # To read more on the debate about using the terms percent vs ratio, see: + # https://openedx.atlassian.net/wiki/spaces/OpenDev/pages/245465398/Naming+with+Percent+or+Ratio def emit_completion(self, completion_percent): """ Emits completion event through Completion API. @@ -46,7 +45,7 @@ def emit_completion(self, completion_percent): ) if completion_percent is None or not 0.0 <= completion_percent <= 1.0: - raise ValueError("Completion ratio must be in [0.0; 1.0] interval, {} given".format(completion_percent)) + raise ValueError("Completion percent must be in [0.0; 1.0] interval, {} given".format(completion_percent)) self.runtime.publish( self, From 734b6805e09bc9b2b11a26bb6f012895afda5742 Mon Sep 17 00:00:00 2001 From: Eugeny Kolpakov Date: Sun, 29 Oct 2017 16:35:18 +0800 Subject: [PATCH 5/5] Added link to discussion in the commetn --- xblock/test/test_core.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xblock/test/test_core.py b/xblock/test/test_core.py index 52407d527..4bf57b706 100644 --- a/xblock/test/test_core.py +++ b/xblock/test/test_core.py @@ -1046,6 +1046,7 @@ class TestBlock(XBlock): # Example: XBlock(runtime, field_data=Whatever(), scopeIds=ScopeIds()) # Note that test_mixins have that pattern in abundance - the only reason it didn't fail earlier is that # test_mixins are alphabetically preceding + # Related: https://github.com/edx/XBlock/pull/368#discussion_r146740102 def test_field_data_paramater(self): field_data = Mock(spec=FieldData) with self.assertWarns(FieldDataDeprecationWarning):