From 01eb5f04e6661d389d8c8f681373b5c50fe259b9 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 20 Sep 2017 10:17:25 -0400 Subject: [PATCH] Aggressively retry buggy submit_login() function in Selenium tests. I don't get why when we click submit the user does not actually get logged in, but based on the last round of improved error messages this seems to be the case. You might think there is some callback in the login form that doesn't get registered by the time Selenium clicks the submit button - but this doesn't seem to be the case - I don't see any jquery magic happening in login.mako. Should fix failures like this: https://jenkins.galaxyproject.org/job/selenium/482/testReport/junit/selenium_tests.test_saved_histories/SavedHistoriesTestCase/test_history_publish/ I'd say at this point this is the most common problem in the Selenium tests. This also introduces a framework for taking state snapshots of the Galaxy interface during tests that will only get written out if the tests fail. We now take screenshots before and after submitting the login form but this is a general purpose debugging mechanism that could be used other places. --- test/galaxy_selenium/navigates_galaxy.py | 36 +++++++++++++++++---- test/selenium_tests/framework.py | 28 ++++++++++++++-- test/selenium_tests/test_custom_builds.py | 2 +- test/selenium_tests/test_history_sharing.py | 13 +++++--- test/selenium_tests/test_saved_histories.py | 2 +- 5 files changed, 66 insertions(+), 15 deletions(-) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index 53a3a1f1938..a3b54999bcb 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -15,7 +15,7 @@ import requests import yaml from .data import NAVIGATION_DATA -from .has_driver import exception_indicates_stale_element, HasDriver +from .has_driver import exception_indicates_stale_element, HasDriver, TimeoutException from . import sizzle # Test case data @@ -219,7 +219,7 @@ class NavigatesGalaxy(HasDriver): domain = domain or 'test.test' return self._get_random_name(prefix=username, suffix="@" + domain) - def submit_login(self, email, password=None, assert_valid=True): + def submit_login(self, email, password=None, assert_valid=True, retries=0): if password is None: password = self.default_password @@ -234,10 +234,20 @@ class NavigatesGalaxy(HasDriver): with self.main_panel(): form = self.wait_for_selector(self.navigation_data["selectors"]["loginPage"]["form"]) self.fill(form, login_info) + self.snapshot("logging-in") self.click_submit(form) + self.snapshot("login-submitted") if assert_valid: - self.wait_for_logged_in() + try: + self.wait_for_logged_in() + except NotLoggedInException: + self.snapshot("login-failed") + if retries > 0: + self.submit_login(email, password, assert_valid, retries - 1) + else: + raise + self.snapshot("logged-in") def register(self, email=None, password=None, username=None, confirm=None, assert_valid=True): if email is None: @@ -302,10 +312,9 @@ class NavigatesGalaxy(HasDriver): if "username" in user_info: template = "Failed waiting for masthead to update for login, but user API response indicates [%s] is logged in. This seems to be a bug in Galaxy. API response was [%s]. " message = template % (user_info["username"], user_info) + raise self.prepend_timeout_message(e, message) else: - template = "Failed waiting for masthead to update for login, API indicates no user is logged in - there is a problem with this test. API response was [%s]. " - message = template % user_info - raise self.prepend_timeout_message(e, message) + raise NotLoggedInException(e, user_info) def click_center(self): action_chains = self.action_chains() @@ -851,3 +860,18 @@ class NavigatesGalaxy(HasDriver): action_chains = self.action_chains() action_chains.move_to_element(select_elem).click().perform() self.wait_for_selector_absent_or_hidden("#select2-drop") + + def snapshot(self, description): + """Test case subclass overrides this to provide detailed logging.""" + + +class NotLoggedInException(TimeoutException): + + def __init__(self, timeout_exception, user_info): + template = "Waiting for UI to reflect user logged in but it did not occur. API indicates no user is currently logged in. API response was [%s]. %s" + msg = template % (user_info, timeout_exception.msg) + super(NotLoggedInException, self).__init__( + msg=msg, + screen=timeout_exception.screen, + stacktrace=timeout_exception.stacktrace + ) diff --git a/test/selenium_tests/framework.py b/test/selenium_tests/framework.py index 3576354d9b4..f775ade1679 100644 --- a/test/selenium_tests/framework.py +++ b/test/selenium_tests/framework.py @@ -83,15 +83,19 @@ def selenium_test(f): result_name = f.__name__ + datetime.datetime.now().strftime("%Y%m%d%H%M%s") target_directory = os.path.join(GALAXY_TEST_ERRORS_DIRECTORY, result_name) - def write_file(name, content): + def write_file(name, content, raw=False): with open(os.path.join(target_directory, name), "wb") as buf: - buf.write(content.encode("utf-8")) + buf.write(content.encode("utf-8") if not raw else content) os.makedirs(target_directory) self.driver.save_screenshot(os.path.join(target_directory, "last.png")) write_file("page_source.txt", self.driver.page_source) write_file("DOM.txt", self.driver.execute_script("return document.documentElement.outerHTML")) write_file("stacktrace.txt", traceback.format_exc()) + + for snapshot in getattr(self, "snapshots", []): + snapshot.write_to_error_directory(write_file) + for log_type in ["browser", "driver"]: try: write_file("%s.log.json" % log_type, json.dumps(self.driver.get_log(log_type))) @@ -116,6 +120,22 @@ def selenium_test(f): retry_assertion_during_transitions = partial(retry_during_transitions, exception_check=lambda e: isinstance(e, AssertionError)) +class TestSnapshot(object): + + def __init__(self, driver, index, description): + self.screenshot_binary = driver.get_screenshot_as_png() + self.description = description + self.index = index + self.exc = traceback.format_exc() + self.stack = traceback.format_stack() + + def write_to_error_directory(self, write_file_func): + prefix = "%d-%s" % (self.index, self.description) + write_file_func("%s-screenshot.png" % prefix, self.screenshot_binary, raw=True) + write_file_func("%s-traceback.txt" % prefix, self.exc) + write_file_func("%s-stack.txt" % prefix, str(self.stack)) + + class SeleniumTestCase(FunctionalTestCase, NavigatesGalaxy): framework_tool_and_types = True @@ -130,6 +150,7 @@ class SeleniumTestCase(FunctionalTestCase, NavigatesGalaxy): else: self.target_url_from_selenium = self.url self.setup_driver_and_session() + self.snapshots = [] def tearDown(self): exception = None @@ -146,6 +167,9 @@ class SeleniumTestCase(FunctionalTestCase, NavigatesGalaxy): if exception is not None: raise exception + def snapshot(self, description): + self.snapshots.append(TestSnapshot(self.driver, len(self.snapshots), description)) + def reset_driver_and_session(self): self.tear_down_driver() self.setup_driver_and_session() diff --git a/test/selenium_tests/test_custom_builds.py b/test/selenium_tests/test_custom_builds.py index 55946b91e06..f29d04e189b 100644 --- a/test/selenium_tests/test_custom_builds.py +++ b/test/selenium_tests/test_custom_builds.py @@ -12,7 +12,7 @@ class CustomBuildsTestcase(SharedStateSeleniumTestCase): def setUp(self): super(CustomBuildsTestcase, self).setUp() self.home() # ensure Galaxy is loaded - self.submit_login(self.user_email) + self.submit_login(self.user_email, retries=2) @selenium_test def test_build_add(self): diff --git a/test/selenium_tests/test_history_sharing.py b/test/selenium_tests/test_history_sharing.py index c21b8bf2e45..dc77592b02f 100644 --- a/test/selenium_tests/test_history_sharing.py +++ b/test/selenium_tests/test_history_sharing.py @@ -1,27 +1,30 @@ from .framework import SeleniumTestCase from .framework import selenium_test +# Remove hack when submit_login works more consistently. +VALID_LOGIN_RETRIES = 3 + class HistorySharingTestCase(SeleniumTestCase): @selenium_test def test_sharing_valid(self): user1_email, user2_email, history_id = self.setup_two_users_with_one_shared_history() - self.submit_login(user2_email) + self.submit_login(user2_email, retries=VALID_LOGIN_RETRIES) response = self.api_get("histories/%s" % history_id, raw=True) assert response.status_code == 200, response.json() @selenium_test def test_sharing_valid_by_id(self): user1_email, user2_email, history_id = self.setup_two_users_with_one_shared_history(share_by_id=True) - self.submit_login(user2_email) + self.submit_login(user2_email, retries=VALID_LOGIN_RETRIES) response = self.api_get("histories/%s" % history_id, raw=True) assert response.status_code == 200, response.json() @selenium_test def test_unsharing(self): user1_email, user2_email, history_id = self.setup_two_users_with_one_shared_history() - self.submit_login(user1_email) + self.submit_login(user1_email, retries=VALID_LOGIN_RETRIES) self.navigate_to_history_share_page() with self.main_panel(): @@ -36,7 +39,7 @@ class HistorySharingTestCase(SeleniumTestCase): self.assert_selector_absent("#user-0-popup") self.logout_if_needed() - self.submit_login(user2_email) + self.submit_login(user2_email, retries=VALID_LOGIN_RETRIES) response = self.api_get("histories/%s" % history_id, raw=True) assert response.status_code == 403 @@ -80,7 +83,7 @@ class HistorySharingTestCase(SeleniumTestCase): user2_id = self.api_get("users")[0]["id"] self.logout_if_needed() - self.submit_login(user1_email) + self.submit_login(user1_email, retries=VALID_LOGIN_RETRIES) # Can't share an empty history... self.perform_upload(self.get_filename("1.txt")) self.wait_for_history() diff --git a/test/selenium_tests/test_saved_histories.py b/test/selenium_tests/test_saved_histories.py index 83504246cd7..a721b5a87c5 100644 --- a/test/selenium_tests/test_saved_histories.py +++ b/test/selenium_tests/test_saved_histories.py @@ -12,7 +12,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): def setUp(self): super(SavedHistoriesTestCase, self).setUp() self.home() - self.submit_login(self.user_email) + self.submit_login(self.user_email, retries=3) @selenium_test def test_saved_histories_list(self):