From 31e2721aabf1eaec6f91c71e4f78ee7543da8f7f Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 07:16:52 -0400 Subject: [PATCH 1/8] Selenium - handle potentially stale elements when fetching tags. https://jenkins.galaxyproject.org/job/selenium/529/artifact/529-test-errors/test_tagging2017092204301506069010/stacktrace.txt --- test/galaxy_selenium/navigates_galaxy.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index 7bdcc96d987..5a87c5732c8 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -516,6 +516,7 @@ class NavigatesGalaxy(HasDriver): tag_display = workflow_row_element.find_element_by_css_selector(".tags-display") tag_display.click() + @retry_during_transitions def workflow_index_tags(self, workflow_index=0): workflow_row_element = self.workflow_index_table_row(workflow_index) tag_display = workflow_row_element.find_element_by_css_selector(".tags-display") From 9e6e340adc99884a88dc50a2d6d35cdbd3f74fc6 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 07:18:02 -0400 Subject: [PATCH 2/8] Selenium - handle potentially stale search box on workflow index page. https://jenkins.galaxyproject.org/job/selenium/530/artifact/530-test-errors/test_index_search2017092205301506072611/stacktrace.txt --- .../test_workflow_management.py | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/test/selenium_tests/test_workflow_management.py b/test/selenium_tests/test_workflow_management.py index 776c4ed9ee7..3d76b0f1c9c 100644 --- a/test/selenium_tests/test_workflow_management.py +++ b/test/selenium_tests/test_workflow_management.py @@ -1,5 +1,6 @@ from .framework import ( retry_assertion_during_transitions, + retry_during_transitions, selenium_test, SeleniumTestCase, ) @@ -73,20 +74,13 @@ class WorkflowManagementTestCase(SeleniumTestCase): self.workflow_index_rename("searchforthis") self._assert_showing_n_workflows(1) - search_box = self.workflow_index_click_search() - search_box.send_keys("doesnotmatch") + self._click_and_search("doesnotmatch") self._assert_showing_n_workflows(0) - # Prevent stale element textbox by re-fetching, seems to be - # needed but I don't understand why exactly. -John - search_box = self.workflow_index_click_search() - search_box.clear() - self.send_enter(search_box) + self._click_and_search() self._assert_showing_n_workflows(1) - search_box = self.workflow_index_click_search() - search_box.send_keys("searchforthis") - self.send_enter(search_box) + self._click_and_search("searchforthis") self._assert_showing_n_workflows(1) @selenium_test @@ -109,6 +103,16 @@ class WorkflowManagementTestCase(SeleniumTestCase): self.workflow_index_open() assert_published_column_text_is("Yes") + @retry_during_transitions + def _click_and_search(self, search_term=None): + # Allow default search_term of None to just clear search + search_box = self.workflow_index_click_search() + search_box.clear() + if search_term is not None: + search_box.send_keys(search_term) + self.send_enter(search_box) + return search_box + @retry_assertion_during_transitions def _assert_showing_n_workflows(self, n): self.assertEqual(len(self.workflow_index_table_elements()), n) From 3ffdfc67e75a608c05b35086233d40d7c09440cd Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 07:58:18 -0400 Subject: [PATCH 3/8] Selenium - catch popups fading out as transitions to retry various actions on. Previously we only detected transitions for retrying actions based on stale element exceptions, this expands that to include popups that may be fading out for instance. I think this is what is happening with the transiently failing test here https://jenkins.galaxyproject.org/job/selenium/528/artifact/528-test-errors/test_save_as2017092203291506065342/stacktrace.txt. The exception indicates that a click was not clickable because a modal element that was fading out - though it had been previously clickable. I think this should fix that. --- test/galaxy_selenium/has_driver.py | 12 ++++++++++ test/galaxy_selenium/navigates_galaxy.py | 28 +++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/test/galaxy_selenium/has_driver.py b/test/galaxy_selenium/has_driver.py index 021de2eac0a..4dca4fbe5d3 100644 --- a/test/galaxy_selenium/has_driver.py +++ b/test/galaxy_selenium/has_driver.py @@ -140,5 +140,17 @@ class HasDriver: ) +def execption_indicates_not_clickable(exception): + return "not clickable" in str(exception) + + def exception_indicates_stale_element(exception): return "stale" in str(exception) + + +__all__ = ( + "execption_indicates_not_clickable", + "exception_indicates_stale_element", + "HasDriver", + "TimeoutException", +) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index 5a87c5732c8..ed3773cb87c 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -15,7 +15,12 @@ import requests import yaml from .data import NAVIGATION_DATA -from .has_driver import exception_indicates_stale_element, HasDriver, TimeoutException +from .has_driver import ( + execption_indicates_not_clickable, + exception_indicates_stale_element, + HasDriver, + TimeoutException, +) from . import sizzle # Test case data @@ -28,7 +33,24 @@ class NullTourCallback(object): pass -def retry_call_during_transitions(f, attempts=5, sleep=.1, exception_check=exception_indicates_stale_element): +def excepion_seems_to_indicate_transition(e): + """True if exception seems to indicate the page state is transitioning. + + Galaxy features many different transition effects that change the page state over time. + These transitions make it slightly more difficult to test Galaxy because atomic input + actions take an indeterminate amount of time to be reflected on the screen. This method + takes a Selenium assertion and tries to infer if such a transition could be the root + cause of the exception. The methods that follow use it to allow retrying actions during + transitions. + + Currently the two kinds of exceptions that we say may indicate a transition are + StaleElement exceptions (a DOM element grabbed at one step is no longer available) + and "not clickable" exceptions (so perhaps a popup modal is blocking a click). + """ + return exception_indicates_stale_element(e) or execption_indicates_not_clickable(e) + + +def retry_call_during_transitions(f, attempts=5, sleep=.1, exception_check=excepion_seems_to_indicate_transition): previous_attempts = 0 while True: try: @@ -44,7 +66,7 @@ def retry_call_during_transitions(f, attempts=5, sleep=.1, exception_check=excep previous_attempts += 1 -def retry_during_transitions(f, attempts=5, sleep=.1, exception_check=exception_indicates_stale_element): +def retry_during_transitions(f, attempts=5, sleep=.1, exception_check=excepion_seems_to_indicate_transition): @wraps(f) def _retry(*args, **kwds): From 77b61ba82d3eec55c5acd2ae4ad3ec9fde0cc9db Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 08:16:53 -0400 Subject: [PATCH 4/8] Selenium - attempted fix for transiently failing history options test. Comments inline about this - but the target failure I'm hoping this fixes is: https://jenkins.galaxyproject.org/job/selenium/528/artifact/528-test-errors/test_options2017092203081506064101/stacktrace.txt --- test/galaxy_selenium/navigates_galaxy.py | 12 ++++++++++++ test/selenium_tests/test_history_options.py | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index ed3773cb87c..e500422446a 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -819,6 +819,18 @@ class NavigatesGalaxy(HasDriver): self.click_center() return text + @retry_during_transitions + def assert_selector_absent_or_hidden_after_transitions(self, selector): + """Variant of assert_selector_absent_or_hidden that retries during transitions. + + In the parent method - the element is found and then it is checked to see + if it is visible. It may disappear from the page in the middle there + and cause a StaleElement error. For checks where we care about the final + resting state after transitions - this method can be used to retry + during those transitions. + """ + return self.assert_selector_absent_or_hidden(selector) + def assert_tooltip_text(self, element, expected, sleep=0, click_away=True): text = self.get_tooltip_text(element, sleep=sleep, click_away=click_away) assert text == expected, "Tooltip text [%s] was not expected text [%s]." % (text, expected) diff --git a/test/selenium_tests/test_history_options.py b/test/selenium_tests/test_history_options.py index 8e8f975801c..f701f65d357 100644 --- a/test/selenium_tests/test_history_options.py +++ b/test/selenium_tests/test_history_options.py @@ -27,4 +27,4 @@ class HistoryOptionsTestCase(SeleniumTestCase): self.click_hda_title(hda_id, wait=True) - self.assert_selector_absent_or_hidden(hda_body_selector) + self.assert_selector_absent_or_hidden_after_transitions(hda_body_selector) From 17348a62fe92716ed80cac6f0925e5d5c3203deb Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 08:41:25 -0400 Subject: [PATCH 5/8] Selenium - more robust inline searching for published histories grid. Refactor recent search improvement for workflow management into a method that can be shared with published histories grid. Should address the transient failure test here: https://jenkins.galaxyproject.org/job/selenium/521/artifact/521-test-errors/test_history_grid_search_standard2017092120151506039303/stacktrace.txt --- test/galaxy_selenium/navigates_galaxy.py | 21 +++++++++++++++++++ .../test_published_histories_grid.py | 10 ++------- .../test_workflow_management.py | 16 +++----------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index e500422446a..ecb594869ee 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -223,12 +223,27 @@ class NavigatesGalaxy(HasDriver): raise self.prepend_timeout_message(e, message) return history_item_selector_state + def published_grid_search_for(self, search_term=None): + return self._inline_search_for( + '#input-free-text-search-filter', + search_term, + ) + def get_logged_in_user(self): return self.api_get("users/current") def is_logged_in(self): return "email" in self.get_logged_in_user() + @retry_during_transitions + def _inline_search_for(self, selector, search_term=None): + search_box = self.wait_for_and_click_selector(selector) + search_box.clear() + if search_term is not None: + search_box.send_keys(search_term) + self.send_enter(search_box) + return search_box + def _get_random_name(self, prefix=None, suffix=None, len=10): return '%s%s%s' % ( prefix or '', @@ -502,6 +517,12 @@ class NavigatesGalaxy(HasDriver): def workflow_index_click_search(self): return self.wait_for_and_click_selector("input.search-wf") + def workflow_index_search_for(self, search_term=None): + return self._inline_search_for( + "input.search-wf", + search_term, + ) + def workflow_index_click_import(self): self.wait_for_and_click_selector(self.test_data["selectors"]["workflows"]["import_button"]) diff --git a/test/selenium_tests/test_published_histories_grid.py b/test/selenium_tests/test_published_histories_grid.py index 9937a19e602..d1c0aa8df5e 100644 --- a/test/selenium_tests/test_published_histories_grid.py +++ b/test/selenium_tests/test_published_histories_grid.py @@ -22,17 +22,11 @@ class HistoryGridTestCase(SharedStateSeleniumTestCase): def test_history_grid_search_standard(self): self.navigate_to_published_histories_page() - input_selector = '#input-free-text-search-filter' - search_input = self.wait_for_selector(input_selector) - search_input.send_keys(self.history1_name) - self.send_enter(search_input) - + self.published_grid_search_for(self.history1_name) self.assert_grid_histories_are([self.history1_name]) self.unset_filter('free-text-search', self.history1_name) - search_input = self.wait_for_selector(input_selector) - search_input.send_keys(self.history4_name) - self.send_enter(search_input) + self.published_grid_search_for(self.history4_name) self.assert_grid_histories_are(['No Items']) diff --git a/test/selenium_tests/test_workflow_management.py b/test/selenium_tests/test_workflow_management.py index 3d76b0f1c9c..beea3684851 100644 --- a/test/selenium_tests/test_workflow_management.py +++ b/test/selenium_tests/test_workflow_management.py @@ -74,13 +74,13 @@ class WorkflowManagementTestCase(SeleniumTestCase): self.workflow_index_rename("searchforthis") self._assert_showing_n_workflows(1) - self._click_and_search("doesnotmatch") + self.workflow_index_search_for("doesnotmatch") self._assert_showing_n_workflows(0) - self._click_and_search() + self.workflow_index_search_for() self._assert_showing_n_workflows(1) - self._click_and_search("searchforthis") + self.workflow_index_search_for("searchforthis") self._assert_showing_n_workflows(1) @selenium_test @@ -103,16 +103,6 @@ class WorkflowManagementTestCase(SeleniumTestCase): self.workflow_index_open() assert_published_column_text_is("Yes") - @retry_during_transitions - def _click_and_search(self, search_term=None): - # Allow default search_term of None to just clear search - search_box = self.workflow_index_click_search() - search_box.clear() - if search_term is not None: - search_box.send_keys(search_term) - self.send_enter(search_box) - return search_box - @retry_assertion_during_transitions def _assert_showing_n_workflows(self, n): self.assertEqual(len(self.workflow_index_table_elements()), n) From cf037fcc0c0e1f8954361812ec36d14515c8f3ab Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 08:53:24 -0400 Subject: [PATCH 6/8] Selenium - more robust modal checking in test_workflow_editor. During this transient failure: https://jenkins.galaxyproject.org/job/selenium/519/artifact/519-test-errors/test_missing_tools2017092118271506032874/stacktrace.txt the workflow was still loading when an assertion about the resulting modal text was issued. Avoiding the arbitrary sleep and instead just retrying the assertion until it holds true or times out is more robust and should prevent that. --- test/galaxy_selenium/navigates_galaxy.py | 7 ++++-- test/selenium_tests/test_workflow_editor.py | 24 ++++++++++----------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/test/galaxy_selenium/navigates_galaxy.py b/test/galaxy_selenium/navigates_galaxy.py index ecb594869ee..cff486af9a7 100644 --- a/test/galaxy_selenium/navigates_galaxy.py +++ b/test/galaxy_selenium/navigates_galaxy.py @@ -26,6 +26,9 @@ from . import sizzle # Test case data DEFAULT_PASSWORD = '123456' +RETRY_DURING_TRANSITIONS_SLEEP_DEFAULT = .1 +RETRY_DURING_TRANSITIONS_ATTEMPTS_DEFAULT = 10 + class NullTourCallback(object): @@ -50,7 +53,7 @@ def excepion_seems_to_indicate_transition(e): return exception_indicates_stale_element(e) or execption_indicates_not_clickable(e) -def retry_call_during_transitions(f, attempts=5, sleep=.1, exception_check=excepion_seems_to_indicate_transition): +def retry_call_during_transitions(f, attempts=RETRY_DURING_TRANSITIONS_ATTEMPTS_DEFAULT, sleep=RETRY_DURING_TRANSITIONS_SLEEP_DEFAULT, exception_check=excepion_seems_to_indicate_transition): previous_attempts = 0 while True: try: @@ -66,7 +69,7 @@ def retry_call_during_transitions(f, attempts=5, sleep=.1, exception_check=excep previous_attempts += 1 -def retry_during_transitions(f, attempts=5, sleep=.1, exception_check=excepion_seems_to_indicate_transition): +def retry_during_transitions(f, attempts=RETRY_DURING_TRANSITIONS_ATTEMPTS_DEFAULT, sleep=RETRY_DURING_TRANSITIONS_SLEEP_DEFAULT, exception_check=excepion_seems_to_indicate_transition): @wraps(f) def _retry(*args, **kwds): diff --git a/test/selenium_tests/test_workflow_editor.py b/test/selenium_tests/test_workflow_editor.py index 6dd7044b500..458cb3c032a 100644 --- a/test/selenium_tests/test_workflow_editor.py +++ b/test/selenium_tests/test_workflow_editor.py @@ -1,6 +1,7 @@ import time from .framework import ( + retry_assertion_during_transitions, selenium_test, SeleniumTestCase ) @@ -52,10 +53,7 @@ class WorkflowEditorTestCase(SeleniumTestCase): workflow_populator.upload_yaml_workflow(WORKFLOW_WITH_OLD_TOOL_VERSION, exact_tools=True) self.workflow_index_open() self.workflow_index_click_option("Edit") - time.sleep(.5) - modal_element = self.wait_for_selector_visible(self.modal_body_selector()) - text = modal_element.text - assert "Using version '0.2' instead of version '0.0.1'" in text, text + self.assert_modal_has_text("Using version '0.2' instead of version '0.0.1'") @selenium_test def test_editor_invalid_tool_state(self): @@ -63,11 +61,8 @@ class WorkflowEditorTestCase(SeleniumTestCase): workflow_populator.upload_yaml_workflow(WORKFLOW_WITH_INVALID_STATE, exact_tools=True) self.workflow_index_open() self.workflow_index_click_option("Edit") - time.sleep(.5) - modal_element = self.wait_for_selector_visible(self.modal_body_selector()) - text = modal_element.text - assert "Using version '0.2' instead of version '0.0.1'" in text, text - assert "Using default: '1'" in text, text + self.assert_modal_has_text("Using version '0.2' instead of version '0.0.1'") + self.assert_modal_has_text("Using default: '1'") @selenium_test def test_missing_tools(self): @@ -84,10 +79,7 @@ steps: """) self.workflow_index_open() self.workflow_index_click_option("Edit") - time.sleep(.5) - modal_element = self.wait_for_selector_visible(self.modal_body_selector()) - text = modal_element.text - assert "Tool is not installed" in text, text + self.assert_modal_has_text("Tool is not installed") def workflow_create_new(self, name=None, annotation=None): self.workflow_index_open() @@ -104,3 +96,9 @@ steps: 'workflow_annotation': annotation, }) self.click_submit(form_element) + + @retry_assertion_during_transitions + def assert_modal_has_text(self, expected_text): + modal_element = self.wait_for_selector_visible(self.modal_body_selector()) + text = modal_element.text + assert expected_text in text, "Failed to find expected text [%s] in modal text [%s]" % (expected_text, text) From 341667437ae718258a8d543c4e09d187f7f85326 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 22 Sep 2017 09:22:09 -0400 Subject: [PATCH 7/8] Fix selenium test retrying of tests in test_saved_histories. The automatic retrying of tests only runs for methods - not for the full test case (if that makes sense) - so setUp() isn't rerun. By refactoring the login action out of the setUp method and into the test function for these cases - when the test is retried the user should be logged back in again as well. --- .../test_published_histories_grid.py | 4 ---- test/selenium_tests/test_saved_histories.py | 21 ++++++++++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/test/selenium_tests/test_published_histories_grid.py b/test/selenium_tests/test_published_histories_grid.py index d1c0aa8df5e..7cf86e06884 100644 --- a/test/selenium_tests/test_published_histories_grid.py +++ b/test/selenium_tests/test_published_histories_grid.py @@ -9,10 +9,6 @@ from .framework import ( class HistoryGridTestCase(SharedStateSeleniumTestCase): - def setUp(self): - super(HistoryGridTestCase, self).setUp() - self.home() - @selenium_test def test_history_grid_histories(self): self.navigate_to_published_histories_page() diff --git a/test/selenium_tests/test_saved_histories.py b/test/selenium_tests/test_saved_histories.py index 422b7c8d713..8bd65ec0348 100644 --- a/test/selenium_tests/test_saved_histories.py +++ b/test/selenium_tests/test_saved_histories.py @@ -9,18 +9,15 @@ from .framework import ( class SavedHistoriesTestCase(SharedStateSeleniumTestCase): - def setUp(self): - super(SavedHistoriesTestCase, self).setUp() - self.home() - self.submit_login(self.user_email, retries=3) - @selenium_test def test_saved_histories_list(self): + self._login() self.navigate_to_saved_histories_page() self.assert_histories_in_grid([self.history2_name, self.history3_name]) @selenium_test def test_history_switch(self): + self._login() self.navigate_to_saved_histories_page() self.click_popup_option(self.history2_name, 'Switch') time.sleep(1) @@ -29,6 +26,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_history_view(self): + self._login() self.navigate_to_saved_histories_page() self.click_popup_option(self.history2_name, 'View') history_name = self.wait_for_selector('.name.editable-text') @@ -36,6 +34,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_history_publish(self): + self._login() self.navigate_to_saved_histories_page() # Publish the history @@ -52,6 +51,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_rename_history(self): + self._login() self.navigate_to_saved_histories_page() self.click_popup_option('Unnamed history', 'Rename') @@ -72,6 +72,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_delete_and_undelete_history(self): + self._login() self.navigate_to_saved_histories_page() # Delete the history @@ -93,6 +94,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_permanently_delete_history(self): + self._login() self.create_history(self.history4_name) self.navigate_to_saved_histories_page() @@ -111,6 +113,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_delete_and_undelete_multiple_histories(self): + self._login() self.navigate_to_saved_histories_page() delete_button_selector = 'input[type="button"][value="Delete"]' @@ -139,6 +142,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_sort_by_name(self): + self._login() self.navigate_to_saved_histories_page() self.wait_for_and_click_selector('.sort-link[sort_key="name"]') @@ -156,6 +160,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_standard_search(self): + self._login() self.navigate_to_saved_histories_page() input_selector = '#input-free-text-search-filter' @@ -174,6 +179,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_advanced_search(self): + self._login() self.navigate_to_saved_histories_page() self.show_advanced_search() @@ -201,6 +207,7 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): @selenium_test def test_tags(self): + self._login() self.navigate_to_saved_histories_page() # Click the add tag button @@ -221,6 +228,10 @@ class SavedHistoriesTestCase(SharedStateSeleniumTestCase): self.assert_grid_histories_are([self.history2_name], False) + def _login(self): + self.home() + self.submit_login(self.user_email, retries=3) + @retry_assertion_during_transitions def assert_grid_histories_are(self, expected_histories, sort_matters=True): actual_histories = self.get_histories() From 3014b1068bc4b7dc42d3f4c0754f6eed881b453f Mon Sep 17 00:00:00 2001 From: Martin Cech Date: Fri, 22 Sep 2017 14:44:59 -0400 Subject: [PATCH 8/8] remove unused import --- test/selenium_tests/test_workflow_management.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/selenium_tests/test_workflow_management.py b/test/selenium_tests/test_workflow_management.py index beea3684851..760d1c85108 100644 --- a/test/selenium_tests/test_workflow_management.py +++ b/test/selenium_tests/test_workflow_management.py @@ -1,6 +1,5 @@ from .framework import ( retry_assertion_during_transitions, - retry_during_transitions, selenium_test, SeleniumTestCase, )