From e99b7c942b66f90a2fa1e6cc45f006adec3ec906 Mon Sep 17 00:00:00 2001 From: Joseph Nuthalapati Date: Mon, 26 Mar 2018 18:12:15 +0530 Subject: [PATCH] tests: Improve waiting for installation and configuration - Use Selenium's built-in waiting constructs to wait for page loads to complete - Make tests independent of language (tested in Chinese) Signed-off-by: Joseph Nuthalapati Reviewed-by: James Valleroy --- functional_tests/support/application.py | 41 +++++----- functional_tests/support/interface.py | 20 +++-- functional_tests/support/service.py | 25 +++++- functional_tests/support/system.py | 2 +- .../templates/matrix-synapse-pre-setup.html | 2 +- plinth/templates/service.html | 2 +- plinth/templates/setup.html | 77 ++++++++----------- 7 files changed, 94 insertions(+), 75 deletions(-) diff --git a/functional_tests/support/application.py b/functional_tests/support/application.py index a0ea35703..cf1d7bd6a 100644 --- a/functional_tests/support/application.py +++ b/functional_tests/support/application.py @@ -18,6 +18,7 @@ from time import sleep import splinter + from support import config, interface from support.service import eventually @@ -55,20 +56,29 @@ def get_app_checkbox_id(app_name): def install(browser, app_name): interface.nav_to_module(browser, get_app_module(app_name)) - install = browser.find_by_value('Install') + install = browser.find_by_css('.form-install input[type=submit]') + + def install_in_progress(): + selectors = [ + '.install-state-' + state + for state in ['pre', 'post', 'installing'] + ] + return any( + browser.is_element_present_by_css(selector) + for selector in selectors) + if install: install.click() - while browser.is_text_present('Installing') \ - or browser.is_text_present('Performing post-install operation'): + while install_in_progress(): sleep(1) - sleep(2) + sleep(2) # XXX This shouldn't be required. def _change_status(browser, app_name, change_status_to='enabled'): interface.nav_to_module(browser, get_app_module(app_name)) checkbox = browser.find_by_id(get_app_checkbox_id(app_name)) checkbox.check() if change_status_to == 'enabled' else checkbox.uncheck() - browser.find_by_value('Update setup').click() + interface.submit(browser, 'form-configuration') if app_name == app_config_updating_text: wait_for_config_update(browser, app_name) @@ -90,7 +100,7 @@ def select_domain_name(browser, app_name, domain_name): browser.visit('{}/plinth/apps/{}/setup/'.format(default_url, app_name)) drop_down = browser.find_by_id('id_domain_name') drop_down.select(domain_name) - browser.find_by_value('Update setup').click() + interface.submit(browser, 'form-configuration') def configure_shadowsocks(browser): @@ -98,22 +108,14 @@ def configure_shadowsocks(browser): browser.visit('{}/plinth/apps/shadowsocks/'.format(default_url)) browser.find_by_id('id_server').fill('some.shadow.tunnel') browser.find_by_id('id_password').fill('fakepassword') - browser.find_by_value('Update setup').click() + interface.submit(browser, 'form-configuration') def modify_max_file_size(browser, size): """Change the maximum file size of coquelicot to the given value""" browser.visit('{}/plinth/apps/coquelicot/'.format(default_url)) browser.find_by_id('id_max_file_size').fill(size) - browser.find_by_value('Update setup').click() - # Wait for the service to restart after updating maximum file size - eventually(message_or_setting_unchanged, - args=[browser, 'Maximum file size updated']) - - -def message_or_setting_unchanged(browser, message): - return browser.is_text_present(message) or browser.is_text_present( - 'Setting unchanged') + interface.submit(browser, 'form-configuration') def get_max_file_size(browser): @@ -126,9 +128,7 @@ def modify_upload_password(browser, password): """Change the upload password for coquelicot to the given value""" browser.visit('{}/plinth/apps/coquelicot/'.format(default_url)) browser.find_by_id('id_upload_password').fill(password) - browser.find_by_value('Update setup').click() - # Wait for the service to restart after updating password - eventually(browser.is_text_present, args=['Upload password updated']) + interface.submit(browser, 'form-configuration') def remove_share(browser, name): @@ -160,7 +160,8 @@ def edit_share(browser, old_name, new_name, path, group): browser.fill('sharing-name', new_name) browser.fill('sharing-path', path) browser.find_by_css('#id_sharing-groups input').uncheck() - browser.find_by_css('#id_sharing-groups input[value="{}"]'.format(group)).check() + browser.find_by_css( + '#id_sharing-groups input[value="{}"]'.format(group)).check() browser.find_by_css('input[type="submit"]').click() eventually(browser.is_text_present, args=['Share edited.']) diff --git a/functional_tests/support/interface.py b/functional_tests/support/interface.py index 2c3505c82..0b47c75b1 100644 --- a/functional_tests/support/interface.py +++ b/functional_tests/support/interface.py @@ -14,8 +14,11 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . # + from support import config +from .service import wait_for_page_update + sys_modules = [ 'avahi', 'cockpit', 'config', 'datetime', 'diagnostics', 'firewall', 'letsencrypt', 'monkeysphere', 'names', 'networks', 'power', 'snapshot', @@ -61,8 +64,13 @@ def nav_to_module(browser, module): browser.find_link_by_href('/plinth/apps/' + module + '/').first.click() -def submit(browser): - browser.find_by_value('Submit').click() +def submit(browser, form_class=None): + with wait_for_page_update(browser): + if form_class: + browser.find_by_css( + '.{} input[type=submit]'.format(form_class)).click() + else: + browser.find_by_css('input[type=submit]').click() def create_user(browser, name, password): @@ -76,16 +84,16 @@ def create_user(browser, name, password): def rename_user(browser, old_name, new_name): nav_to_module(browser, 'users') - browser.find_link_by_href('/plinth/sys/users/' + old_name + - '/edit/').first.click() + browser.find_link_by_href( + '/plinth/sys/users/' + old_name + '/edit/').first.click() browser.find_by_id('id_username').fill(new_name) browser.find_by_value('Save Changes').click() def delete_user(browser, name): nav_to_module(browser, 'users') - browser.find_link_by_href('/plinth/sys/users/' + name + - '/delete/').first.click() + browser.find_link_by_href( + '/plinth/sys/users/' + name + '/delete/').first.click() browser.find_by_value('Delete ' + name).click() diff --git a/functional_tests/support/service.py b/functional_tests/support/service.py index 1dee6b8d3..d5839f047 100644 --- a/functional_tests/support/service.py +++ b/functional_tests/support/service.py @@ -15,8 +15,12 @@ # along with this program. If not, see . # +from contextlib import contextmanager from time import sleep +from selenium.common.exceptions import StaleElementReferenceException +from selenium.webdriver.support.ui import WebDriverWait + from support import interface # unlisted services just use the service_name as module name @@ -42,7 +46,7 @@ def is_not_running(browser, service_name): return browser.is_text_present('is not running') -def eventually(function, args, timeout=30): +def eventually(function, args=[], timeout=30): """Execute a function returning a boolean expression till it returns True or a timeout is reached""" counter = 1 @@ -53,3 +57,22 @@ def eventually(function, args, timeout=30): counter += 1 sleep(1) return False + + +@contextmanager +def wait_for_page_update(browser, timeout=30): + current_page = browser.find_by_tag('html').first + yield + WebDriverWait(browser, timeout).until(is_stale(current_page)) + + +class is_stale(): + def __init__(self, element): + self.element = element + + def __call__(self, driver): + try: + self.element.has_class('whatever_class') + return False + except StaleElementReferenceException: + return True diff --git a/functional_tests/support/system.py b/functional_tests/support/system.py index 3bfac455c..58735ea03 100644 --- a/functional_tests/support/system.py +++ b/functional_tests/support/system.py @@ -64,7 +64,7 @@ def set_language(browser, language_code): '/plinth/sys/users/{}/edit/'.format(username)) browser.find_by_xpath('//select[@id="id_language"]//option[@value="' + language_code + '"]').first.click() - browser.find_by_css('input[type=submit]').click() + submit(browser) def check_language(browser, language_code): diff --git a/plinth/modules/matrixsynapse/templates/matrix-synapse-pre-setup.html b/plinth/modules/matrixsynapse/templates/matrix-synapse-pre-setup.html index 5bd568cf7..d61546c88 100644 --- a/plinth/modules/matrixsynapse/templates/matrix-synapse-pre-setup.html +++ b/plinth/modules/matrixsynapse/templates/matrix-synapse-pre-setup.html @@ -54,7 +54,7 @@ {% endblocktrans %}

