From 4f622088c7a02561e5f4151eb6979c57da23c915 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:17:34 +0100 Subject: [PATCH 01/12] test: Avoid parentheses for tuples in subscripts Fixes RUF031 with ruff 0.6.7. --- test/common/packagelib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/packagelib.py b/test/common/packagelib.py index 02877b40d36e..5a2f22ec2592 100644 --- a/test/common/packagelib.py +++ b/test/common/packagelib.py @@ -157,7 +157,7 @@ def createPackage(self, name, version, release, install=False, else: self.createRpm(name, version, release, depends, postinst, install, content, arch, provides) if updateinfo: - self.updateInfo[(name, version, release)] = updateinfo + self.updateInfo[name, version, release] = updateinfo def createDeb(self, name, version, depends, postinst, install, content, arch, provides): """Create a dummy deb in repo_dir on self.machine From 71e80dcd2fd8a2ac507078a7c77ace07e6268c6c Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 12:22:00 +0100 Subject: [PATCH 02/12] test: Annotate js_error_codes type This should be obvious, but current mypy complains: > Need type annotation for "js_error_codes" [var-annotated] --- test/common/typecheck | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/typecheck b/test/common/typecheck index 153a5b5c033e..ea684800f3c2 100755 --- a/test/common/typecheck +++ b/test/common/typecheck @@ -165,7 +165,7 @@ except FileNotFoundError: sys.exit(77) cur = [] -js_error_codes = Counter() +js_error_codes: Counter = Counter() parser = argparse.ArgumentParser(description="Run tsc as a typechecker with an allowlist of errors for JavaScript and TypeScript files") parser.add_argument("--stats", action="store_true", help="Display stats of the most common TypeScript errors in JavaScript files") args = parser.parse_args() From 0e7058d130ba9e177cee475ce2d2d90fb6492f72 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:18:19 +0100 Subject: [PATCH 03/12] bridge: Sort imports Fixes ruff I001 with ruff 0.6.7. --- src/cockpit/polyfills.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cockpit/polyfills.py b/src/cockpit/polyfills.py index d226c8ebce1d..434c77dade0f 100644 --- a/src/cockpit/polyfills.py +++ b/src/cockpit/polyfills.py @@ -24,9 +24,8 @@ def install(): # introduced in 3.9 if not hasattr(socket, 'recv_fds'): - import array - import _socket + import array def recv_fds(sock, bufsize, maxfds, flags=0): fds = array.array("i") From 9c8c7fb048183873c63599d3841554c375a3dc95 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:32:50 +0100 Subject: [PATCH 04/12] bridge: Silence B901 ruff warning > Using `yield` and `return {value}` in a generator function can lead to confusing behavior But that's the expected GeneratorChannel API (it looks at `StopIteration.value`), and also tricky to avoid. --- src/cockpit/channels/filesystem.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cockpit/channels/filesystem.py b/src/cockpit/channels/filesystem.py index 7d7afdbf6c59..e6c07babe9e0 100644 --- a/src/cockpit/channels/filesystem.py +++ b/src/cockpit/channels/filesystem.py @@ -151,7 +151,8 @@ def do_yield_data(self, options: JsonObject) -> Generator[bytes, None, JsonObjec return {'tag': tag_from_stat(buf)} except FileNotFoundError: - return {'tag': '-'} + # Using `yield` and `return {value}` generator, but GeneratorChannel does expect this + return {'tag': '-'} # noqa: B901 except PermissionError as exc: raise ChannelError('access-denied') from exc except OSError as exc: From 3d02bdad1e0ffe1e2512175a345f8dbc09808423 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:35:36 +0100 Subject: [PATCH 05/12] bridge: Rename channels/http.py A005 Module `http` is shadowing a Python builtin module --- src/cockpit/channels/__init__.py | 2 +- src/cockpit/channels/{http.py => http_channel.py} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/cockpit/channels/{http.py => http_channel.py} (100%) diff --git a/src/cockpit/channels/__init__.py b/src/cockpit/channels/__init__.py index 14ff6442cf64..5771e2f59ce8 100644 --- a/src/cockpit/channels/__init__.py +++ b/src/cockpit/channels/__init__.py @@ -17,7 +17,7 @@ from .dbus import DBusChannel from .filesystem import FsInfoChannel, FsListChannel, FsReadChannel, FsReplaceChannel, FsWatchChannel -from .http import HttpChannel +from .http_channel import HttpChannel from .metrics import InternalMetricsChannel from .packages import PackagesChannel from .pcp import PcpMetricsChannel diff --git a/src/cockpit/channels/http.py b/src/cockpit/channels/http_channel.py similarity index 100% rename from src/cockpit/channels/http.py rename to src/cockpit/channels/http_channel.py From f6354a1079eadc2eb5da754e519b8959a5d44a72 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:39:45 +0100 Subject: [PATCH 06/12] bridge: Fix typing of filesystem tag API Precisely type the various `tag_from_*()` helpers and the `_tag` member variable. Fix return type of `set_contents()` -- `tag_from_path()` can return None, so this function can, too. Fixes error with mypy 1.13.0: > Incompatible types in assignment (expression has type "Any | str", variable has type "None") [assignment] --- src/cockpit/channels/filesystem.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cockpit/channels/filesystem.py b/src/cockpit/channels/filesystem.py index e6c07babe9e0..24a54cbc7d3e 100644 --- a/src/cockpit/channels/filesystem.py +++ b/src/cockpit/channels/filesystem.py @@ -57,11 +57,11 @@ def my_umask() -> int: return (match and int(match.group(1), 8)) or 0o077 -def tag_from_stat(buf): +def tag_from_stat(buf) -> str: return f'1:{buf.st_ino}-{buf.st_mtime}-{buf.st_mode:o}-{buf.st_uid}-{buf.st_gid}' -def tag_from_path(path): +def tag_from_path(path) -> 'str | None': try: return tag_from_stat(os.stat(path)) except FileNotFoundError: @@ -70,7 +70,7 @@ def tag_from_path(path): return None -def tag_from_fd(fd): +def tag_from_fd(fd) -> 'str | None': try: return tag_from_stat(os.fstat(fd)) except OSError: @@ -169,7 +169,9 @@ def delete(self, path: str, tag: 'str | None') -> str: os.unlink(path) return '-' - async def set_contents(self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None') -> str: + async def set_contents( + self, path: str, tag: 'str | None', data: 'bytes | None', size: 'int | None' + ) -> 'str | None': dirname, basename = os.path.split(path) tmpname: str | None fd, tmpname = tempfile.mkstemp(dir=dirname, prefix=f'.{basename}-') @@ -262,7 +264,7 @@ async def run(self, options: JsonObject) -> JsonObject: class FsWatchChannel(Channel, PathWatchListener): payload = 'fswatch1' - _tag = None + _tag: 'str | None' = None _watch = None # The C bridge doesn't send the initial event, and the JS calls read() From a493546732606a709abaa0886f2dc20f95c536f9 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:45:21 +0100 Subject: [PATCH 07/12] build_backend: Adjust type ignore for mypy 1.13.0 The error name changed: > No overload variant of "open" matches argument types "GzipFile", "str", "bool" [call-overload] So accept both the old and new spelling for the time being. --- src/build_backend.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/build_backend.py b/src/build_backend.py index 4b5f5f249cf7..1572d6b3019a 100644 --- a/src/build_backend.py +++ b/src/build_backend.py @@ -49,7 +49,7 @@ def build_sdist(sdist_directory: str, # We do this manually to avoid adding timestamps. See https://bugs.python.org/issue31526 with gzip.GzipFile(f'{sdist_directory}/{sdist_filename}', mode='w', mtime=0) as gz: # https://github.com/python/typeshed/issues/5491 - with tarfile.open(fileobj=gz, mode='w|', dereference=True) as sdist: # type: ignore[arg-type] + with tarfile.open(fileobj=gz, mode='w|', dereference=True) as sdist: # type: ignore[arg-type,call-overload] for filename in sorted(find_sources(srcpkg=True)): sdist.add(filename, arcname=f'{PACKAGE}/{filename}', ) return sdist_filename From b2a7a1d052876872d382877c3e9ebdffc1c44e7b Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 09:58:49 +0100 Subject: [PATCH 08/12] Use raw strings with `re` functions Fixes lots of complaints with ruff 0.8 like > RUF039 First argument to `re.match()` is not raw string --- containers/flatpak/add-release | 2 +- pkg/storaged/btrfs/btrfs-tool.py | 8 ++++---- pkg/storaged/crypto/luksmeta-monitor-hack.py | 6 +++--- pkg/storaged/nfs/nfs-mounts.py | 8 ++++---- src/cockpit/packages.py | 2 +- test/common/lcov.py | 2 +- test/common/netlib.py | 2 +- test/common/storagelib.py | 2 +- test/common/typecheck | 2 +- test/verify/check-shell-multi-machine-key | 2 +- test/verify/check-system-info | 4 ++-- test/verify/check-system-realms | 2 +- test/verify/check-testlib | 7 +++++-- 13 files changed, 26 insertions(+), 23 deletions(-) diff --git a/containers/flatpak/add-release b/containers/flatpak/add-release index aee1f72c7767..a3c1424ec642 100755 --- a/containers/flatpak/add-release +++ b/containers/flatpak/add-release @@ -15,7 +15,7 @@ def element(tag, text=None, children=(), **kwargs): def points_from_body(body): - for text in re.split('^ *-', body, flags=re.MULTILINE): + for text in re.split(r'^ *-', body, flags=re.MULTILINE): if point := ' '.join(text.split()).strip(): yield element('li', point) diff --git a/pkg/storaged/btrfs/btrfs-tool.py b/pkg/storaged/btrfs/btrfs-tool.py index 50ca74ed024a..1ddf96d17aa7 100755 --- a/pkg/storaged/btrfs/btrfs-tool.py +++ b/pkg/storaged/btrfs/btrfs-tool.py @@ -156,7 +156,7 @@ def get_subvolume_info(mp): lines = subprocess.check_output(["btrfs", "subvolume", "list", "-apuq", mp]).splitlines() subvols = [] for line in lines: - match = re.match(b"ID (\\d+).*parent (\\d+).*parent_uuid (.*)uuid (.*) path (/)?(.*)", line) + match = re.match(rb"ID (\d+).*parent (\d+).*parent_uuid (.*)uuid (.*) path (/)?(.*)", line) if match: pathname = match[6].decode(errors='replace') # Ignore podman btrfs subvolumes, they are an implementation detail. @@ -169,13 +169,13 @@ def get_subvolume_info(mp): 'uuid': match[4].decode(), 'parent_uuid': None if match[3][0] == ord("-") else match[3].decode().strip() } - ] + ] return subvols def get_default_subvolume(mp): output = subprocess.check_output(["btrfs", "subvolume", "get-default", mp]) - match = re.match(b"ID (\\d+).*", output) + match = re.match(rb"ID (\d+).*", output) if match: return int(match[1]) else: @@ -186,7 +186,7 @@ def get_usages(uuid): output = subprocess.check_output(["btrfs", "filesystem", "show", "--raw", uuid]) usages = {} for line in output.splitlines(): - match = re.match(b".*used\\s+(\\d+)\\s+path\\s+([\\w/]+).*", line) + match = re.match(rb".*used\s+(\d+)\s+path\s+([\w/]+).*", line) if match: usages[match[2].decode()] = int(match[1]) return usages diff --git a/pkg/storaged/crypto/luksmeta-monitor-hack.py b/pkg/storaged/crypto/luksmeta-monitor-hack.py index a55c4e9ce407..ec521e981b93 100755 --- a/pkg/storaged/crypto/luksmeta-monitor-hack.py +++ b/pkg/storaged/crypto/luksmeta-monitor-hack.py @@ -73,9 +73,9 @@ def info(dev): in_luks2_token_section = True if in_luks2_slot_section: - match = re.match(b" ([0-9]+): luks2$", line) + match = re.match(rb" ([0-9]+): luks2$", line) else: - match = re.match(b"Key Slot ([0-9]+): ENABLED$", line) + match = re.match(rb"Key Slot ([0-9]+): ENABLED$", line) if match: slot = int(match.group(1)) entry = {"Index": {"v": slot}} @@ -93,7 +93,7 @@ def info(dev): slots[slot] = entry if in_luks2_token_section: - match = re.match(b" ([0-9]+): clevis$", line) + match = re.match(rb" ([0-9]+): clevis$", line) if match: try: token = subprocess.check_output(["cryptsetup", "token", "export", diff --git a/pkg/storaged/nfs/nfs-mounts.py b/pkg/storaged/nfs/nfs-mounts.py index e00fc75671d0..a6e74f6b4f08 100755 --- a/pkg/storaged/nfs/nfs-mounts.py +++ b/pkg/storaged/nfs/nfs-mounts.py @@ -36,11 +36,11 @@ def event(_wd, mask, name): def field_escape(field): - return field.replace("\\", "\\134").replace(" ", "\\040").replace("\t", "\\011") + return field.replace("\\", r"\134").replace(" ", r"\040").replace("\t", r"\011") def field_unescape(field): - return re.sub("\\\\([0-7]{1,3})", lambda m: chr(int(m.group(1), 8)), field) + return re.sub(r"\\([0-7]{1,3})", lambda m: chr(int(m.group(1), 8)), field) def parse_tab(name): @@ -50,7 +50,7 @@ def parse_tab(name): sline = line.strip() if sline == "" or sline[0] == "#": continue - fields = list(map(field_unescape, re.split("[ \t]+", sline))) + fields = list(map(field_unescape, re.split(r"[ \t]+", sline))) if len(fields) > 2 and ":" in fields[0] and fields[2].startswith("nfs"): entries.append(fields) return entries @@ -75,7 +75,7 @@ def modify_tab(name, modify): if sline == "" or sline[0] == "#": new_lines.append(line) else: - fields = list(map(field_unescape, re.split("[ \t]+", sline))) + fields = list(map(field_unescape, re.split(r"[ \t]+", sline))) if len(fields) > 0 and ":" in fields[0]: new_fields = modify(fields) if new_fields: diff --git a/src/cockpit/packages.py b/src/cockpit/packages.py index c9f2525f2ab4..13fbe3e83967 100644 --- a/src/cockpit/packages.py +++ b/src/cockpit/packages.py @@ -271,7 +271,7 @@ def ensure_scanned(self) -> None: self.translations[f'{basename}.js'][lower_locale] = name else: # strip out trailing '.gz' components - basename = re.sub('.gz$', '', name) + basename = re.sub(r'.gz$', '', name) logger.debug('Adding content %r -> %r', basename, name) self.files[basename] = name diff --git a/test/common/lcov.py b/test/common/lcov.py index 990c3287bd9e..ba6e052068d2 100755 --- a/test/common/lcov.py +++ b/test/common/lcov.py @@ -406,7 +406,7 @@ def is_interesting_line(text): # Don't complain about lines that contain only punctuation, or # nothing but "else". We don't seem to get reliable # information for them. - if not re.search('[a-zA-Z0-9]', text.replace("else", "")): + if not re.search(r'[a-zA-Z0-9]', text.replace("else", "")): return False return True diff --git a/test/common/netlib.py b/test/common/netlib.py index 053830211d0a..3a4b10521ec3 100644 --- a/test/common/netlib.py +++ b/test/common/netlib.py @@ -120,7 +120,7 @@ def cleanupDevs(): ver = self.machine.execute( "busctl --system get-property org.freedesktop.NetworkManager /org/freedesktop/NetworkManager org.freedesktop.NetworkManager Version || true") - ver_match = re.match('s "(.*)"', ver) + ver_match = re.match(r's "(.*)"', ver) if ver_match: self.networkmanager_version = [int(x) for x in ver_match.group(1).split(".")] else: diff --git a/test/common/storagelib.py b/test/common/storagelib.py index aa3c41f4f51d..328e06caf091 100644 --- a/test/common/storagelib.py +++ b/test/common/storagelib.py @@ -660,7 +660,7 @@ def setUp(self): super().setUp() ver = self.machine.execute("busctl --system get-property org.freedesktop.UDisks2 /org/freedesktop/UDisks2/Manager org.freedesktop.UDisks2.Manager Version || true") - m = re.match('s "(.*)"', ver) + m = re.match(r's "(.*)"', ver) if m: self.storaged_version = list(map(int, m.group(1).split("."))) else: diff --git a/test/common/typecheck b/test/common/typecheck index ea684800f3c2..b61b9c2047f0 100755 --- a/test/common/typecheck +++ b/test/common/typecheck @@ -144,7 +144,7 @@ def show_error(lines): def consider(lines, js_error_codes): - m = re.match("([^:]*)\\(.*\\): error ([^:]*): .*", lines[0]) + m = re.match(r"([^:]*)\(.*\): error ([^:]*): .*", lines[0]) if m and not should_ignore(m[1]): relaxed = is_relaxed(m[1]) is_js = m[1].endswith(('.js', '.jsx')) diff --git a/test/verify/check-shell-multi-machine-key b/test/verify/check-shell-multi-machine-key index 3acb385c0669..b8ece3df37a0 100755 --- a/test/verify/check-shell-multi-machine-key +++ b/test/verify/check-shell-multi-machine-key @@ -89,7 +89,7 @@ class TestMultiMachineKeyAuth(testlib.MachineCase): def check_keys(self, keys_md5, keys): def normalize(k): - return re.sub("/home/admin/\\.ssh/[^ ]*|test@test|ecdsa w/o comment", "", k) + return re.sub(r"/home/admin/\.ssh/[^ ]*|test@test|ecdsa w/o comment", "", k) self.assertIn(normalize(self.browser.eval_js("cockpit.spawn([ 'ssh-add', '-l' ])")), [normalize("\n".join(keys_md5) + "\n"), normalize("\n".join(keys) + "\n")]) diff --git a/test/verify/check-system-info b/test/verify/check-system-info index dc4780363d17..43071fc9d9a9 100755 --- a/test/verify/check-system-info +++ b/test/verify/check-system-info @@ -61,7 +61,7 @@ class TestSystemInfo(testlib.MachineCase): # Most OSes don't set nosmt by default, but there are some exceptions self.expect_smt_default = self.machine.image in ["fedora-coreos"] - self.supportsFIPS = re.match('fedora|((centos|rhel)-(8|9)).*', self.machine.image) + self.supportsFIPS = re.match(r'fedora|((centos|rhel)-(8|9)).*', self.machine.image) # HACK: temporary: we are in the middle of transitioning centos/rhel-10 images away from fips-mode-setup; # until https://github.com/cockpit-project/bots/pull/7095 and https://github.com/cockpit-project/bots/pull/7091 # land, do runtime detection @@ -810,7 +810,7 @@ password=foobar # RHEL 8/10 have no SHA1 policy, so do not show it. b.click("#crypto-policy-button") - func = b.wait_not_present if re.match('(centos|rhel)-(8|10).*', m.image) else b.wait_visible + func = b.wait_not_present if re.match(r'(centos|rhel)-(8|10).*', m.image) else b.wait_visible func(".pf-v5-c-menu__item-main .pf-v5-c-menu__item-text:contains('DEFAULT:SHA1')") b.click("#crypto-policy-dialog button:contains('Cancel')") b.wait_not_present("#crypto-policy-dialog") diff --git a/test/verify/check-system-realms b/test/verify/check-system-realms index 00b5b134adba..6cdd34d0a207 100755 --- a/test/verify/check-system-realms +++ b/test/verify/check-system-realms @@ -1054,7 +1054,7 @@ class TestKerberos(testlib.MachineCase): self.assertIn("HTTP/1.1 200 OK", output) self.assertIn('"csrf-token"', output) - cookie = re.search("Set-Cookie: cockpit=([^ ;]+)", output).group(1) + cookie = re.search(r"Set-Cookie: cockpit=([^ ;]+)", output).group(1) b.open("/system/terminal", cookie={"name": "cockpit", "value": cookie, "domain": m.web_address, "path": "/"}) b.wait_visible('#content') diff --git a/test/verify/check-testlib b/test/verify/check-testlib index 34aa7bfd71f2..c3ef0b1adb8b 100755 --- a/test/verify/check-testlib +++ b/test/verify/check-testlib @@ -55,8 +55,11 @@ class TestRunTestListing(unittest.TestCase): self.assertIn(b"TestNondestructiveExample.testOne\nTestNondestructiveExample.testTwo", ndtests.stdout) # nondestructive tests are sorted alphabetically - verify_ndtests = subprocess.run([run_tests, "--test-dir", VERIFY_DIR, "-n", "-l"], check=True, capture_output=True) - self.assertRegex(verify_ndtests.stdout, re.compile(b".*TestAccounts.*TestFirewall.*TestLogin.*TestServices.*TestTerminal.*", re.S)) + verify_ndtests = subprocess.run([run_tests, "--test-dir", VERIFY_DIR, "-n", "-l"], + check=True, capture_output=True) + self.assertRegex( + verify_ndtests.stdout, + re.compile(rb".*TestAccounts.*TestFirewall.*TestLogin.*TestServices.*TestTerminal.*", re.S)) def testNonDestructive(self): self.assertEqual(subprocess.check_output([run_tests, "--test-dir", EXAMPLE_DIR, "--nondestructive", "-l", "TestExample"]).strip().decode(), From d71202ed28d071114028f9eadb9f702b5b6a6804 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 10:02:49 +0100 Subject: [PATCH 09/12] Don't mark used function arguments as dummy Fixes lots of issues with ruff 0.8: > RUF052 [*] Local dummy variable `_msg` is accessed This is right -- all these `_msg` are actually being used. Use `type_` and `class_`, follow ruff's recommendation: > help: Prefer using trailing underscores to avoid shadowing a built-in --- src/cockpit/channel.py | 4 ++-- src/cockpit/protocol.py | 10 +++++----- src/cockpit/router.py | 12 ++++++------ test/common/testlib.py | 4 ++-- test/pytest/mocktransport.py | 4 ++-- test/verify/check-packagekit | 12 ++++++------ 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/cockpit/channel.py b/src/cockpit/channel.py index 6f469fe0de6a..7dcabefdd250 100644 --- a/src/cockpit/channel.py +++ b/src/cockpit/channel.py @@ -327,8 +327,8 @@ def send_text(self, data: str) -> bool: """ return self.send_bytes(data.encode()) - def send_json(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> bool: - pretty = self.json_encoder.encode(create_object(_msg, kwargs)) + '\n' + def send_json(self, msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> bool: + pretty = self.json_encoder.encode(create_object(msg, kwargs)) + '\n' return self.send_text(pretty) def do_pong(self, message): diff --git a/src/cockpit/protocol.py b/src/cockpit/protocol.py index 6671878b9f8c..e2df4275c446 100644 --- a/src/cockpit/protocol.py +++ b/src/cockpit/protocol.py @@ -39,9 +39,9 @@ class CockpitProblem(Exception): """ attrs: JsonObject - def __init__(self, problem: str, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: + def __init__(self, problem: str, msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: kwargs['problem'] = problem - self.attrs = create_object(_msg, kwargs) + self.attrs = create_object(msg, kwargs) super().__init__(get_str(self.attrs, 'message', problem)) def get_attrs(self) -> JsonObject: @@ -179,10 +179,10 @@ def write_channel_data(self, channel: str, payload: bytes) -> None: else: logger.debug('cannot write to closed transport') - def write_control(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: + def write_control(self, msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: """Write a control message. See jsonutil.create_object() for details.""" - logger.debug('sending control message %r %r', _msg, kwargs) - pretty = json.dumps(create_object(_msg, kwargs), indent=2) + '\n' + logger.debug('sending control message %r %r', msg, kwargs) + pretty = json.dumps(create_object(msg, kwargs), indent=2) + '\n' self.write_channel_data('', pretty.encode()) def data_received(self, data: bytes) -> None: diff --git a/src/cockpit/router.py b/src/cockpit/router.py index dda836fc1011..50429c463c5a 100644 --- a/src/cockpit/router.py +++ b/src/cockpit/router.py @@ -94,15 +94,15 @@ def send_channel_data(self, channel: str, data: bytes) -> None: self.router.write_channel_data(channel, data) def send_channel_control( - self, channel: str, command: str, _msg: 'JsonObject | None', **kwargs: JsonValue + self, channel: str, command: str, msg: 'JsonObject | None', **kwargs: JsonValue ) -> None: - self.router.write_control(_msg, channel=channel, command=command, **kwargs) + self.router.write_control(msg, channel=channel, command=command, **kwargs) if command == 'close': self.router.endpoints[self].remove(channel) self.router.drop_channel(channel) - def shutdown_endpoint(self, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: - self.router.shutdown_endpoint(self, _msg, **kwargs) + def shutdown_endpoint(self, msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: + self.router.shutdown_endpoint(self, msg, **kwargs) class RoutingError(CockpitProblem): @@ -168,11 +168,11 @@ def add_endpoint(self, endpoint: Endpoint) -> None: self.endpoints[endpoint] = set() self.no_endpoints.clear() - def shutdown_endpoint(self, endpoint: Endpoint, _msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: + def shutdown_endpoint(self, endpoint: Endpoint, msg: 'JsonObject | None' = None, **kwargs: JsonValue) -> None: channels = self.endpoints.pop(endpoint) logger.debug('shutdown_endpoint(%s, %s) will close %s', endpoint, kwargs, channels) for channel in channels: - self.write_control(_msg, command='close', channel=channel, **kwargs) + self.write_control(msg, command='close', channel=channel, **kwargs) self.drop_channel(channel) if not self.endpoints: diff --git a/test/common/testlib.py b/test/common/testlib.py index 3c7f25bb31b5..c3334ad5941c 100644 --- a/test/common/testlib.py +++ b/test/common/testlib.py @@ -2489,13 +2489,13 @@ def jsquote(js: object) -> str: return json.dumps(js) -def get_decorator(method: object, _class: object, name: str, default: Any = None) -> Any: +def get_decorator(method: object, class_: object, name: str, default: Any = None) -> Any: """Get decorator value of a test method or its class Return None if the decorator was not set. """ attr = "_testlib__" + name - return getattr(method, attr, getattr(_class, attr, default)) + return getattr(method, attr, getattr(class_, attr, default)) ########################### diff --git a/test/pytest/mocktransport.py b/test/pytest/mocktransport.py index ed2c6f7219ca..dc454c407ba8 100644 --- a/test/pytest/mocktransport.py +++ b/test/pytest/mocktransport.py @@ -17,10 +17,10 @@ async def assert_empty(self): await asyncio.sleep(0.1) assert self.queue.qsize() == 0 - def send_json(self, _channel: str, **kwargs) -> None: + def send_json(self, channel_: str, **kwargs) -> None: # max_read_size is one of our special keys which uses underscores msg = {k.replace('_', '-') if k != "max_read_size" else k: v for k, v in kwargs.items()} - self.send_data(_channel, json.dumps(msg).encode('ascii')) + self.send_data(channel_, json.dumps(msg).encode('ascii')) def send_data(self, channel: str, data: bytes) -> None: msg = channel.encode('ascii') + b'\n' + data diff --git a/test/verify/check-packagekit b/test/verify/check-packagekit index 46ad1be4297f..861fbd0d82d0 100755 --- a/test/verify/check-packagekit +++ b/test/verify/check-packagekit @@ -1425,19 +1425,19 @@ class TestAutoUpdates(NoSubManCase): else: raise NotImplementedError(self.backend) - def assertTypeDnf(_type): - if _type == "all": + def assertTypeDnf(type_): + if type_ == "all": match = '= default' - elif _type == "security": + elif type_ == "security": match = '= security' else: - raise ValueError(_type) + raise ValueError(type_) self.assertIn(match, m.execute(f"grep upgrade_type {self.config}")) - def assertType(_type): + def assertType(type_): if "dnf" in self.backend: - assertTypeDnf(_type) + assertTypeDnf(type_) else: raise NotImplementedError(self.backend) From 752289b6aa33cf5770a200e3a0f491f25d11ec3f Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 13:36:28 +0100 Subject: [PATCH 10/12] test: Skip TestPages.testPageStatus on chromium Our synthetic mouse events on latest Chromium 131 are sometimes missed. It works reliably with TEST_SHOW_BROWSER and with the real mouse events in Firefox. Given the resounding silence of our other chromium bug reports, let's just skip that test there. --- test/verify/check-pages | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/verify/check-pages b/test/verify/check-pages index 9f57345fa63a..e115e8f8dd64 100755 --- a/test/verify/check-pages +++ b/test/verify/check-pages @@ -752,6 +752,7 @@ OnCalendar=daily with b.wait_timeout(60): b.wait_js_func("ph_plot_data_plateau", "pmcd", mem_avail * 0.85, mem_avail * 1.15, 15, "mem") + @testlib.skipBrowser("Headless chromium is missing the synthetic mouseenter", "chromium") def testPageStatus(self): b = self.browser @@ -766,6 +767,7 @@ OnCalendar=daily b.mouse("#development-info", "mouseenter") b.wait_in_text(".pf-v5-c-tooltip", "My Little Page Status") b.mouse("#development-info", "mouseleave") + b.wait_not_present(".pf-v5-c-tooltip") b.go("/playground/notifications-receiver") b.enter_page("/playground/notifications-receiver") From 3ffafffac576cd9cb13ffe405b2aeb35a13e4b1f Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 9 Dec 2024 16:32:01 +0100 Subject: [PATCH 11/12] test: Avoid focused host selector for pixel test TestHostSwitching.testBasic does pixel tests of the host selector menu, which is the top left element at position (0, 0). Current Chromium changes behaviour to always focus that element, and it's fairly stubborn. To avoid pixel testing the focused (light-gray) variant, move the mouse away first. --- test/verify/check-shell-host-switching | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/verify/check-shell-host-switching b/test/verify/check-shell-host-switching index 94e029277457..db4bf02186bf 100755 --- a/test/verify/check-shell-host-switching +++ b/test/verify/check-shell-host-switching @@ -183,6 +183,14 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers): b.wait_not_present("#nav-system.interact") b.set_layout("desktop") + # this looks different (focused) when the mouse is inside (default 0,0 possition), so move it away + # chromium is stubborn here, we *have* to move the real mouse, not just do this with synthetic MouseEvents + b.bidi("input.performActions", context=b.driver.context, actions=[{ + "id": "move-away", + "type": "pointer", + "parameters": {"pointerType": "mouse"}, + "actions": [{"type": "pointerMove", "x": 200, "y": 200, "origin": "viewport"}] + }]) b.assert_pixels("#hosts-sel", "hosts-sel-closed") b.click("#hosts-sel button") From 16e7ee0272204cc68881451b1970184b44525dda Mon Sep 17 00:00:00 2001 From: Cockpit Project Date: Mon, 9 Dec 2024 07:52:12 +0000 Subject: [PATCH 12/12] cockpit-ci: Update container to 2024-12-08 Adjust pixel tests for subtle font rendering changes with updated Chromium. Closes #21410 --- .cockpit-ci/container | 2 +- test/reference | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.cockpit-ci/container b/.cockpit-ci/container index d0a4acece11d..ef8adaf82620 100644 --- a/.cockpit-ci/container +++ b/.cockpit-ci/container @@ -1 +1 @@ -ghcr.io/cockpit-project/tasks:2024-10-07 +ghcr.io/cockpit-project/tasks:2024-12-08 diff --git a/test/reference b/test/reference index 1f17d8f6d2b3..3bf71fee9f64 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit 1f17d8f6d2b39be2c9f19771004fe27c52c1bbce +Subproject commit 3bf71fee9f64c84b8feb99b02c88108e114676db