Patterns for better bok choy tests

* visit() already does a wait_for_page()
* wait_for_page() already asserts on is_browser_on_page(), as it will raise a BrokenPromise after polling
* don't use is_browser_on_page() in an assertion unless you are 100% certain you're already on the page
* if an action takes you to another page, synchronize with a wait_for_page() for that new page

common/test/acceptance/pages/lms/teams.py

@@ -163,6 +163,9 @@ class BrowseTopicsPage(CoursePage, PaginatedUIMixin):
 
     def is_browser_on_page(self):
         """Check if the Browse tab is being viewed."""
+        # First off, you need to make sure that you're on the Teams page.
+        if not self.q(css='.teams-main').visible:
+            return False
         button_classes = self.q(css=BROWSE_BUTTON_CSS).attrs('class')
         if len(button_classes) == 0:
             return False
@@ -211,6 +214,8 @@ class BaseTeamsPage(CoursePage, PaginatedUIMixin, TeamCardsMixin, BreadcrumbsMix
         the same convention as a course module's topic.
         """
         super(BaseTeamsPage, self).__init__(browser, course_id)
+        self.browser = browser
+        self.course_id = course_id
         self.topic = topic
 
     def is_browser_on_page(self):
@@ -248,7 +253,10 @@ class BaseTeamsPage(CoursePage, PaginatedUIMixin, TeamCardsMixin, BreadcrumbsMix
         query = self.q(css=CREATE_TEAM_LINK_CSS)
         if query.present:
             query.first.click()
-            self.wait_for_ajax()
+
+        # This will bring you to the team management page
+        team_management_page = TeamManagementPage(self.browser, self.course_id, self.topic)
+        team_management_page.wait_for_page()
 
     def click_search_team_link(self):
         """ Click on create team link."""
@@ -332,7 +340,9 @@ class TeamManagementPage(CoursePage, FieldsMixin, BreadcrumbsMixin):
 
     def is_browser_on_page(self):
         """Check if we're on the create team page for a particular topic."""
-        return self.q(css='.team-edit-fields').present
+        fields_css = '.team-edit-fields'
+        button_sr_css = '.action.action-primary > .sr'
+        return self.q(css=fields_css).present and self.q(css=button_sr_css).visible
 
     @property
     def header_page_name(self):
@@ -349,6 +359,15 @@ class TeamManagementPage(CoursePage, FieldsMixin, BreadcrumbsMixin):
         """Get the error message text"""
         return self.q(css='.create-team.wrapper-msg .copy')[0].text
 
+    def create_team(self, name='Team Name', description='Team description.'):
+        """Create a new team"""
+        self.value_for_text_field(field_id='name', value=name, press_enter=False)
+        self.set_value_for_textarea_field(
+            field_id='description',
+            value=description
+        )
+        self.submit_form()
+
     def submit_form(self):
         """Click on create team button"""
         self.q(css='.create-team .action-primary').first.click()
@@ -444,7 +463,7 @@ class TeamPage(CoursePage, PaginatedUIMixin, BreadcrumbsMixin):
         if self.team:
             if not self.url.endswith(self.url_path):
                 return False
-        return self.q(css='.team-profile').present
+        return self.q(css='.teams-main .team-members').visible
 
     @property
     def discussion_id(self):
@@ -502,7 +521,9 @@ class TeamPage(CoursePage, PaginatedUIMixin, BreadcrumbsMixin):
 
     def click_leave_team_link(self, remaining_members=0, cancel=False):
         """ Click on Leave Team link"""
-        click_css(self, '.leave-team-link', require_notification=False)
+        leave_team_css = '.leave-team-link'
+        self.wait_for_element_visibility(leave_team_css, 'Leave Team link is visible.')
+        click_css(self, leave_team_css, require_notification=False)
         confirm_prompt(self, cancel, require_notification=False)
 
         if cancel is False:

common/test/acceptance/tests/lms/test_lms.py

@@ -1146,8 +1146,6 @@ class EntranceExamTest(UniqueCourseTest):
 
         # visit course settings page and set/enabled entrance exam for that course.
         self.settings_page.visit()
-        self.settings_page.wait_for_page()
-        self.assertTrue(self.settings_page.is_browser_on_page())
         self.settings_page.entrance_exam_field.click()
         self.settings_page.save_changes()
 

common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py

@@ -76,7 +76,6 @@ class BulkEmailTest(BaseInstructorDashboardTest):
 
     @ddt.data(["myself"], ["staff"], ["learners"], ["myself", "staff", "learners"])
     def test_email_queued_for_sending(self, recipient):
-        self.assertTrue(self.send_email_page.is_browser_on_page())
         self.send_email_page.send_message(recipient)
         self.send_email_page.verify_message_queued_successfully()
 

common/test/acceptance/tests/lms/test_lms_problems.py

@@ -608,7 +608,7 @@ class LogoutDuringAnswering(ProblemsTest):
         with problem_page.handle_alert(confirm=False):
             problem_page.click_submit()
 
-        self.assertTrue(problem_page.is_browser_on_page())
+        problem_page.wait_for_page()
         self.assertEqual(problem_page.problem_name, 'TEST PROBLEM')
 
 

common/test/acceptance/tests/lms/test_teams.py

@@ -67,6 +67,7 @@ class TeamsTabBase(EventsTestMixin, UniqueCourseTest):
             # Sadly, this sleep is necessary in order to ensure that
             # sorting by last_activity_at works correctly when running
             # in Jenkins.
+            # THIS IS AN ANTI-PATTERN - DO NOT COPY.
             time.sleep(time_between_creation)
         return teams
 
@@ -423,19 +424,16 @@ class BrowseTopicsTest(TeamsTabBase):
         topic = [t for t in topics if t['name'] == topic_name][0]
         self.topics_page.browse_teams_for_topic(topic_name)
         browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, topic)
