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 <njoseph@thoughtworks.com>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

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') \
+        while install_in_progress():
-              or browser.is_text_present('Performing post-install operation'):
             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()
+    interface.submit(browser, 'form-configuration')
-    # 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')
 
 
 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()
+    interface.submit(browser, 'form-configuration')
-    # Wait for the service to restart after updating password
-    eventually(browser.is_text_present, args=['Upload password updated'])
 
 
 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.'])
 

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 <http://www.gnu.org/licenses/>.
 #
+
 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):
+def submit(browser, form_class=None):
-    browser.find_by_value('Submit').click()
+    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 +
+    browser.find_link_by_href(
-                              '/edit/').first.click()
+        '/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 +
+    browser.find_link_by_href(
-                              '/delete/').first.click()
+        '/plinth/sys/users/' + name + '/delete/').first.click()
     browser.find_by_value('Delete ' + name).click()
 
 

functional_tests/support/service.py

@@ -15,8 +15,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+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

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):

plinth/modules/matrixsynapse/templates/matrix-synapse-pre-setup.html

@@ -54,7 +54,7 @@
       {% endblocktrans %}
     </p>
   {% else %}
-    <form class="form" method="post">
+    <form class="form form-configuration" method="post">
       {% csrf_token %}
 
       {{ form|bootstrap }}

plinth/templates/service.html

@@ -76,7 +76,7 @@
   {% block configuration %}
     <h3>{% trans "Configuration" %}</h3>
 
-    <form class="form" method="post">
+    <form class="form form-configuration" method="post">
       {% csrf_token %}
 
       {{ form|bootstrap }}

plinth/templates/setup.html

@@ -50,70 +50,57 @@
 
   {% if not setup_helper.current_operation %}
 
-    {% if setup_helper.get_state == 'needs-setup' %}
+    <p>
-      <p>
+      {% if setup_helper.get_state == 'needs-setup' %}
         {% blocktrans trimmed %}
           Install this application?
         {% endblocktrans %}
-      </p>
+      {% elif setup_helper.get_state == 'needs-update' %}
-
-      <form action="" method="post">
-        {% csrf_token %}
-
-        {% if package_manager_is_busy %}
-          <div class="alert alert-warning" role="alert">
-            {% blocktrans trimmed %}
-              Another installation or upgrade is already running.
-              Please wait for a few moments before trying again.
-            {% endblocktrans %}
-          </div>
-          <input type="submit" class="btn btn-md btn-primary"
-                 disabled="disabled" value="{% trans "Install" %}" />
-        {% else %}
-          <input type="submit" class="btn btn-md btn-primary"
-                 value="{% trans "Install" %}" />
-        {% endif %}
-      </form>
-    {% elif setup_helper.get_state == 'needs-update' %}
-      <p>
         {% blocktrans trimmed %}
           This application needs an update. Update now?
         {% endblocktrans %}
-      </p>
+      {% endif %}
+    </p>
 
-      <form action="" method="post">
+    <form class="form-install" action="" method="post">
-        {% csrf_token %}
+      {% csrf_token %}
 
-        {% if package_manager_is_busy %}
+      {% if package_manager_is_busy %}
-          <div class="alert alert-warning" role="alert">
+        <div class="alert alert-warning" role="alert">
-            {% blocktrans trimmed %}
+          {% blocktrans trimmed %}
-              Another installation or upgrade is already running.
+            Another installation or upgrade is already running.
-              Please wait for a few moments before trying again.
+            Please wait for a few moments before trying again.
-            {% endblocktrans %}
+          {% endblocktrans %}
-          </div>
+        </div>
-          <input type="submit" class="btn btn-md btn-primary"
+      {% endif %}
-                 disabled="disabled" value="{% trans "Install" %}" />
+      <input type="submit" class="btn btn-md btn-primary"
-        {% else %}
+             {{ package_manager_is_busy|yesno:'disabled="disabled"' }}
-          <input type="submit" class="btn btn-md btn-primary"
+             {% if setup_helper.get_state == 'needs-setup' %}
-                 value="{% trans "Update" %}" />
+             value="{% trans "Install" %}"
-        {% endif %}
+             {% elif setup_helper.get_state == 'needs-upgrade' %}
-      </form>
+             value="{% trans "Update" %}"
-    {% endif %}
+             {% endif %} />
+
+    </form>
 
   {% else %}
 
     {% if setup_helper.current_operation.step == 'pre' %}
-      {% trans "Performing pre-install operation" %}
+      <div class="install-state-pre">
+        {% trans "Performing pre-install operation" %}
+      </div>
     {% elif setup_helper.current_operation.step == 'post' %}
-      {% trans "Performing post-install operation" %}
+      <div class="install-state-post">
+        {% trans "Performing post-install operation" %}
+      </div>
     {% elif setup_helper.current_operation.step == 'install' %}
       {% with transaction=setup_helper.current_operation.transaction %}
-        <div>
+        <div class="install-state-installing">
           {% blocktrans trimmed with package_names=transaction.package_names|join:", " status=transaction.status_string %}
             Installing {{ package_names }}: {{ status }}
           {% endblocktrans %}
         </div>
-        <div class="progress">
+        <div class="install-state-progress">
           <div class="progress-bar progress-bar-striped active"
                role="progressbar" aria-valuemin="0" aria-valuemax="100"
                aria-valuenow="{{ transaction.percentage }}"