From dd350051321fa47d86b2030798d8d665b2cf56fe Mon Sep 17 00:00:00 2001 From: Jesse Zoldak Date: Wed, 15 Oct 2014 16:53:40 -0400 Subject: [PATCH] Fix sychronization issues in LTI test Better verification for troubleshooting LTI test failures Dismiss any lingering alerts after lettuce scenarios --- common/djangoapps/terrain/setup_prereqs.py | 23 +++++++- lms/djangoapps/courseware/features/lti.py | 63 +++++++++++++++++----- 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/common/djangoapps/terrain/setup_prereqs.py b/common/djangoapps/terrain/setup_prereqs.py index 9550b25f33..c39b1cb9a7 100644 --- a/common/djangoapps/terrain/setup_prereqs.py +++ b/common/djangoapps/terrain/setup_prereqs.py @@ -11,6 +11,7 @@ from terrain.stubs.youtube import StubYouTubeService from terrain.stubs.xqueue import StubXQueueService from terrain.stubs.lti import StubLtiService from terrain.stubs.video_source import VideoSourceHttpService +from selenium.common.exceptions import NoAlertPresentException import re import requests @@ -56,7 +57,7 @@ def stop_video_server(_total): video_server.shutdown() -@before.each_scenario +@before.each_scenario # pylint: disable=E1101 def process_requires_tags(scenario): """ Process the scenario tags to make sure that any @@ -124,7 +125,7 @@ def is_youtube_available(urls): return True -@after.each_scenario +@after.each_scenario # pylint: disable=E1101 def stop_stubs(_scenario): """ Shut down any stub services that were started up for the scenario. @@ -133,3 +134,21 @@ def stop_stubs(_scenario): stub_server = getattr(world, name, None) if stub_server is not None: stub_server.shutdown() + + +@after.each_scenario # pylint: disable=E1101 +def clear_alerts(_scenario): + """ + Clear any alerts that might still exist, so that + the next scenario will not fail due to their existence. + + Note that the splinter documentation indicates that + get_alert should return None if no alert is present, + however that is not the case. Instead a + NoAlertPresentException is raised. + """ + try: + with world.browser.get_alert() as alert: + alert.dismiss() + except NoAlertPresentException: + pass diff --git a/lms/djangoapps/courseware/features/lti.py b/lms/djangoapps/courseware/features/lti.py index e6b5c745b0..d64f734bc9 100644 --- a/lms/djangoapps/courseware/features/lti.py +++ b/lms/djangoapps/courseware/features/lti.py @@ -6,7 +6,8 @@ from django.conf import settings from mock import patch from pytz import UTC from splinter.exceptions import ElementDoesNotExist -from nose.tools import assert_true, assert_equal, assert_in +from selenium.common.exceptions import NoAlertPresentException +from nose.tools import assert_true, assert_equal, assert_in, assert_is_none from lettuce import world, step from courseware.tests.factories import InstructorFactory, BetaTesterFactory @@ -72,13 +73,39 @@ def view_lti_permission_alert(_step): assert len(world.browser.windows) == 1 +def check_no_alert(): + """ + Make sure the alert has gone away. + + Note that the splinter documentation indicates that + get_alert should return None if no alert is present, + however that is not the case. Instead a + NoAlertPresentException is raised. + """ + try: + assert_is_none(world.browser.get_alert()) + except NoAlertPresentException: + pass + + @step('I accept the permission alert and view the LTI$') def accept_lti_permission_alert(_step): parent_window = world.browser.current_window # Save the parent window + + # To start with you should only have one window/tab assert len(world.browser.windows) == 1 alert = world.browser.get_alert() alert.accept() - assert len(world.browser.windows) != 1 + check_no_alert() + + # Give it a few seconds for the LTI window to appear + world.wait_for( + lambda _: len(world.browser.windows) == 2, + timeout=5, + timeout_msg="Timed out waiting for the LTI window to appear." + ) + + # Verify the LTI window check_lti_popup(parent_window) @@ -86,6 +113,7 @@ def accept_lti_permission_alert(_step): def reject_lti_permission_alert(_step): alert = world.browser.get_alert() alert.dismiss() + check_no_alert() assert len(world.browser.windows) == 1 @@ -234,20 +262,29 @@ def i_am_registered_for_the_course(coursenum, metadata, user='Instructor'): def check_lti_popup(parent_window): - assert len(world.browser.windows) != 1 + # You should now have 2 browser windows open, the original courseware and the LTI + windows = world.browser.windows + assert_equal(len(windows), 2) - for window in world.browser.windows: - world.browser.switch_to_window(window) # Switch to a different window (the pop-up) - # Check if this is the one we want by comparing the url - url = world.browser.url - basename = os.path.basename(url) - pathname = os.path.splitext(basename)[0] - if pathname == u'correct_lti_endpoint': - break + # For verification, iterate through the window titles and make sure that + # both are there. + tabs = [] + for window in windows: + world.browser.switch_to_window(window) + tabs.append(world.browser.title) + assert_equal(tabs, [u'LTI | Test Section | test_course Courseware | edX', u'TEST TITLE']) + + # Now verify the contents of the LTI window (which is the 2nd window/tab) + # Note: The LTI opens in a new browser window, but Selenium sticks with the + # current window until you explicitly switch to the context of the new one. + world.browser.switch_to_window(windows[1]) + url = world.browser.url + basename = os.path.basename(url) + pathname = os.path.splitext(basename)[0] + assert_equal(pathname, u'correct_lti_endpoint') result = world.css_find('.result').first.text - - assert result == u'This is LTI tool. Success.' + assert_equal(result, u'This is LTI tool. Success.') world.browser.driver.close() # Close the pop-up window world.browser.switch_to_window(parent_window) # Switch to the main window again