-        self.assertTrue(browse_teams_page.is_browser_on_page())
+        browse_teams_page.wait_for_page()
         browse_teams_page.click_create_team_link()
         create_team_page = TeamManagementPage(self.browser, self.course_id, topic)
-        create_team_page.value_for_text_field(field_id='name', value='Team Name', press_enter=False)
-        create_team_page.set_value_for_textarea_field(
-            field_id='description',
-            value='Team description.'
-        )
-        create_team_page.submit_form()
+        create_team_page.create_team()
+
         team_page = TeamPage(self.browser, self.course_id)
-        self.assertTrue(team_page.is_browser_on_page())
+        team_page.wait_for_page()
+
         team_page.click_all_topics()
-        self.assertTrue(self.topics_page.is_browser_on_page())
+        self.topics_page.wait_for_page()
         self.topics_page.wait_for_ajax()
         self.assertEqual(topic_name, self.topics_page.topic_names[0])
 
@@ -575,7 +573,7 @@ class BrowseTopicsTest(TeamsTabBase):
         self.topics_page.visit()
         self.topics_page.browse_teams_for_topic('Example Topic')
         browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, topic)
-        self.assertTrue(browse_teams_page.is_browser_on_page())
+        browse_teams_page.wait_for_page()
         self.assertEqual(browse_teams_page.header_name, 'Example Topic')
         self.assertEqual(browse_teams_page.header_description, 'Description')
 
@@ -827,7 +825,7 @@ class BrowseTeamsWithinTopicTest(TeamsTabBase):
         self.verify_page_header()
 
         self.browse_teams_page.click_browse_all_teams_link()
-        self.assertTrue(self.topics_page.is_browser_on_page())
+        self.topics_page.wait_for_page()
 
     def test_search(self):
         """
@@ -1172,7 +1170,7 @@ class CreateTeamTest(TeamFormActions):
 
         self.team_management_page.cancel_team()
 
-        self.assertTrue(self.browse_teams_page.is_browser_on_page())
+        self.browse_teams_page.wait_for_page()
         self.assertTrue(self.browse_teams_page.get_pagination_header_text().startswith('Showing 0 out of 0 total'))
 
         self.teams_page.click_all_topics()
@@ -1235,7 +1233,7 @@ class DeleteTeamTest(TeamFormActions):
         Then I should still see the team
         """
         self.delete_team(cancel=True)
-        self.assertTrue(self.team_management_page.is_browser_on_page())
+        self.team_management_page.wait_for_page()
         self.browser.refresh()
         self.team_management_page.wait_for_page()
         self.assertEqual(
@@ -1268,7 +1266,7 @@ class DeleteTeamTest(TeamFormActions):
         self.team_page.visit()
         self.delete_team(require_notification=False)
         browse_teams_page = BrowseTeamsPage(self.browser, self.course_id, self.topic)
-        self.assertTrue(browse_teams_page.is_browser_on_page())
+        browse_teams_page.wait_for_page()
         self.assertNotIn(self.team['name'], browse_teams_page.team_names)
 
     def delete_team(self, **kwargs):
@@ -1318,7 +1316,7 @@ class DeleteTeamTest(TeamFormActions):
         self.delete_team(require_notification=False)
         BrowseTeamsPage(self.browser, self.course_id, self.topic).click_all_topics()
         topics_page = BrowseTopicsPage(self.browser, self.course_id)
-        self.assertTrue(topics_page.is_browser_on_page())
+        topics_page.wait_for_page()
         self.teams_page.verify_topic_team_count(0)
 
 
@@ -1604,7 +1602,7 @@ class EditMembershipTest(TeamFormActions):
             ):
                 self.edit_membership_page.confirm_delete_membership_dialog()
             self.assertEqual(self.edit_membership_page.team_members, 0)
