From 176dc69fc5a45dd1de7443d543389c172fb0254e Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 28 May 2020 09:12:52 -0700 Subject: [PATCH] tests: functional: Remove implicit and explicit wait times - Splinter/selenium have implicit and explicit waiting time. Implicit wait time will make every negative lookup wait for about 3 seconds before it actually fails. Because we ensure missing elements in quite a few places, this introduces many 3 seconds wait periods during testing. Remove it instead rely on explicit waiting whenever needed. - Explicit wait time is only used during explicitly requests waiting conditions. In a loop the API waits for a maximum of timeout period until a given condition is satisfied. Each time the condition is checked, it goes into sleep for explicit wait period amount of time. This is typically a second or so. Since we are impatient, make it 0.1 instead. - Also make sure that whenever a page is visit()ed, we automatically wait until the page is fully loaded by overriding the splinter wait condition. Otherwise, we will need to introduce waiting code in a lot of places. - Using document.readyState == complete is a better check to ensure that a page is fully loaded. If we proceed with the page 'loading' or 'interactive' state, we will have to change a lot of code to make it wait. - Handle Apache restarts when waiting for page load. The error page apparently is never reaches document.readyState == 'complete'. So, if an error page is encountered, always reload. - While waiting for installation, ensure that we atomically check that page has loaded fully and the installation progress is not being shown. Otherwise, there would be race condition due to installation page refreshing itself. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- conftest.py | 26 +++++++++++++++ plinth/templates/setup.html | 6 ++-- plinth/tests/functional/__init__.py | 51 +++++++++++++---------------- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/conftest.py b/conftest.py index 873409358..d892c6120 100644 --- a/conftest.py +++ b/conftest.py @@ -85,3 +85,29 @@ def fixture_needs_sudo(): """Skip test if sudo command is not available.""" if not os.path.isfile('/usr/bin/sudo'): pytest.skip('Needs sudo command installed.') + + +@pytest.fixture(scope='session') +def splinter_selenium_implicit_wait(): + """Disable implicit waiting.""" + return 0 + + +@pytest.fixture(scope='session') +def splinter_wait_time(): + """Disable explicit waiting.""" + return 0.01 + + +@pytest.fixture(scope='session') +def splinter_browser_load_condition(): + """When a page it loaded, wait until is available.""" + + def _load_condition(browser): + if browser.url == 'about:blank': + return True + + ready_state = browser.execute_script('return document.readyState;') + return ready_state == 'complete' + + return _load_condition diff --git a/plinth/templates/setup.html b/plinth/templates/setup.html index 9ddbf5aa6..760373691 100644 --- a/plinth/templates/setup.html +++ b/plinth/templates/setup.html @@ -79,16 +79,16 @@ {% else %} {% if setup_current_operation.step == 'pre' %} -
+
{% trans "Performing pre-install operation" %}
{% elif setup_current_operation.step == 'post' %} -
+
{% trans "Performing post-install operation" %}
{% elif setup_current_operation.step == 'install' %} {% with transaction=setup_current_operation.transaction %} -
+
{% blocktrans trimmed with package_names=transaction.package_names|join:", " status=transaction.status_string %} Installing {{ package_names }}: {{ status }} {% endblocktrans %} diff --git a/plinth/tests/functional/__init__.py b/plinth/tests/functional/__init__.py index 0f3cf136e..56138bb7c 100644 --- a/plinth/tests/functional/__init__.py +++ b/plinth/tests/functional/__init__.py @@ -94,11 +94,22 @@ class _PageLoaded(): try: self.element.has_class('whatever_class') except StaleElementReferenceException: - if self.expected_url is None: + # After a domain name change, Let's Encrypt will reload the web + # server and could cause a connection failure. + if driver.find_by_id('netErrorButtonContainer'): + driver.visit(driver.url) + return False + + is_fully_loaded = driver.execute_script( + 'return document.readyState;') == 'complete' + if not is_fully_loaded: + is_stale = False + elif self.expected_url is None: is_stale = True else: if driver.url.endswith(self.expected_url): is_stale = True + return is_stale @@ -273,33 +284,19 @@ def is_installed(browser, app_name): def install(browser, app_name): install_button = _find_install_button(browser, app_name) + if not install_button: + return - 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) - - def is_server_restarting(): - return browser.is_element_present_by_css('.neterror') - - def wait_for_install(): - if install_in_progress(): - time.sleep(1) - elif is_server_restarting(): - time.sleep(1) + install_button.click() + while True: + script = 'return (document.readyState == "complete") && ' \ + '(!Boolean(document.querySelector(".installing")));' + if not browser.execute_script(script): + time.sleep(0.1) + elif browser.is_element_present_by_css('.neterror'): browser.visit(browser.url) else: - return - wait_for_install() - - if install_button: - install_button.click() - wait_for_install() - # sleep(2) # XXX This shouldn't be required. + break ################################ @@ -349,10 +346,6 @@ def set_domain_name(browser, domain_name): nav_to_module(browser, 'config') browser.find_by_id('id_domainname').fill(domain_name) submit(browser) - # After a domain name change, Let's Encrypt will reload the web server and - # could cause a connection failure. - if browser.find_by_id('netErrorButtonContainer'): - browser.visit(browser.url) ########################