Merge pull request #3999 from edx/christina/updating-split-test

Ability to change the associated group configuration for a split test

common/test/acceptance/fixtures/course.py

@@ -200,6 +200,7 @@ class CourseFixture(StudioApiFixture):
         self._handouts = []
         self._children = []
         self._assets = []
+        self._advanced_settings = {}
 
     def __str__(self):
         """
@@ -236,6 +237,12 @@ class CourseFixture(StudioApiFixture):
         """
         self._assets.extend(asset_name)
 
+    def add_advanced_settings(self, settings):
+        """
+        Adds advanced settings to be set on the course when the install method is called.
+        """
+        self._advanced_settings.update(settings)
+
     def install(self):
         """
         Create the course and XBlocks within the course.
@@ -248,6 +255,7 @@ class CourseFixture(StudioApiFixture):
         self._install_course_handouts()
         self._configure_course()
         self._upload_assets()
+        self._add_advanced_settings()
         self._create_xblock_children(self._course_location, self._children)
 
         return self
@@ -415,6 +423,23 @@ class CourseFixture(StudioApiFixture):
                 raise CourseFixtureError('Could not upload {asset_name} with {url}. Status code: {code}'.format(
                     asset_name=asset_name, url=url, code=upload_response.status_code))
 
+    def _add_advanced_settings(self):
+        """
+        Add advanced settings.
+        """
+        url = STUDIO_BASE_URL + "/settings/advanced/" + self._course_key
+
+        # POST advanced settings to Studio
+        response = self.session.post(
+            url, data=self._encode_post_dict(self._advanced_settings),
+            headers=self.headers,
+        )
+
+        if not response.ok:
+            raise CourseFixtureError(
+                "Could not update advanced details to '{0}' with {1}: Status was {2}.".format(
+                    self._advanced_settings, url, response.status_code))
+
     def _create_xblock_children(self, parent_loc, xblock_descriptions):
         """
         Recursively create XBlock children.
@@ -489,6 +514,6 @@ class CourseFixture(StudioApiFixture):
         Encode `post_dict` (a dictionary) as UTF-8 encoded JSON.
         """
         return json.dumps({
-            k: v.encode('utf-8') if v is not None else v
+            k: v.encode('utf-8') if isinstance(v, basestring) else v
             for k, v in post_dict.items()
         })

common/test/acceptance/pages/studio/component_editor.py

@@ -2,6 +2,7 @@ from bok_choy.page_object import PageObject
 from selenium.webdriver.common.keys import Keys
 from selenium.webdriver.common.action_chains import ActionChains
 from utils import click_css
+from selenium.webdriver.support.ui import Select
 
 
 class ComponentEditorView(PageObject):
@@ -40,7 +41,7 @@ class ComponentEditorView(PageObject):
         """
         return None
 
-    def get_setting_entry_index(self, label):
+    def get_setting_element(self, label):
         """
         Returns the index of the setting entry with given label (display name) within the Settings modal.
         """
@@ -48,15 +49,14 @@ class ComponentEditorView(PageObject):
         setting_labels = self.q(css=self._bounded_selector('.metadata_edit .wrapper-comp-setting .setting-label'))
         for index, setting in enumerate(setting_labels):
             if setting.text == label:
-                return index
+                return self.q(css=self._bounded_selector('.metadata_edit div.wrapper-comp-setting .setting-input'))[index]
         return None
 
     def set_field_value_and_save(self, label, value):
         """
-        Set the field with given label (display name) to the specified value, and presses Save.
+        Sets the text field with given label (display name) to the specified value, and presses Save.
         """
-        index = self.get_setting_entry_index(label)
-        elem = self.q(css=self._bounded_selector('.metadata_edit div.wrapper-comp-setting input.setting-input'))[index]
+        elem = self.get_setting_element(label)
         # Click in the field, delete the value there.
         action = ActionChains(self.browser).click(elem)
         for _x in range(0, len(elem.get_attribute('value'))):
@@ -64,3 +64,12 @@ class ComponentEditorView(PageObject):
         # Send the new text, then Tab to move to the next field (so change event is triggered).
         action.send_keys(value).send_keys(Keys.TAB).perform()
         click_css(self, 'a.action-save')