-        self.assertTrue(self.edit_membership_page.is_browser_on_page)
+        self.edit_membership_page.wait_for_page()
 
     @ddt.data('Moderator', 'Community TA', 'Administrator', None)
     def test_remove_membership(self, role):
@@ -1718,7 +1716,7 @@ class TeamPageTest(TeamsTabBase):
         self.team_page.visit()
         self.assertEqual(self.team_page.discussion_id, self.teams[0]['discussion_topic_id'])
         discussion = self.team_page.discussion_page
-        self.assertTrue(discussion.is_browser_on_page())
+        discussion.wait_for_page()
         self.assertTrue(discussion.is_discussion_expanded())
         self.assertEqual(discussion.get_num_displayed_threads(), 1)
         self.assertTrue(discussion.has_thread(thread['id']))

common/test/acceptance/tests/studio/test_import_export.py

@@ -105,11 +105,7 @@ class BadExportMixin(object):
         self.export_page.click_export()
         self.export_page.wait_for_error_modal()
         self.export_page.click_modal_button()
-        EmptyPromise(
-            lambda: self.edit_page.is_browser_on_page,
-            'Arrived at component edit page',
-            timeout=30
-        )
+        self.edit_page.wait_for_page()
 
 
 @attr(shard=7)

common/test/acceptance/tests/studio/test_studio_container.py

@@ -974,7 +974,7 @@ class UnitPublishingTest(ContainerBase):
         Verifies that the browser is on the staff page and returns a StaffPage.
         """
         page = StaffPage(self.browser, self.course_id)
-        EmptyPromise(page.is_browser_on_page, 'Browser is on staff page in LMS').fulfill()
+        page.wait_for_page()
         return page
 
     def _verify_student_view_locked(self):

common/test/acceptance/tests/studio/test_studio_outline.py

@@ -1075,7 +1075,7 @@ class CreateSectionsTest(CourseOutlineTest):
         self.assertEqual(len(self.course_outline_page.section_at(0).subsections()), 1)
         self.course_outline_page.section_at(0).subsection_at(0).add_unit()
         unit_page = ContainerPage(self.browser, None)
-        EmptyPromise(unit_page.is_browser_on_page, 'Browser is on the unit page').fulfill()
+        unit_page.wait_for_page()
         self.assertTrue(unit_page.is_inline_editing_display_name())
 
 
@@ -1473,7 +1473,7 @@ class UnitNavigationTest(CourseOutlineTest):
         self.course_outline_page.visit()
         self.course_outline_page.section_at(0).subsection_at(0).expand_subsection()
         unit = self.course_outline_page.section_at(0).subsection_at(0).unit_at(0).go_to()
-        self.assertTrue(unit.is_browser_on_page)
+        unit.wait_for_page()
 
 
 @attr(shard=3)

common/test/acceptance/tests/studio/test_studio_settings.py

@@ -228,10 +228,7 @@ class ContentGroupConfigurationTest(StudioCourseTest):
         config.click_outline_anchor()
 
         # Waiting for the page load and verify that we've landed on course outline page
-        EmptyPromise(
-            lambda: self.outline_page.is_browser_on_page(), "loaded page {!r}".format(self.outline_page),
-            timeout=30
-        ).fulfill()
+        self.outline_page.wait_for_page()
 
 
 @attr(shard=8)
@@ -253,7 +250,6 @@ class AdvancedSettingsValidationTest(StudioCourseTest):
 
         # Before every test, make sure to visit the page first
         self.advanced_settings.visit()
-        self.assertTrue(self.advanced_settings.is_browser_on_page())
 
     def test_modal_shows_one_validation_error(self):
         """

common/test/acceptance/tests/studio/test_studio_settings_details.py

@@ -34,7 +34,6 @@ class StudioSettingsDetailsTest(StudioCourseTest):
 
         # Before every test, make sure to visit the page first
         self.settings_detail.visit()
-        self.assertTrue(self.settings_detail.is_browser_on_page())
 
 
 @attr(shard=4)

common/test/acceptance/tests/studio/test_studio_split_test.py

@@ -759,10 +759,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
         config.click_outline_anchor()
 
         # Waiting for the page load and verify that we've landed on course outline page
-        EmptyPromise(
-            lambda: self.outline_page.is_browser_on_page(), "loaded page {!r}".format(self.outline_page),
-            timeout=30
-        ).fulfill()
+        self.outline_page.wait_for_page()
 
     def test_group_configuration_non_empty_usage(self):
         """
@@ -806,10 +803,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
 
         unit = ContainerPage(self.browser, vertical.locator)
         # Waiting for the page load and verify that we've landed on the unit page
-        EmptyPromise(
-            lambda: unit.is_browser_on_page(), "loaded page {!r}".format(unit),
-            timeout=30
-        ).fulfill()
+        unit.wait_for_page()
 
         self.assertIn(unit.name, usage)