From 81773af4861c8e4daeefa6b95f01cb9152e44fd0 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Tue, 30 Jul 2024 21:46:59 +0200 Subject: [PATCH 1/7] test: Port from CDP to BiDi browser automation This slightly changes the API of `Browser.keys()` with modifiers. We only use that once inside of testlib.py and twice in check-system-terminal (and nowhere in external projects), so just adjust the three callers. The `Browser.get_js_log()` output format also changes slightly, to be much more detailed. Adjust tests accordingly. Skip `TestGrafanaClient` and `TestPages.testAllLanguages` on Chromium for now. There is a really weird bug where Cockpit/Grafana spontaneously logs out itself, but *only* in headless Chromium. So run the test only on Firefox for the time being. As penance, enable testUpload*() on Firefox, the BiDi API works fine there. We can enable other tests on Firefox as a follow-up as well (they need some work unrelated to BiDi though). This removes excess right/bottom padding from two mobile pixel tests, update them. https://issues.redhat.com/browse/COCKPIT-1143 --- pyproject.toml | 1 + test/common/storagelib.py | 2 +- test/common/testlib.py | 437 +++++++++++++---------- test/common/webdriver_bidi.py | 461 +++++++++++++++++++++++++ test/reference | 2 +- test/verify/check-embed | 2 +- test/verify/check-kdump | 6 +- test/verify/check-metrics | 2 + test/verify/check-pages | 5 +- test/verify/check-selinux | 6 +- test/verify/check-shell-host-switching | 6 +- test/verify/check-shell-menu | 14 +- test/verify/check-sosreport | 3 +- test/verify/check-static-login | 8 +- test/verify/check-system-terminal | 12 +- tools/vulture_suppressions/cdp.py | 4 + 16 files changed, 755 insertions(+), 216 deletions(-) create mode 100644 test/common/webdriver_bidi.py create mode 100644 tools/vulture_suppressions/cdp.py diff --git a/pyproject.toml b/pyproject.toml index 3a75f3c77d94..e9b00ff62166 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,7 @@ module = [ # test/common 'cdp', 'testlib', + 'webdriver_bidi', ] [tool.pylint] diff --git a/test/common/storagelib.py b/test/common/storagelib.py index 38877302eae2..357b0651a4c5 100644 --- a/test/common/storagelib.py +++ b/test/common/storagelib.py @@ -241,7 +241,7 @@ def dialog_cancel(self): def dialog_wait_close(self): # file system operations often take longer than 10s - with self.browser.wait_timeout(max(self.browser.cdp.timeout, 60)): + with self.browser.wait_timeout(max(self.browser.timeout, 60)): self.browser.wait_not_present('#dialog') def dialog_check(self, expect): diff --git a/test/common/testlib.py b/test/common/testlib.py index 7a87e5d47d7a..2c2ab842d47f 100644 --- a/test/common/testlib.py +++ b/test/common/testlib.py @@ -18,6 +18,7 @@ """Tools for writing Cockpit test cases.""" import argparse +import asyncio import base64 import contextlib import errno @@ -25,6 +26,7 @@ import glob import io import json +import logging import os import re import shutil @@ -32,13 +34,15 @@ import subprocess import sys import tempfile +import threading import time import traceback import unittest -from collections.abc import Collection, Container, Iterator, Mapping, Sequence -from typing import Any, Callable, ClassVar, Literal, Never, TypedDict, TypeVar +from collections.abc import Collection, Container, Coroutine, Iterator, Mapping, Sequence +from pathlib import Path +from typing import Any, Callable, ClassVar, Literal, TypedDict, TypeVar -import cdp +import webdriver_bidi from lcov import write_lcov from lib.constants import OSTREE_IMAGES from machine import testvm @@ -47,6 +51,8 @@ _T = TypeVar('_T') _FT = TypeVar("_FT", bound=Callable[..., Any]) +JsonObject = dict[str, Any] + BASE_DIR = os.path.realpath(f'{__file__}/../../..') TEST_DIR = f'{BASE_DIR}/test' BOTS_DIR = f'{BASE_DIR}/bots' @@ -92,6 +98,25 @@ opts.coverage = False +# https://w3c.github.io/webdriver/#keyboard-actions for encoding key names +WEBDRIVER_KEYS = { + "Backspace": "\uE003", + "Tab": "\uE004", + "Return": "\uE006", + "Enter": "\uE007", + "Shift": "\uE008", + "Control": "\uE009", + "Alt": "\uE00A", + "Escape": "\uE00C", + "ArrowLeft": "\uE012", + "ArrowUp": "\uE013", + "ArrowRight": "\uE014", + "ArrowDown": "\uE015", + "Insert": "\uE016", + "Delete": "\uE017", +} + + # Browser layouts # # A browser can be switched into a number of different layouts, such @@ -185,6 +210,8 @@ def unique_filename(base: str, ext: str) -> str: class Browser: + driver: webdriver_bidi.WebdriverBidi + browser: str layouts: Sequence[BrowserLayout] current_layout: BrowserLayout | None port: str | int @@ -211,39 +238,97 @@ def __init__( self.used_pixel_references = set[str]() self.coverage_label = coverage_label self.machine = machine - path = os.path.dirname(__file__) - sizzle_js = os.path.join(path, "../../node_modules/sizzle/dist/sizzle.js") - helpers = [os.path.join(path, "test-functions.js")] - if os.path.exists(sizzle_js): - helpers.append(sizzle_js) - self.cdp = cdp.CDP("C.utf8", verbose=opts.trace, trace=opts.trace, - inject_helpers=helpers, - start_profile=coverage_label is not None) + + headless = not bool(os.environ.get("TEST_SHOW_BROWSER", "")) + self.browser = os.environ.get("TEST_BROWSER", "chromium") + if self.browser == "chromium": + self.driver = webdriver_bidi.ChromiumBidi(headless=headless) + elif self.browser == "firefox": + self.driver = webdriver_bidi.FirefoxBidi(headless=headless) + else: + raise ValueError(f"unknown browser {self.browser}") + self.loop = asyncio.new_event_loop() + self.bidi_thread = threading.Thread(target=self.asyncio_loop_thread, args=(self.loop,)) + self.bidi_thread.start() + + self.run_async(self.driver.start_session()) + + if opts.trace: + logging.basicConfig(level=logging.INFO) + webdriver_bidi.log_command.setLevel(logging.INFO if opts.trace else logging.WARNING) + # not appropriate for --trace, just enable for debugging low-level protocol with browser + # bidiwebdriver_.log_proto.setLevel(logging.DEBUG) + + test_functions = (Path(__file__).parent / "test-functions.js").read_text() + # Don't redefine globals, this confuses Firefox + test_functions = "if (window.ph_select) return; " + test_functions + self.bidi("script.addPreloadScript", quiet=True, functionDeclaration=f"() => {{ {test_functions} }}") + + try: + sizzle_js = (Path(__file__).parent.parent.parent / "node_modules/sizzle/dist/sizzle.js").read_text() + # HACK: injecting sizzle fails on missing `document` in assert() + sizzle_js = sizzle_js.replace('function assert( fn ) {', + 'function assert( fn ) { if (true) return true; else ') + # HACK: sizzle tracks document and when we switch frames, it sees the old document + # although we execute it in different context. + sizzle_js = sizzle_js.replace('context = context || document;', 'context = context || window.document;') + self.bidi("script.addPreloadScript", quiet=True, functionDeclaration=f"() => {{ {sizzle_js} }}") + except FileNotFoundError: + pass + + if coverage_label: + self.cdp_command("Profiler.enable") + self.cdp_command("Profiler.startPreciseCoverage", callCount=False, detailed=True) + self.password = "foobar" self.timeout_factor = int(os.getenv("TEST_TIMEOUT_FACTOR", "1")) + self.timeout = 15 self.failed_pixel_tests = 0 self.allow_oops = False - self.body_clip = None try: with open(f'{TEST_DIR}/browser-layouts.json') as fp: self.layouts = json.load(fp) except FileNotFoundError: self.layouts = default_layouts - # Firefox CDP does not support setting EmulatedMedia - # https://bugzilla.mozilla.org/show_bug.cgi?id=1549434 - if self.cdp.browser.name != "chromium": - self.layouts = [layout for layout in self.layouts if layout["theme"] != "dark"] self.current_layout = None + self.valid = True - # we don't have a proper SSL certificate for our tests, ignore it - # Firefox does not support this, tests which rely on it need to skip it - if self.cdp.browser.name == "chromium": - self.cdp.command("setSSLBadCertificateAction('continue')") + def run_async(self, coro: Coroutine[Any, Any, Any]) -> JsonObject: + """Run coro in main loop in our BiDi thread - def allow_download(self) -> None: - """Allow browser downloads""" - if self.cdp.browser.name == "chromium": - self.cdp.invoke("Page.setDownloadBehavior", behavior="allow", downloadPath=self.cdp.download_dir) + Wait for the result and return it. + """ + return asyncio.run_coroutine_threadsafe(coro, self.loop).result() + + @staticmethod + def asyncio_loop_thread(loop: asyncio.AbstractEventLoop) -> None: + asyncio.set_event_loop(loop) + loop.run_forever() + + def kill(self) -> None: + if not self.valid: + return + self.run_async(self.driver.close()) + self.loop.call_soon_threadsafe(self.loop.stop) + self.bidi_thread.join() + self.valid = False + + def bidi(self, method: str, **params: Any) -> webdriver_bidi.JsonObject: + """Send a Webdriver BiDi command and return the JSON response""" + + try: + return self.run_async(self.driver.bidi(method, **params)) + except webdriver_bidi.WebdriverError as e: + raise Error(str(e)) from None + + def cdp_command(self, method: str, **params: Any) -> webdriver_bidi.JsonObject: + """Send a Chrome DevTools Protocol command and return the JSON response""" + + if self.browser == "chromium": + assert isinstance(self.driver, webdriver_bidi.ChromiumBidi) + return self.run_async(self.driver.cdp(method, **params)) + else: + raise webdriver_bidi.WebdriverError("CDP is only supported in Chromium") def open(self, href: str, cookie: Mapping[str, str] | None = None, tls: bool = False) -> None: """Load a page into the browser. @@ -264,14 +349,15 @@ def open(self, href: str, cookie: Mapping[str, str] | None = None, tls: bool = F size = self.current_layout["shell_size"] self._set_window_size(size[0], size[1]) if cookie: - self.cdp.invoke("Network.setCookie", **cookie) + c = {**cookie, "value": {"type": "string", "value": cookie["value"]}} + self.bidi("storage.setCookie", cookie=c) self.switch_to_top() # Some browsers optimize this away if the current URL is already href # (e.g. in TestKeys.testAuthorizedKeys). Load the blank page first to always # force a load. - self.cdp.invoke("Page.navigate", url="about:blank") - self.cdp.invoke("Page.navigate", url=href) + self.bidi("browsingContext.navigate", context=self.driver.context, url="about:blank", wait="complete") + self.bidi("browsingContext.navigate", context=self.driver.context, url=href, wait="complete") def set_user_agent(self, ua: str) -> None: """Set the user agent of the browser @@ -279,7 +365,10 @@ def set_user_agent(self, ua: str) -> None: :param ua: user agent string :type ua: str """ - self.cdp.invoke("Emulation.setUserAgentOverride", userAgent=ua) + if self.browser == "chromium": + self.cdp_command("Emulation.setUserAgentOverride", userAgent=ua) + else: + raise NotImplementedError def reload(self, ignore_cache: bool = False) -> None: """Reload the current page @@ -289,8 +378,15 @@ def reload(self, ignore_cache: bool = False) -> None: """ self.switch_to_top() - self.wait_js_cond("ph_select('iframe.container-frame').every(function (e) { return e.getAttribute('data-loaded'); })") - self.cdp.invoke("Page.reload", ignoreCache=ignore_cache) + self.wait_js_cond("ph_select('iframe.container-frame').every(e => e.getAttribute('data-loaded'))") + if self.browser == "firefox": + if ignore_cache: + webdriver_bidi.log_command.warning( + "Browser.reload(): ignore_cache==True not yet supported with Firefox, ignoring") + self.bidi("browsingContext.reload", context=self.driver.context, wait="complete") + else: + self.bidi("browsingContext.reload", context=self.driver.context, ignoreCache=ignore_cache, + wait="complete") self.machine.allow_restart_journal_messages() @@ -302,14 +398,23 @@ def switch_to_frame(self, name: str | None) -> None: :param name: frame name """ - self.cdp.set_frame(name) + if name is None: + self.switch_to_top() + else: + self.run_async(self.driver.switch_to_frame(name)) def switch_to_top(self) -> None: """Switch to the main frame Switch to the main frame from for example an iframe. """ - self.cdp.set_frame(None) + self.driver.switch_to_top() + + def allow_download(self) -> None: + """Allow browser downloads""" + # this is only necessary for headless chromium + if self.browser == "chromium": + self.cdp_command("Page.setDownloadBehavior", behavior="allow", downloadPath=str(self.driver.download_dir)) def upload_files(self, selector: str, files: Sequence[str]) -> None: """Upload a local file to the browser @@ -317,23 +422,8 @@ def upload_files(self, selector: str, files: Sequence[str]) -> None: The selector should select the element. Files is a list of absolute paths to files which should be uploaded. """ - r = self.cdp.invoke("Runtime.evaluate", expression='document.querySelector(%s)' % jsquote(selector)) - objectId = r["result"]["objectId"] - self.cdp.invoke("DOM.setFileInputFiles", files=files, objectId=objectId) - - def raise_cdp_exception( - self, func: str, arg: str, details: Mapping[str, Any], trailer: str | None = None - ) -> Never: - # unwrap a typical error string - if details.get("exception", {}).get("type") == "string": - msg = details["exception"]["value"] - elif details.get("text", None): - msg = details.get("text", None) - else: - msg = str(details) - if trailer: - msg += "\n" + trailer - raise Error("%s(%s): %s" % (func, arg, msg)) + element = self.eval_js(f"ph_find({jsquote(selector)})") + self.bidi("input.setFiles", context=self.driver.context, element=element, files=files) def inject_js(self, code: str) -> None: """Execute JS code that does not return anything @@ -341,8 +431,8 @@ def inject_js(self, code: str) -> None: :param code: a string containing JavaScript code :type code: str """ - self.cdp.invoke("Runtime.evaluate", expression=code, trace=code, - silent=False, awaitPromise=True, returnByValue=False, no_trace=True) + # this is redundant now; the difference used to be important with CDP + self.eval_js(code) def eval_js(self, code: str, no_trace: bool = False) -> Any: """Execute JS code that returns something @@ -350,21 +440,8 @@ def eval_js(self, code: str, no_trace: bool = False) -> Any: :param code: a string containing JavaScript code :param no_trace: do not print information about unknown return values (default False) """ - result = self.cdp.invoke("Runtime.evaluate", expression=code, trace=code, - silent=False, awaitPromise=True, returnByValue=True, no_trace=no_trace) - if "exceptionDetails" in result: - self.raise_cdp_exception("eval_js", code, result["exceptionDetails"]) - _type = result.get("result", {}).get("type") - if _type == 'object' and result["result"].get("subtype", "") == "error": - raise Error(result["result"]["description"]) - if _type == "undefined": - return None - if _type and "value" in result["result"]: - return result["result"]["value"] - - if opts.trace: - print("eval_js(%s): cannot interpret return value %s" % (code, result)) - return None + return self.bidi("script.evaluate", expression=code, quiet=no_trace, + awaitPromise=True, target={"context": self.driver.context})["result"] def call_js_func(self, func: str, *args: object) -> Any: """Call a JavaScript function @@ -396,11 +473,13 @@ def cookie(self, name: str) -> Mapping[str, object] | None: :param name: the name of the cookie :type name: str """ - cookies = self.cdp.invoke("Network.getCookies") - for c in cookies["cookies"]: - assert isinstance(c, Mapping) - if c["name"] == name: - return c + cookies = self.bidi("storage.getCookies", filter={"name": name})["cookies"] + if len(cookies) > 0: + c = cookies[0] + # if we ever need to handle "base64", add that + assert c["value"]["type"] == "string" + c["value"] = c["value"]["value"] + return c return None def go(self, url_hash: str) -> None: @@ -439,7 +518,7 @@ def click(self, selector: str) -> None: :param selector: the selector to click on """ - self.mouse(selector + ":not([disabled]):not([aria-disabled=true])", "click", 0, 0, 0) + self.mouse(selector + ":not([disabled]):not([aria-disabled=true])", "click") def val(self, selector: str) -> Any: """Get the value attribute of a selector. @@ -502,8 +581,10 @@ def set_checked(self, selector: str, val: bool) -> None: :param selector: the selector :param val: boolean value to enable or disable checkbox """ - self.wait_visible(selector + ':not([disabled]):not([aria-disabled=true])') - self.call_js_func('ph_set_checked', selector, val) + # avoid ph_set_checked, that doesn't use proper mouse emulation + checked = self.get_checked(selector) + if checked != val: + self.click(selector) def focus(self, selector: str) -> None: """Set focus on selected element. @@ -522,43 +603,37 @@ def blur(self, selector: str) -> None: self.call_js_func('ph_blur', selector) def input_text(self, text: str) -> None: - for char in text: - if char == "\n": - self.key("Enter") - else: - self.cdp.invoke("Input.dispatchKeyEvent", type="keyDown", text=char, key=char) - self.cdp.invoke("Input.dispatchKeyEvent", type="keyUp", text=char, key=char) - - def key(self, name: str, repeat: int = 1, modifiers: int = 0) -> None: + actions = [] + for c in text: + actions.append({"type": "keyDown", "value": c}) + actions.append({"type": "keyUp", "value": c}) + self.bidi("input.performActions", context=self.driver.context, actions=[ + {"type": "key", "id": "key-0", "actions": actions}]) + + def key(self, name: str, repeat: int = 1, modifiers: list[str] | None = None) -> None: """Press and release a named keyboard key. Use this function to input special characters or modifiers. - :param name: key name like "Enter", "Delete", or "ArrowLeft" + :param name: ASCII value or key name like "Enter", "Delete", or "ArrowLeft" (entry in WEBDRIVER_KEYS) :param repeat: number of times to repeat this key (default 1) - :param modifiers: bit field: Alt=1, Ctrl=2, Meta/Command=4, Shift=8 + :param modifiers: "Shift", "Control", "Alt" """ - args: dict[str, int | str] = {} - if self.cdp.browser.name == "chromium": - # HACK: chromium doesn't understand some key codes in some situations - win_keys = { - "Backspace": 8, - "Enter": 13, - "Escape": 27, - "ArrowDown": 40, - "Insert": 45, - } - if name in win_keys: - args["windowsVirtualKeyCode"] = win_keys[name] - if name == 'Enter': - args["text"] = '\r' - # HACK: chromium needs windowsVirtualKeyCode with modifiers - elif len(name) == 1 and name.isalnum() and modifiers != 0: - args["windowsVirtualKeyCode"] = ord(name.upper()) + actions = [] + actions_pre = [] + actions_post = [] + keycode = WEBDRIVER_KEYS.get(name, name) + + for m in (modifiers or []): + actions_pre.append({"type": "keyDown", "value": WEBDRIVER_KEYS[m]}) + actions_post.append({"type": "keyUp", "value": WEBDRIVER_KEYS[m]}) for _ in range(repeat): - self.cdp.invoke("Input.dispatchKeyEvent", type="keyDown", key=name, modifiers=modifiers, **args) - self.cdp.invoke("Input.dispatchKeyEvent", type="keyUp", key=name, modifiers=modifiers, **args) + actions.append({"type": "keyDown", "value": keycode}) + actions.append({"type": "keyUp", "value": keycode}) + + self.bidi("input.performActions", context=self.driver.context, actions=[ + {"type": "key", "id": "key-0", "actions": actions_pre + actions + actions_post}]) def select_from_dropdown(self, selector: str, value: object) -> None: """For an actual