{% else %} -
+ {% csrf_token %} {{ form|bootstrap }} diff --git a/plinth/templates/service.html b/plinth/templates/service.html index e1e4a5cdd..4377d9bbf 100644 --- a/plinth/templates/service.html +++ b/plinth/templates/service.html @@ -76,7 +76,7 @@ {% block configuration %}

{% trans "Configuration" %}

- + {% csrf_token %} {{ form|bootstrap }} diff --git a/plinth/templates/setup.html b/plinth/templates/setup.html index 60cfd4a34..e24134253 100644 --- a/plinth/templates/setup.html +++ b/plinth/templates/setup.html @@ -50,70 +50,57 @@ {% if not setup_helper.current_operation %} - {% if setup_helper.get_state == 'needs-setup' %} -

+

+ {% if setup_helper.get_state == 'needs-setup' %} {% blocktrans trimmed %} Install this application? {% endblocktrans %} -

- - - {% csrf_token %} - - {% if package_manager_is_busy %} - - - {% else %} - - {% endif %} -
- {% elif setup_helper.get_state == 'needs-update' %} -

+ {% elif setup_helper.get_state == 'needs-update' %} {% blocktrans trimmed %} This application needs an update. Update now? {% endblocktrans %} -

+ {% endif %} +

-
- {% csrf_token %} + + {% csrf_token %} - {% if package_manager_is_busy %} - - - {% else %} - - {% endif %} -
- {% endif %} + {% if package_manager_is_busy %} + + {% endif %} + + + {% else %} {% if setup_helper.current_operation.step == 'pre' %} - {% trans "Performing pre-install operation" %} +
+ {% trans "Performing pre-install operation" %} +
{% elif setup_helper.current_operation.step == 'post' %} - {% trans "Performing post-install operation" %} +
+ {% trans "Performing post-install operation" %} +
{% elif setup_helper.current_operation.step == 'install' %} {% with transaction=setup_helper.current_operation.transaction %} -
+
{% blocktrans trimmed with package_names=transaction.package_names|join:", " status=transaction.status_string %} Installing {{ package_names }}: {{ status }} {% endblocktrans %}
-
+