From 87bb576c39dadadbf119167dee581a6801a54cea Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 19 Sep 2017 10:21:37 -0400 Subject: [PATCH] Improved state handling for @anatskiy-style Selenium tests. These tests aren't idealized unittest.TestCase because they initialize class level data in instance level methods. While this isn't ideal, it is seems an entirely fair workaround given that SeleniumTestCase setups up the Selenium connection itself in an instance method - so class-level initializers would not be able to setup Galaxy data. Since I think we will continue using this pattern then, probably best to formalize it a bit and improve error handling. This provides a formal super class for these test cases that provides a uniform method for setting up the class level data and tracks whether this is successful or not. This serves a couple purposes beyond simple uniformity. First, it tracks if the state has actually been setup or not and will skip subsequent tests if it hasn't. Some of these tests aren't passing very consistently on Jenkins and so we get a bunch of extra noise for tests that are attempting to run without their preconditions met - this will fix that and make the original errors much more clear. Moving the "hacky" part of this into the framework itself also means the tests themselves don't have to repeat hacks like seeing if variables are set with ``getattr`` and such - I always prefer one framework hack to a dozen application hacks. --- test/selenium_tests/framework.py | 34 ++++++++++++++++++- test/selenium_tests/test_custom_builds.py | 14 +++----- .../test_published_histories_grid.py | 12 +++---- test/selenium_tests/test_saved_histories.py | 14 +++----- 4 files changed, 47 insertions(+), 27 deletions(-) diff --git a/test/selenium_tests/framework.py b/test/selenium_tests/framework.py index 1374e34af8f4..3576354d9b42 100644 --- a/test/selenium_tests/framework.py +++ b/test/selenium_tests/framework.py @@ -6,8 +6,8 @@ import json import os import time - import traceback +import unittest from functools import partial, wraps @@ -221,6 +221,38 @@ def workflow_populator(self): return SeleniumSessionWorkflowPopulator(self) +class SharedStateSeleniumTestCase(SeleniumTestCase): + """This describes a class Selenium tests that setup class state for all tests. + + This is a bit hacky because we are simulating class level initialization + with instance level methods. The problem is that super.setUp() works at + instance level. It might be worth considering having two variants of + SeleniumTestCase - one that initializes with the class and the other that + initializes with the instance but all the helpers are instance helpers. + """ + + shared_state_initialized = False + shared_state_in_error = False + + def setUp(self): + super(SharedStateSeleniumTestCase, self).setUp() + if not self.__class__.shared_state_initialized: + try: + self.setup_shared_state() + self.logout_if_needed() + except Exception: + self.__class__.shared_state_in_error = True + raise + finally: + self.__class__.shared_state_initialized = True + else: + if self.__class__.shared_state_in_error: + raise unittest.SkipTest("Skipping test, failed to initialize state previously.") + + def setup_shared_state(self): + """Override this to setup shared data for tests that gets initialized only once.""" + + class UsesHistoryItemAssertions: def assert_item_peek_includes(self, hid, expected): diff --git a/test/selenium_tests/test_custom_builds.py b/test/selenium_tests/test_custom_builds.py index 13c16e92e9f2..55946b91e067 100644 --- a/test/selenium_tests/test_custom_builds.py +++ b/test/selenium_tests/test_custom_builds.py @@ -3,15 +3,16 @@ from .framework import ( retry_assertion_during_transitions, selenium_test, - SeleniumTestCase, + SharedStateSeleniumTestCase, ) -class CustomBuildsTestcase(SeleniumTestCase): +class CustomBuildsTestcase(SharedStateSeleniumTestCase): def setUp(self): super(CustomBuildsTestcase, self).setUp() - self.ensure_user() + self.home() # ensure Galaxy is loaded + self.submit_login(self.user_email) @selenium_test def test_build_add(self): @@ -91,12 +92,7 @@ def navigate_to_custom_builds_page(self): self.click_label(label) self.wait_for_and_click_selector('a[href="/custom_builds"]') - def ensure_user(self): - if getattr(CustomBuildsTestcase, 'user_email', None): - self.home() # ensure Galaxy is loaded - self.submit_login(self.user_email) - return - + def setup_shared_state(self): CustomBuildsTestcase.user_email = self._get_random_email() self.register(self.user_email) diff --git a/test/selenium_tests/test_published_histories_grid.py b/test/selenium_tests/test_published_histories_grid.py index 56f7411696a1..a6b610118c15 100644 --- a/test/selenium_tests/test_published_histories_grid.py +++ b/test/selenium_tests/test_published_histories_grid.py @@ -3,15 +3,15 @@ from .framework import ( retry_assertion_during_transitions, selenium_test, - SeleniumTestCase, + SharedStateSeleniumTestCase, ) -class HistoryGridTestCase(SeleniumTestCase): +class HistoryGridTestCase(SharedStateSeleniumTestCase): def setUp(self): super(HistoryGridTestCase, self).setUp() - self.ensure_users_and_histories() + self.home() @selenium_test def test_history_grid_histories(self): @@ -178,11 +178,7 @@ def set_annotation(self, annotation): annon_area_editable.send_keys(annotation) anno_done_button.click() - def ensure_users_and_histories(self): - if getattr(HistoryGridTestCase, 'user1_email', None): - self.home() # ensure Galaxy is loaded - return - + def setup_shared_state(self): tag1 = self._get_random_name(len=5) tag2 = self._get_random_name(len=5) tag3 = self._get_random_name(len=5) diff --git a/test/selenium_tests/test_saved_histories.py b/test/selenium_tests/test_saved_histories.py index c1b7feec6247..83504246cd7f 100644 --- a/test/selenium_tests/test_saved_histories.py +++ b/test/selenium_tests/test_saved_histories.py @@ -3,15 +3,16 @@ from .framework import ( retry_assertion_during_transitions, selenium_test, - SeleniumTestCase, + SharedStateSeleniumTestCase, ) -class SavedHistoriesTestCase(SeleniumTestCase): +class SavedHistoriesTestCase(SharedStateSeleniumTestCase): def setUp(self): super(SavedHistoriesTestCase, self).setUp() - self.ensure_user_and_histories() + self.home() + self.submit_login(self.user_email) @selenium_test def test_saved_histories_list(self): @@ -263,12 +264,7 @@ def navigate_to_saved_histories_page(self): self.click_label(label) self.wait_for_and_click_selector('a[href="/histories/list"]') - def ensure_user_and_histories(self): - if getattr(SavedHistoriesTestCase, 'user_email', None): - self.home() # ensure Galaxy is loaded - self.submit_login(self.user_email) - return - + def setup_shared_state(self): SavedHistoriesTestCase.user_email = self._get_random_email() SavedHistoriesTestCase.history1_name = self._get_random_name() SavedHistoriesTestCase.history2_name = self._get_random_name()