+
+    def set_select_value_and_save(self, label, value):
+        """
+        Sets the select with given label (display name) to the specified value, and presses Save.
+        """
+        elem = self.get_setting_element(label)
+        select = Select(elem)
+        select.select_by_value(value)
+        click_css(self, 'a.action-save')

common/test/acceptance/pages/studio/container.py

@@ -54,7 +54,24 @@ class ContainerPage(PageObject):
         """
         Return a list of xblocks loaded on the container page.
         """
-        return self.q(css=XBlockWrapper.BODY_SELECTOR).map(
+        return self._get_xblocks()
+
+    @property
+    def inactive_xblocks(self):
+        """
+        Return a list of inactive xblocks loaded on the container page.
+        """
+        return self._get_xblocks(".is-inactive ")
+
+    @property
+    def active_xblocks(self):
+        """
+        Return a list of active xblocks loaded on the container page.
+        """
+        return self._get_xblocks(".is-active ")
+
+    def _get_xblocks(self, prefix=""):
+        return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map(
             lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results
 
     def drag(self, source_index, target_index):
@@ -77,15 +94,6 @@ class ContainerPage(PageObject):
         ).release().perform()
         wait_for_notification(self)
 
-    def add_discussion(self, menu_index):
-        """
-        Add a new instance of the discussion category.
-
-        menu_index specifies which instance of the menus should be used (based on vertical
-        placement within the page).
-        """
-        click_css(self, 'a>span.large-discussion-icon', menu_index)
-
     def duplicate(self, source_index):
         """
         Duplicate the item with index source_index (based on vertical placement in page).
@@ -101,6 +109,11 @@ class ContainerPage(PageObject):
         click_css(self, 'a.button.action-primary', 0)
 
     def edit(self):
+        """
+        Clicks the "edit" button for the first component on the page.
+
+        Same as the implementation in unit.py, unit and component pages will be merging.
+        """
         self.q(css='.edit-button').first.click()
         EmptyPromise(
             lambda: self.q(css='.xblock-studio_view').present,
@@ -109,6 +122,18 @@ class ContainerPage(PageObject):
 
         return self
 
+    def add_missing_groups(self):
+        """
+        Click the "add missing groups" link.
+        """
+        click_css(self, '.add-missing-groups-button')
+
+    def missing_groups_button_present(self):
+        """
+        Returns True if the "add missing groups" button is present.
+        """
+        return self.q(css='.add-missing-groups-button').present
+
 
 class XBlockWrapper(PageObject):
     """
@@ -161,4 +186,4 @@ class XBlockWrapper(PageObject):
 
     @property
     def preview_selector(self):
-        return self._bounded_selector('.xblock-student_view')
+        return self._bounded_selector('.xblock-student_view,.xblock-author_view')

common/test/acceptance/pages/studio/unit.py

@@ -27,7 +27,7 @@ class UnitPage(PageObject):
 
         def _is_finished_loading():
             # Wait until all components have been loaded
-            number_of_leaf_xblocks = len(self.q(css='{} .xblock-student_view'.format(Component.BODY_SELECTOR)).results)
+            number_of_leaf_xblocks = len(self.q(css='{} .xblock-author_view,.xblock-student_view'.format(Component.BODY_SELECTOR)).results)
             is_done = len(self.q(css=Component.BODY_SELECTOR).results) == number_of_leaf_xblocks
             return (is_done, is_done)
 
@@ -99,9 +99,14 @@ class Component(PageObject):
 
     @property
     def preview_selector(self):
-        return self._bounded_selector('.xblock-student_view')
+        return self._bounded_selector('.xblock-author_view,.xblock-student_view')
 
     def edit(self):
+        """
+        Clicks the "edit" button for the first component on the page.
+
+        Same as the implementation in unit.py, unit and component pages will be merging.
+        """
         self.q(css=self._bounded_selector('.edit-button')).first.click()
         EmptyPromise(
             lambda: self.q(css='.xblock-studio_view').present,

common/test/acceptance/pages/studio/utils.py

@@ -9,10 +9,12 @@ def click_css(page, css, source_index=0, require_notification=True):
     """
     Click the button/link with the given css and index on the specified page (subclass of PageObject).
 
+    Will only consider buttons with a non-zero size.
+
     If require_notification is False (default value is True), the method will return immediately.
     Otherwise, it will wait for the "mini-notification" to appear and disappear.
     """
-    buttons = page.q(css=css)
+    buttons = page.q(css=css).filter(lambda el: el.size['width'] > 0)
     target = buttons[source_index]
     ActionChains(page.browser).click(target).release().perform()
     if require_notification:
@@ -31,5 +33,36 @@ def wait_for_notification(page):
         num_notifications = len(page.q(css='.wrapper-notification-mini.is-hiding'))
         return (num_notifications == 1, num_notifications)
 
-    Promise(_is_saving, 'Notification showing.').fulfill()
-    Promise(_is_saving_done, 'Notification hidden.').fulfill()
+    Promise(_is_saving, 'Notification should have been shown.').fulfill()
+    Promise(_is_saving_done, 'Notification should have been hidden.').fulfill()
+
+
+def add_discussion(page, menu_index):
+    """
+    Add a new instance of the discussion category.
+
+    menu_index specifies which instance of the menus should be used (based on vertical
+    placement within the page).
+    """
+    click_css(page, 'a>span.large-discussion-icon', menu_index)
+
+
+def add_advanced_component(page, menu_index, name):
+    """
+    Adds an instance of the advanced component with the specified name.
+
+    menu_index specifies which instance of the menus should be used (based on vertical
+    placement within the page).
+    """
+    click_css(page, 'a>span.large-advanced-icon', menu_index, require_notification=False)
+
+    # Sporadically, the advanced component was not getting created after the click_css call on the category (below).
+    # Try making sure that the menu of advanced components is visible before clicking (the HTML is always on the
+    # page, but will have display none until the large-advanced-icon is clicked).
+    def is_advanced_components_showing():
+        advanced_buttons = page.q(css=".new-component-advanced").filter(lambda el: el.size['width'] > 0)
+        return (len(advanced_buttons) == 1, len(advanced_buttons))
+
+    Promise(is_advanced_components_showing, "Advanced component menu not showing").fulfill()
+
+    click_css(page, 'a[data-category={}]'.format(name))

common/test/acceptance/tests/test_studio_container.py

@@ -8,6 +8,7 @@ from ..fixtures.course import CourseFixture, XBlockFixtureDesc
 
 from .helpers import UniqueCourseTest
 from ..pages.studio.component_editor import ComponentEditorView
+from ..pages.studio.utils import add_discussion
 
 from unittest import skip
 
@@ -32,6 +33,87 @@ class ContainerBase(UniqueCourseTest):
             self.course_info['run']
         )
 
+        self.setup_fixtures()
+
+        self.auth_page = AutoAuthPage(
+            self.browser,
+            staff=False,
+            username=self.user.get('username'),
+            email=self.user.get('email'),
+            password=self.user.get('password')
+        )
+
+        self.auth_page.visit()
+
+    def setup_fixtures(self):
+        pass
+
+    def go_to_container_page(self, make_draft=False):
+        """
+        Go to the test container page.
+
+        If make_draft is true, the unit page (accessed on way to container page) will be put into draft mode.
+        """
+        unit = self.go_to_unit_page(make_draft)
+        container = unit.components[0].go_to_container()
+        return container
+
+    def go_to_unit_page(self, make_draft=False):
+        """
+        Go to the test unit page.
+
+        If make_draft is true, the unit page will be put into draft mode.
+        """
+        self.outline.visit()
+        subsection = self.outline.section('Test Section').subsection('Test Subsection')
+        unit = subsection.toggle_expand().unit('Test Unit').go_to()
+        if make_draft:
+            unit.edit_draft()
+        return unit
+
+    def verify_ordering(self, container, expected_orderings):
+        """
+        Verifies the expected ordering of xblocks on the page.
+        """
+        xblocks = container.xblocks
+        blocks_checked = set()
+        for expected_ordering in expected_orderings:
+            for xblock in xblocks:
+                parent = expected_ordering.keys()[0]
+                if xblock.name == parent:
+                    blocks_checked.add(parent)
+                    children = xblock.children
+                    expected_length = len(expected_ordering.get(parent))
+                    self.assertEqual(
+                        expected_length, len(children),
+                        "Number of children incorrect for group {0}. Expected {1} but got {2}.".format(parent, expected_length, len(children)))
+                    for idx, expected in enumerate(expected_ordering.get(parent)):
+                        self.assertEqual(expected, children[idx].name)
+                        blocks_checked.add(expected)
+                    break
+        self.assertEqual(len(blocks_checked), len(xblocks))
+
+    def do_action_and_verify(self, action, expected_ordering):
+        """
+        Perform the supplied action and then verify the resulting ordering.
+        """
+        container = self.go_to_container_page(make_draft=True)
+        action(container)
+
+        self.verify_ordering(container, expected_ordering)
+
+        # Reload the page to see that the change was persisted.
+        container = self.go_to_container_page()
+        self.verify_ordering(container, expected_ordering)
+
+
+class NestedVerticalTest(ContainerBase):
+    __test__ = False
+
+    """
+    Sets up a course structure with nested verticals.
+    """
+    def setup_fixtures(self):
         self.container_title = ""
         self.group_a = "Expand or Collapse\nGroup A"
         self.group_b = "Expand or Collapse\nGroup B"
@@ -55,18 +137,6 @@ class ContainerBase(UniqueCourseTest):
         self.duplicate_label = "Duplicate of '{0}'"
         self.discussion_label = "Discussion"
 
-        self.setup_fixtures()
-
-        self.auth_page = AutoAuthPage(
-            self.browser,
-            staff=False,
-            username=self.user.get('username'),
-            email=self.user.get('email'),
-            password=self.user.get('password')
-        )
-        self.auth_page.visit()
-
-    def setup_fixtures(self):
         course_fix = CourseFixture(
             self.course_info['org'],
             self.course_info['number'],
@@ -96,46 +166,8 @@ class ContainerBase(UniqueCourseTest):
 
         self.user = course_fix.user
 
-    def go_to_container_page(self, make_draft=False):
-        unit = self.go_to_unit_page(make_draft)
-        container = unit.components[0].go_to_container()
-        return container
 
-    def go_to_unit_page(self, make_draft=False):
-        self.outline.visit()
-        subsection = self.outline.section('Test Section').subsection('Test Subsection')
-        unit = subsection.toggle_expand().unit('Test Unit').go_to()
-        if make_draft:
-            unit.edit_draft()
-        return unit
-
-    def verify_ordering(self, container, expected_orderings):
-        xblocks = container.xblocks
-        for expected_ordering in expected_orderings:
-            for xblock in xblocks:
-                parent = expected_ordering.keys()[0]
-                if xblock.name == parent:
-                    children = xblock.children
-                    expected_length = len(expected_ordering.get(parent))
-                    self.assertEqual(
-                        expected_length, len(children),
-                        "Number of children incorrect for group {0}. Expected {1} but got {2}.".format(parent, expected_length, len(children)))
-                    for idx, expected in enumerate(expected_ordering.get(parent)):
-                        self.assertEqual(expected, children[idx].name)
-                    break
-
-    def do_action_and_verify(self, action, expected_ordering):
-        container = self.go_to_container_page(make_draft=True)
-        action(container)
-
-        self.verify_ordering(container, expected_ordering)
-
-        # Reload the page to see that the change was persisted.
-        container = self.go_to_container_page()
-        self.verify_ordering(container, expected_ordering)
-
-
-class DragAndDropTest(ContainerBase):
+class DragAndDropTest(NestedVerticalTest):
     """
     Tests of reordering within the container page.
     """
@@ -196,7 +228,7 @@ class DragAndDropTest(ContainerBase):
 
         def add_new_components_and_rearrange(container):
             # Add a video component to Group 1
-            container.add_discussion(group_a_menu)
+            add_discussion(container, group_a_menu)
             # Duplicate the first item in Group A
             container.duplicate(self.group_a_item_1_action_index)
 
@@ -216,7 +248,7 @@ class DragAndDropTest(ContainerBase):
         self.do_action_and_verify(add_new_components_and_rearrange, expected_ordering)
 
 
-class AddComponentTest(ContainerBase):
+class AddComponentTest(NestedVerticalTest):
     """
     Tests of adding a component to the container page.
     """
@@ -224,7 +256,7 @@ class AddComponentTest(ContainerBase):
 
     def add_and_verify(self, menu_index, expected_ordering):
         self.do_action_and_verify(
-            lambda (container): container.add_discussion(menu_index),
+            lambda (container): add_discussion(container, menu_index),
             expected_ordering
         )
 
@@ -256,7 +288,7 @@ class AddComponentTest(ContainerBase):
         self.add_and_verify(container_menu, expected_ordering)
 
 
-class DuplicateComponentTest(ContainerBase):
+class DuplicateComponentTest(NestedVerticalTest):
     """
     Tests of duplicating a component on the container page.
     """
@@ -302,7 +334,7 @@ class DuplicateComponentTest(ContainerBase):
         self.do_action_and_verify(duplicate_twice, expected_ordering)
 
 
-class DeleteComponentTest(ContainerBase):
+class DeleteComponentTest(NestedVerticalTest):
     """
     Tests of deleting a component from the container page.
     """
@@ -319,10 +351,13 @@ class DeleteComponentTest(ContainerBase):
                              {self.group_a: [self.group_a_item_2]},
                              {self.group_b: [self.group_b_item_1, self.group_b_item_2]},
                              {self.group_empty: []}]
-        self.delete_and_verify(self.group_a_item_1_action_index, expected_ordering)
+
+        # Group A itself has a delete icon now, so item_1 is index 1 instead of 0.
+        group_a_item_1_delete_index = 1
+        self.delete_and_verify(group_a_item_1_delete_index, expected_ordering)
 
 
-class EditContainerTest(ContainerBase):
+class EditContainerTest(NestedVerticalTest):
     """
     Tests of editing a container.
     """

common/test/acceptance/tests/test_studio_split_test.py

@@ -0,0 +1,156 @@
+"""
+Acceptance tests for Studio related to the split_test module.
+"""
+
+from unittest import skip
+
+from ..fixtures.course import CourseFixture, XBlockFixtureDesc
+
+from ..pages.studio.component_editor import ComponentEditorView
+from test_studio_container import ContainerBase
+from ..pages.studio.utils import add_advanced_component
+from xmodule.partitions.partitions import Group, UserPartition
+from bok_choy.promise import Promise
+
+
+class SplitTest(ContainerBase):
+    """
+    Tests for creating and editing split test instances in Studio.
+    """
+    __test__ = True
+
+    def setup_fixtures(self):
+        course_fix = CourseFixture(
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run'],
+            self.course_info['display_name']
+        )
+
+        course_fix.add_advanced_settings(
+            {
+                u"advanced_modules": ["split_test"],
+                u"user_partitions": [
+                    UserPartition(0, 'Configuration alpha,beta', 'first', [Group("0", 'alpha'), Group("1", 'beta')]).to_json(),
+                    UserPartition(1, 'Configuration 0,1,2', 'second', [Group("0", 'Group 0'), Group("1", 'Group 1'), Group("2", 'Group 2')]).to_json()
+                ]
+            }
+        )
+
+        course_fix.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
+                    XBlockFixtureDesc('vertical', 'Test Unit')
+                )
+            )
+        ).install()
+
+        self.course_fix = course_fix
+
+        self.user = course_fix.user
+
+    def verify_groups(self, container, active_groups, inactive_groups, verify_missing_groups_not_present=True):
+        """
+        Check that the groups appear and are correctly categorized as to active and inactive.
+
+        Also checks that the "add missing groups" button/link is not present unless a value of False is passed
+        for verify_missing_groups_not_present.
+        """
+        def wait_for_xblocks_to_render():
+            # First xblock is the container for the page, subtract 1.
+            return (len(active_groups) + len(inactive_groups) == len(container.xblocks) - 1, len(active_groups))
+
+        Promise(wait_for_xblocks_to_render, "Number of xblocks on the page are incorrect").fulfill()
+
+        def check_xblock_names(expected_groups, actual_blocks):
+            self.assertEqual(len(expected_groups), len(actual_blocks))
+            for idx, expected in enumerate(expected_groups):
+                self.assertEqual('Expand or Collapse\n{}'.format(expected), actual_blocks[idx].name)
+
+        check_xblock_names(active_groups, container.active_xblocks)
+        check_xblock_names(inactive_groups, container.inactive_xblocks)
+
+        # Verify inactive xblocks appear after active xblocks
+        check_xblock_names(active_groups + inactive_groups, container.xblocks[1:])
+
+        if verify_missing_groups_not_present:
+            self.verify_add_missing_groups_button_not_present(container)
+
+    def verify_add_missing_groups_button_not_present(self, container):
+        """
+        Checks that the "add missing gorups" button/link is not present.
+        """
+        def missing_groups_button_not_present():
+            button_present = container.missing_groups_button_present()
+            return (not button_present, not button_present)
+
+        Promise(missing_groups_button_not_present, "Add missing groups button should not be showing.").fulfill()
+
+    def create_poorly_configured_split_instance(self):
+        """
+        Creates a split test instance with a missing group and an inactive group.
+
+        Returns the container page.
+        """
+        unit = self.go_to_unit_page(make_draft=True)
+        add_advanced_component(unit, 0, 'split_test')
+        container = self.go_to_container_page()
+        container.edit()
+        component_editor = ComponentEditorView(self.browser, container.locator)
+        component_editor.set_select_value_and_save('Group Configuration', 'Configuration alpha,beta')
+        self.course_fix.add_advanced_settings(
+            {
+                u"user_partitions": [
+                    UserPartition(0, 'Configuration alpha,beta', 'first',
+                                  [Group("0", 'alpha'), Group("2", 'gamma')]).to_json()
+                ]
+            }
+        )
+        self.course_fix._add_advanced_settings()
+        return self.go_to_container_page()
+
+    def test_create_and_select_group_configuration(self):
+        """
+        Tests creating a split test instance on the unit page, and then
+        assigning the group configuration.
+        """
+        unit = self.go_to_unit_page(make_draft=True)
+        add_advanced_component(unit, 0, 'split_test')
+        container = self.go_to_container_page()
+        container.edit()
+        component_editor = ComponentEditorView(self.browser, container.locator)
+        component_editor.set_select_value_and_save('Group Configuration', 'Configuration alpha,beta')
+        self.verify_groups(container, ['alpha', 'beta'], [])
+
+        # Switch to the other group configuration. Must navigate again to the container page so
+        # that there is only a single "editor" on the page.
+        container = self.go_to_container_page()
+        container.edit()
+        component_editor = ComponentEditorView(self.browser, container.locator)
+        component_editor.set_select_value_and_save('Group Configuration', 'Configuration 0,1,2')
+        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['alpha', 'beta'])
+
+        # Reload the page to make sure the groups were persisted.
+        container = self.go_to_container_page()
+        self.verify_groups(container, ['Group 0', 'Group 1', 'Group 2'], ['alpha', 'beta'])
+
+    @skip("This fails periodically where it fails to trigger the add missing groups action.Dis")
+    def test_missing_group(self):
+        """
+        The case of a split test with invalid configuration (missing group).
+        """
+        container = self.create_poorly_configured_split_instance()
+        container.add_missing_groups()
+        self.verify_groups(container, ['alpha', 'gamma'], ['beta'])
+
+        # Reload the page to make sure the groups were persisted.
+        container = self.go_to_container_page()
+        self.verify_groups(container, ['alpha', 'gamma'], ['beta'])
+
+    def test_delete_inactive_group(self):
+        """
+        Test deleting an inactive group.
+        """
+        container = self.create_poorly_configured_split_instance()
+        container.delete(0)
+        self.verify_groups(container, ['alpha'], [], verify_missing_groups_not_present=False)