From 13f89709f70bc87c2e92c961d032346a98979e3c Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Sun, 19 May 2024 01:54:14 +0100 Subject: [PATCH] end2end tests: clean up webdriver init and move to webdriver_utils --- .ci/end2end_tests.Dockerfile | 6 +-- tests/demos.py | 4 +- tests/end2end_test.py | 77 +++++--------------------------- tests/webdriver_utils.py | 86 +++++++++++++++++++++++++++++++++++- tox.ini | 1 - 5 files changed, 101 insertions(+), 73 deletions(-) diff --git a/.ci/end2end_tests.Dockerfile b/.ci/end2end_tests.Dockerfile index 0c4f06ee..dadd24ac 100644 --- a/.ci/end2end_tests.Dockerfile +++ b/.ci/end2end_tests.Dockerfile @@ -2,9 +2,6 @@ FROM ubuntu:jammy ENV DEBIAN_FRONTEND=noninteractive -# used in end2end tests -ENV UNDER_DOCKER=true - RUN apt-get update \ ## install chrome && apt-get install -y wget \ @@ -18,6 +15,9 @@ RUN apt-get update \ && wget 'https://www.googleapis.com/download/storage/v1/b/chromium-browser-snapshots/o/Linux_x64%2F1110897%2Fchromedriver_linux64.zip?generation=1677589097630198&alt=media' -O /tmp/chrome/chromedriver_linux64.zip \ && unzip /tmp/chrome/chrome-linux.zip -d /tmp/chrome \ && unzip /tmp/chrome/chromedriver_linux64.zip -d /tmp/chrome \ + && ln -sf /tmp/chrome/chrome-linux/chrome /usr/bin/google-chrome \ + ## TODO don't need chromedriver anymore since selenium manages to download it itself? + ## TODO don't install chrome above? just install necessary deps ## ## install firefox # install add-apt-repository command diff --git a/tests/demos.py b/tests/demos.py index d9b25ede..5e041967 100644 --- a/tests/demos.py +++ b/tests/demos.py @@ -462,7 +462,7 @@ def scroll_to_text(driver, text: str): # TODO a bit of wait?? -from end2end_test import get_webdriver # type: ignore[attr-defined] +from end2end_test import get_webdriver @uses_x @@ -509,7 +509,7 @@ def before(driver): # TODO go to div class="source" -> a class="original" # driver without the extension ORIG = 'http://nautil.us/issue/66/clockwork/haunted-by-his-brother-he-revolutionized-physics-rp' - with get_webdriver(browser, extension=False) as driver2: + with get_webdriver(browser, extension=False) as driver2: # type: ignore[misc,call-arg] driver2.get(ORIG) # TODO maybe, have annotation 'start' and 'interrupt'? diff --git a/tests/end2end_test.py b/tests/end2end_test.py index 0eac9d97..a854c600 100755 --- a/tests/end2end_test.py +++ b/tests/end2end_test.py @@ -6,9 +6,8 @@ from datetime import datetime import os from pathlib import Path -import shutil from time import sleep -from typing import Iterator, TypeVar, Callable, Dict, Any +from typing import Iterator, TypeVar, Callable import pytest @@ -18,7 +17,6 @@ pytest.main(['-s', __file__]) -from selenium import webdriver from selenium.webdriver import Remote as Driver from selenium.webdriver.common.keys import Keys from selenium.webdriver.common.by import By @@ -30,7 +28,7 @@ from promnesia.logging import LazyLogger from common import under_ci, has_x, local_http_server, notnone -from webdriver_utils import is_visible, wait_for_alert +from webdriver_utils import is_visible, wait_for_alert, get_webdriver from addon import get_addon_source, Addon, LOCALHOST, addon @@ -73,64 +71,6 @@ def browser_(driver: Driver) -> Browser: raise AssertionError(driver) -def _get_webdriver(tdir: Path, browser: Browser, extension: bool = True) -> Driver: - addon = get_addon_source(kind=browser.dist) - driver: Driver - if browser.name == 'firefox': - ff_options = webdriver.FirefoxOptions() - ff_options.set_preference('profile', str(tdir)) - # todo remove when webdriver is updated - # see https://github.com/mozilla/geckodriver/issues/2075 - ff_options.set_preference('fission.autostart', True) - if browser.headless: - ff_options.add_argument('--headless') - # use firefox from here to test https://www.mozilla.org/en-GB/firefox/developer/ - driver = webdriver.Firefox(options=ff_options) - # todo pass firefox-dev binary? - if extension: - driver.install_addon(str(addon), temporary=True) - elif browser.name == 'chrome': - # TODO ugh. very hacky... - assert extension, "TODO add support for extension arg" - ex = tdir / 'extension.zip' - shutil.make_archive(str(ex.with_suffix('')), format='zip', root_dir=addon) - # looks like chrome uses temporary dir for data anyway - cr_options = webdriver.ChromeOptions() - if browser.headless: - if 'UNDER_DOCKER' in os.environ: - # docker runs as root and chrome refuses to use headless in that case - cr_options.add_argument('--no-sandbox') - - # regular --headless doesn't support extensions for some reason - cr_options.add_argument('--headless=new') - cr_options.add_extension(str(ex)) - - # right, so recent chrome/chromium have this regression https://bugs.chromium.org/p/chromedriver/issues/detail?id=4440 - # which breaks tons of tests due to iframe use - use_custom_chromium = True - mexepath: Dict[str, Any] = {} - if use_custom_chromium: - # see setup in end2end_tests.Dockerfile - user_data_dir = tdir / 'udir' - user_data_dir.mkdir() - # seems necessary, otherwise it somehow falls back onto snap chromium and gets stuck? - cr_options.add_argument('--user-data-dir=' + str(user_data_dir)) - # ugh. sadly somehow if you add these chrome dirs to PATH, it doesn't work it still tries to use system binary? - - cr_options.binary_location = '/tmp/chrome/chrome-linux/chrome' - driver_path = Path('/tmp/chrome/chromedriver_linux64/chromedriver') - assert driver_path.exists() - mexepath['executable_path'] = str(driver_path) - from selenium.webdriver.chrome.service import Service - service = Service(**mexepath) - driver = webdriver.Chrome(service=service, options=cr_options) - # TODO ad this to common helper - logger.info(f"using webdriver: {driver.capabilities['browserVersion']} {driver.capabilities['chrome']['chromedriverVersion']}") - else: - raise RuntimeError(f'Unexpected browser {browser}') - return driver - - def confirm(what: str) -> None: is_headless = 'headless' in os.environ.get('PYTEST_CURRENT_TEST', '') if is_headless: @@ -200,11 +140,18 @@ def ff(*args, **kwargs): @pytest.fixture def driver(tmp_path: Path, browser: Browser) -> Iterator[Driver]: - driver = _get_webdriver(tmp_path, browser=browser, extension=True) + profile_dir = tmp_path / 'browser_profile' + res = get_webdriver( + profile_dir=profile_dir, + addon_source=get_addon_source(kind=browser.dist), + browser=browser.name, + headless=browser.headless, + logger=logger, + ) try: - yield driver + yield res finally: - driver.quit() + res.quit() @dataclass diff --git a/tests/webdriver_utils.py b/tests/webdriver_utils.py index 1f3d94cc..bc493122 100644 --- a/tests/webdriver_utils.py +++ b/tests/webdriver_utils.py @@ -1,8 +1,9 @@ from contextlib import contextmanager +from pathlib import Path from time import sleep -from typing import Optional, Iterator - +from typing import Dict, Iterator, Optional +from selenium import webdriver from selenium.common.exceptions import NoAlertPresentException from selenium.webdriver import Remote as Driver from selenium.webdriver.common.alert import Alert @@ -72,3 +73,84 @@ def wait_for_alert(driver: Driver) -> Alert: continue assert e is not None raise e + + +def get_webdriver( + *, + profile_dir: Path, + addon_source: Path, + browser: str, + headless: bool, + logger, +) -> Driver: + # useful for debugging + # import logging + # from selenium.webdriver.remote.remote_connection import LOGGER + # LOGGER.setLevel(logging.DEBUG) + + # hmm. seems like if it can't find the driver, selenium automatically downloads it? + driver: Driver + version_data: Dict[str, str] + if browser == 'firefox': + ff_options = webdriver.FirefoxOptions() + ff_options.set_preference('profile', str(profile_dir)) + # ff_options.binary_location = '' # set custom path here + # e.g. use firefox from here to test https://www.mozilla.org/en-GB/firefox/developer/ + if headless: + ff_options.add_argument('--headless') + driver = webdriver.Firefox(options=ff_options) + + addon_id = driver.install_addon(str(addon_source), temporary=True) + logger.debug(f'firefox addon id: {addon_id}') + + version_data = {} + # TODO 'binary'? might not be present? + for key in ['browserName', 'browserVersion', 'moz:geckodriverVersion', 'moz:headless', 'moz:profile']: + version_data[key] = driver.capabilities[key] + version_data['driver_path'] = getattr(driver.service, '_path') + elif browser == 'chrome': + cr_options = webdriver.ChromeOptions() + chrome_bin: Optional[str] = None # default (e.g. apt version) + if chrome_bin is not None: + cr_options.binary_location = chrome_bin + + cr_options.add_argument(f'--load-extension={addon_source}') + cr_options.add_argument(f'--user-data-dir={profile_dir}') # todo point to a subdir? + + if headless: + if Path('/.dockerenv').exists(): + # necessary, otherwise chrome fails to start under containers + cr_options.add_argument('--no-sandbox') + + # regular --headless doesn't support extensions for some reason + cr_options.add_argument('--headless=new') + + # generally 'selenium manager' download the correct driver version itself + chromedriver_bin: Optional[str] = None # default + + service = webdriver.ChromeService(executable_path=chromedriver_bin) + driver = webdriver.Chrome(service=service, options=cr_options) + + major_version = int(driver.capabilities['browserVersion'].split('.')[0]) + if major_version > 113: + # NOTE: feel free to comment this out if necessary, it's just to avoid hours of debugging + raise RuntimeError( + f""" +NOTE: you're using chrome {driver.capabilities['browserVersion']}. +Some tests aren't working with recent Chrome versions due to regressions in chromedriver. +See https://bugs.chromium.org/p/chromedriver/issues/detail?id=4440 +""" + ) + + version_data = {} + # TODO 'binary'? might not be present? + for key in ['browserName', 'browserVersion']: + version_data[key] = driver.capabilities[key] + version_data['chromedriverVersion'] = driver.capabilities['chrome']['chromedriverVersion'] + version_data['userDataDir'] = driver.capabilities['chrome']['userDataDir'] + version_data['driver_path'] = getattr(driver.service, '_path') + else: + raise RuntimeError(f'Unexpected browser {browser}') + version_string = ' '.join(f'{k}={v}' for k, v in version_data.items()) + logger.info(f'webdriver version: {version_string}') + return driver diff --git a/tox.ini b/tox.ini index 63091f6f..62200cc8 100644 --- a/tox.ini +++ b/tox.ini @@ -24,7 +24,6 @@ passenv = # by default we don't run browser tests to avoid confusing people when they run locally # but we want them on CI, so we allow to pass through the variable when we do want to run them WITH_BROWSER_TESTS - UNDER_DOCKER # todo ugh this is all so confusing... need to simplify