Merge pull request #25538 from regisb/regisb/improve-waffle-tests

[BD-21] Improve waffle_utils tests to not rely on internal API

openedx/core/djangoapps/waffle_utils/__init__.py

@@ -189,7 +189,7 @@ class CourseWaffleFlag(BaseWaffleFlag):
         is_enabled_for_course = self._get_course_override_value(course_key)
         if is_enabled_for_course is not None:
             # pylint: disable=protected-access
-            self.NAMESPACE_CLASS._monitor_value(
+            self.waffle_namespace._monitor_value(
                 self.namespaced_flag_name, is_enabled_for_course
             )
             return is_enabled_for_course

openedx/core/djangoapps/waffle_utils/tests/test_init.py

@@ -7,11 +7,8 @@ import ddt
 from django.test import TestCase
 from django.test.client import RequestFactory
 from django.test.utils import override_settings
-# TODO: we really shouldn't import from edx_toggles' internal API, but that's currently the only way to mock the
-# monitoring functions.
-import edx_toggles.toggles.internal.waffle
 from edx_django_utils.cache import RequestCache
-from mock import call, patch
+from mock import patch
 from opaque_keys.edx.keys import CourseKey
 from waffle.testutils import override_flag
 
@@ -41,21 +38,20 @@ class TestCourseWaffleFlag(TestCase):
     TEST_COURSE_FLAG = CourseWaffleFlag(TEST_NAMESPACE, FLAG_NAME, __name__)
 
     def setUp(self):
-        super(TestCourseWaffleFlag, self).setUp()
+        super().setUp()
         request = RequestFactory().request()
         self.addCleanup(crum.set_current_request, None)
         crum.set_current_request(request)
         RequestCache.clear_all_namespaces()
 
     @override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME])
-    @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute')
     @ddt.data(
         {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True},
         {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.off, 'waffle_enabled': True, 'result': False},
         {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': True, 'result': True},
         {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': False, 'result': False},
     )
-    def test_course_waffle_flag(self, data, mock_set_custom_attribute):
+    def test_course_waffle_flag(self, data):
         """
         Tests various combinations of a flag being set in waffle and overridden
         for a course.
@@ -72,9 +68,6 @@ class TestCourseWaffleFlag(TestCase):
                     self.TEST_COURSE_KEY
                 )
 
-        self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=str(data['result']))
-        mock_set_custom_attribute.reset_mock()
-
         # check flag for a second course
         if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset:
             # When course override wasn't set for the first course, the second course will get the same
@@ -87,12 +80,8 @@ class TestCourseWaffleFlag(TestCase):
             second_value = False
             self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value)
 
-        expected_flag_value = None if second_value == data['result'] else 'Both'
-        self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=expected_flag_value)
-
     @override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME])
-    @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute')
-    def test_undefined_waffle_flag(self, mock_set_custom_attribute):
+    def test_undefined_waffle_flag(self):
         """
         Test flag with undefined waffle flag.
         """
@@ -117,11 +106,6 @@ class TestCourseWaffleFlag(TestCase):
                 self.TEST_COURSE_KEY
             )
 
-        self._assert_waffle_flag_attribute(
-            mock_set_custom_attribute,
-            expected_flag_value=str(False),
-        )
-
     def test_without_request_and_undefined_waffle(self):
         """
         Test the flag behavior when outside a request context and waffle data undefined.
@@ -147,36 +131,6 @@ class TestCourseWaffleFlag(TestCase):
         with override_flag(self.NAMESPACED_FLAG_NAME, active=True):
             self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), True)
 
-    @ddt.data(
-        {'expected_count': 0, 'waffle_flag_attribute_setting': None},
-        {'expected_count': 1, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME]},
-        {'expected_count': 2, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME, NAMESPACED_FLAG_2_NAME]},
-    )
-    @patch.object(edx_toggles.toggles.internal.waffle, 'set_custom_attribute')
-    def test_waffle_flag_attribute_for_various_settings(self, data, mock_set_custom_attribute):
-        """
-        Test that custom attributes are recorded when waffle flag accessed.
-        """
-        with override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=data['waffle_flag_attribute_setting']):
-            test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME, __name__)
-            test_course_flag.is_enabled(self.TEST_COURSE_KEY)
-            test_course_flag_2 = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_2_NAME, __name__)
-            test_course_flag_2.is_enabled(self.TEST_COURSE_KEY)
-
-        self.assertEqual(mock_set_custom_attribute.call_count, data['expected_count'])
-
-    def _assert_waffle_flag_attribute(self, mock_set_custom_attribute, expected_flag_value=None):
-        """
-        Assert that a custom attribute was set as expected on the mock.
-        """
-        if expected_flag_value:
-            expected_flag_name = 'flag_{}'.format(self.NAMESPACED_FLAG_NAME)
-            expected_calls = [call(expected_flag_name, expected_flag_value)]
-            mock_set_custom_attribute.assert_has_calls(expected_calls)
-            self.assertEqual(mock_set_custom_attribute.call_count, 1)
-        else:
-            self.assertEqual(mock_set_custom_attribute.call_count, 0)
-
 
 class DeprecatedWaffleFlagTests(TestCase):
     """