diff --git a/tests/quickshell/control-center-contract.sh b/tests/quickshell/control-center-contract.sh index b1ae077..0627b7f 100755 --- a/tests/quickshell/control-center-contract.sh +++ b/tests/quickshell/control-center-contract.sh @@ -69,7 +69,13 @@ rg -Fq 'readonly property int controlCenterWidth: 430' \ rg -Fq 'readonly property int controlCenterTopGap: 2' \ "$source_config_path/config/Theme.qml" \ || fail 'approved top attachment is missing' -rg -Fq 'margins.top: Theme.barHeight + Theme.controlCenterTopGap' \ +# The margin is the GAP alone. This used to assert the bar height plus the gap, +# which is what the code said and what made the panel open 38 pixels below a bar +# it was written to sit 2 pixels under: an exclusiveZone of 0 already places the +# surface below the bar's reserved space, so naming the bar height again counted +# it twice. The contract agreed with the code and so the bug was invisible to +# both. layer-margin-contract now holds that rule for every surface. +rg -Fq 'margins.top: Theme.controlCenterTopGap' \ "$quicksettings_path/QuickSettings.qml" \ || fail 'Control Center is not tightly attached to the bar' rg -Fq 'implicitWidth: Theme.controlCenterWidth' \ diff --git a/tests/quickshell/settings-page-registry-contract.sh b/tests/quickshell/settings-page-registry-contract.sh deleted file mode 100755 index 0185135..0000000 --- a/tests/quickshell/settings-page-registry-contract.sh +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/env bash - -# A settings page is declared in three places that cannot see each other: -# -# SettingsSidebar.qml the list someone clicks -# SettingsShell.qml the switch that decides which component to build -# ShellState.qml the allow-list that IPC and the launcher go through -# -# Miss one and the failure is silent in the worst way. A page missing from the -# allow-list does not error -- openSettings() falls back to "home", so the -# launcher command opens Settings on the wrong page and logs nothing. That is -# exactly what happened when Storage was added. -# -# Static and read-only; it parses three files. - -set -uo pipefail - -repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -shell_dir="$repo_dir/config/dot/quickshell" -sidebar="$shell_dir/modules/settings/SettingsSidebar.qml" -shell_file="$shell_dir/modules/settings/SettingsShell.qml" -state="$shell_dir/services/ShellState.qml" - -fail() { - printf 'settings page registry contract: %s\n' "$1" >&2 - exit 1 -} - -for path in "$sidebar" "$shell_file" "$state"; do - [[ -r "$path" ]] || fail "missing $path" -done - -listed="$(grep -oE '\{ page: "[a-z-]+"' "$sidebar" | sed 's/.*"\([a-z-]*\)"/\1/' | sort -u)" -routed="$(grep -oE 'case "[a-z-]+": return [a-zA-Z]+Page;' "$shell_file" \ - | sed 's/case "\([a-z-]*\)".*/\1/' | sort -u)" -allowed="$(sed -n 's/.*const allowed = \[\(.*\)\];/\1/p' "$state" \ - | tr ',' '\n' | tr -d ' "' | grep -v '^$' | sort -u)" - -[[ -n "$listed" ]] || fail 'no pages found in the sidebar' -[[ -n "$routed" ]] || fail 'no pages found in the shell switch' -[[ -n "$allowed" ]] || fail 'no allow-list found in ShellState' - -# "home" is the fallback: it is routed by `default:` rather than a case, so it -# is legitimately absent from the switch. -routed="$(printf '%s\nhome\n' "$routed" | sort -u)" - -missing_route="$(comm -23 <(printf '%s\n' "$listed") <(printf '%s\n' "$routed") | tr '\n' ' ')" -[[ -z "${missing_route// }" ]] \ - || fail "these pages are in the sidebar but the shell has no case for them, so they render as Home: $missing_route" - -missing_allow="$(comm -23 <(printf '%s\n' "$listed") <(printf '%s\n' "$allowed") | tr '\n' ' ')" -[[ -z "${missing_allow// }" ]] \ - || fail "these pages are in the sidebar but not in ShellState's allow-list, so opening them by IPC silently lands on Home: $missing_allow" - -orphan_allow="$(comm -13 <(printf '%s\n' "$listed") <(printf '%s\n' "$allowed") | tr '\n' ' ')" -[[ -z "${orphan_allow// }" ]] \ - || fail "ShellState allows pages the sidebar does not have: $orphan_allow" - -printf 'settings page registry contract: PASS (%d pages agree across three files)\n' \ - "$(grep -c . <<<"$listed")" diff --git a/tests/quickshell/settings-write-sweep-contract.sh b/tests/quickshell/settings-write-sweep-contract.sh new file mode 100755 index 0000000..46b352b --- /dev/null +++ b/tests/quickshell/settings-write-sweep-contract.sh @@ -0,0 +1,105 @@ +#!/usr/bin/env bash + +# Every compositor-backed setting must actually change the compositor. +# +# settings-hyprland-write-contract proves this for five settings. There are +# sixty-six, and the ones it does not reach are exactly where a dead setting +# hides: the write path no-ops, nothing fails, nothing logs, and the row simply +# does not do anything. That is what a user reports as "the settings app does +# not work", and it is not visible from any shape-checking test. +# +# Each setting is driven through SystemSettings.commitPreference -- the entry +# point a settings row uses -- flipped to a value it does not hold, read back +# from the live compositor, and put straight back before the next one is +# touched. +# +# This one talks to the running compositor on purpose. Preferences are written +# to an isolated config home so nothing lands in the real settings.json. + +set -uo pipefail + +repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +harness="$repo_dir/config/dot/quickshell/settings-system-harness.qml" +sweep="$repo_dir/tests/quickshell/settings_write_sweep.py" + +fail() { + printf 'settings write sweep contract: %s\n' "$1" >&2 + exit 1 +} + +[[ -r "$harness" ]] || fail 'the settings system harness is missing' +[[ -r "$sweep" ]] || fail 'the sweep is missing' + +command -v qs >/dev/null 2>&1 || { printf 'settings write sweep contract: SKIP (no quickshell)\n'; exit 0; } +command -v hyprctl >/dev/null 2>&1 || { printf 'settings write sweep contract: SKIP (no compositor)\n'; exit 0; } +command -v jq >/dev/null 2>&1 || { printf 'settings write sweep contract: SKIP (no jq)\n'; exit 0; } +hyprctl -j getoption decoration:rounding >/dev/null 2>&1 \ + || { printf 'settings write sweep contract: SKIP (compositor not answering)\n'; exit 0; } + +config_home="$(mktemp -d /tmp/panama-write-sweep.XXXXXX)" + +qs_for_harness() { + XDG_CONFIG_HOME="$config_home" qs -p "$harness" "$@" +} + +cleanup() { + qs_for_harness kill >/dev/null 2>&1 || true + rm -rf "$config_home" +} +trap cleanup EXIT + +# The sibling contract drives the SAME harness file with the compositor seam +# stubbed. Quickshell identifies an instance by config path, so an instance left +# over from that run would answer here and every write would miss the compositor +# entirely -- passing for the worst possible reason. +if qs_for_harness ipc show 2>/dev/null | grep -q '^target settings-system-test$'; then + fail 'a harness instance is already running; a stale one would answer these writes instead of the compositor' +fi + +XDG_CONFIG_HOME="$config_home" PANAMA_SETTINGS_TEST_ISOLATE_COMPOSITOR=0 \ + qs -p "$harness" --daemonize >/dev/null 2>&1 + +for _ in $(seq 1 40); do + qs_for_harness ipc show 2>/dev/null | grep -q '^target settings-system-test$' && break + sleep 0.25 +done +qs_for_harness ipc show 2>/dev/null | grep -q '^target settings-system-test$' \ + || fail 'the harness did not start' + +result="$(cd "$repo_dir" && XDG_CONFIG_HOME="$config_home" \ + timeout 600 python3 "$sweep" "$config_home" "$harness")" \ + || fail 'the sweep did not finish' + +verified="$(jq -r '.verified | length' <<<"$result")" +skipped="$(jq -r '.skipped | length' <<<"$result")" +failed="$(jq -r '.failures | length' <<<"$result")" +total="$(jq -r '.total' <<<"$result")" + +if [[ "$failed" != "0" ]]; then + printf 'settings write sweep contract: %s setting(s) did not reach the compositor:\n' "$failed" >&2 + jq -r '.failures[] | " \(.[0]): \(.[1])"' <<<"$result" >&2 + exit 1 +fi + +# Skipping is legitimate for a shape with no scalar to compare, but a sweep that +# skips most of what it was pointed at is not evidence of anything. +if (( verified * 2 < total )); then + jq -r '.skipped[] | " \(.[0]): \(.[1])"' <<<"$result" >&2 + fail "only $verified of $total settings were actually exercised" +fi + +# ── Settings Panama stores itself ─────────────────────────────────────────── +# No compositor to ask, so the question is whether the value comes back out of +# the store. A setting that silently fails to persist is the same dead row. +local_failed="$(jq -r '.localFailures | length' <<<"$result")" +local_verified="$(jq -r '.localVerified | length' <<<"$result")" +local_skipped="$(jq -r '.localSkipped | length' <<<"$result")" + +if [[ "$local_failed" != "0" ]]; then + printf 'settings write sweep contract: %s stored setting(s) did not round-trip:\n' "$local_failed" >&2 + jq -r '.localFailures[] | " \(.[0]): \(.[1])"' <<<"$result" >&2 + exit 1 +fi + +printf 'settings write sweep contract: PASS (%d of %d compositor settings verified live, %d skipped; %d stored settings round-tripped, %d skipped)\n' \ + "$verified" "$total" "$skipped" "$local_verified" "$local_skipped" diff --git a/tests/quickshell/settings_write_sweep.py b/tests/quickshell/settings_write_sweep.py new file mode 100644 index 0000000..f8b396c --- /dev/null +++ b/tests/quickshell/settings_write_sweep.py @@ -0,0 +1,290 @@ +#!/usr/bin/env python3 + +"""Flip every compositor-backed setting, read it back, and put it back. + +The existing write contract does this for five settings. There are sixty-five, +and the ones it does not cover are exactly where a silently-dead setting would +live: nothing fails, nothing logs, the row just does not do anything. + +Each setting is driven through SystemSettings.commitPreference -- the same entry +point a settings row uses -- so this exercises the real routing, the real +compositor write, and the real read-back verification, not a shape. + +Every original value is captured before anything changes and restored at the +end, through hyprctl directly, so a broken write path cannot also break the +restore. +""" + +from __future__ import annotations + +import ast +import json +import os +import re +import subprocess +import sys +import time + +SCHEMA = "config/dot/quickshell/config/PreferenceSchema.qml" + +# Read-back shapes this sweep knows how to compare. A gradient or a vec2 has no +# single scalar to diff, and guessing one would produce false failures; those +# are reported as skipped rather than quietly counted as verified. +COMPARABLE = {"bool", "int", "float", "str"} + +# Settings whose value this must not choose freely. +# +# kb_layout is the one that can lock someone out of their own keyboard if a +# sweep picks a layout they cannot type in, and the existing write contract +# already covers it deliberately. +SKIP_KEYS = {"keyboardLayout", "keyboardVariant", "keyboardOptions", "keyboardModel"} + + +def run(command: list[str], timeout: float = 20.0) -> subprocess.CompletedProcess: + return subprocess.run(command, capture_output=True, text=True, + timeout=timeout, check=False) + + +def local_entries() -> list[dict]: + """User-facing settings that Panama stores itself. + + These never reach the compositor, so the compositor cannot be asked whether + they took. What can be asked is whether the value came back out of the + preference store -- a setting that silently fails to persist is the same + dead row from the user's side. + """ + source = open(SCHEMA, encoding="utf-8").read() + entries = [] + for chunk in source.split("key: ")[1:]: + head = chunk[:1400] + if re.search(r"internal:\s*true", head) or re.search(r"hypr:\s*\{", head): + continue + key = re.match(r'"([A-Za-z0-9_]+)"', chunk) + kind = re.search(r'type:\s*"([a-z]+)"', head) + if not (key and kind) or kind.group(1) == "json": + continue + entry = {"key": key.group(1), "type": kind.group(1)} + for bound in ("min", "max", "step"): + found = re.search(rf"\b{bound}:\s*(-?[0-9.]+)", head) + if found: + entry[bound] = float(found.group(1)) + options = re.search(r"options:\s*\[(.*?)\]", head, re.S) + if options: + entry["values"] = [ + ast.literal_eval(value) if value.lstrip().startswith(("'", '"')) + else int(value) + for value in re.findall(r"value:\s*(\"[^\"]*\"|-?\d+)", options.group(1)) + ] + entries.append(entry) + return entries + + +def schema_entries() -> list[dict]: + """Every user-facing setting that maps onto a compositor option.""" + source = open(SCHEMA, encoding="utf-8").read() + entries = [] + for chunk in source.split("key: ")[1:]: + head = chunk[:1400] + if re.search(r"internal:\s*true", head): + continue + hypr = re.search(r"hypr:\s*\{(.*?)\n\s*\}", head, re.S) + if not hypr: + continue + key = re.match(r'"([A-Za-z0-9_]+)"', chunk) + kind = re.search(r'type:\s*"([a-z]+)"', head) + option = re.search(r'option:\s*"([^"]+)"', hypr.group(1)) + read_as = re.search(r'readAs:\s*"([a-z]+)"', hypr.group(1)) + if not (key and kind and option): + continue + entry = { + "key": key.group(1), + "type": kind.group(1), + "option": option.group(1), + "readAs": read_as.group(1) if read_as else "int", + "invert": bool(re.search(r"invert:\s*true", hypr.group(1))), + } + for bound in ("min", "max", "step"): + found = re.search(rf"\b{bound}:\s*(-?[0-9.]+)", head) + if found: + entry[bound] = float(found.group(1)) + options = re.search(r"options:\s*\[(.*?)\]", head, re.S) + if options: + entry["values"] = [ + ast.literal_eval(value) if value.lstrip().startswith(("'", '"')) + else int(value) + for value in re.findall(r"value:\s*(\"[^\"]*\"|-?\d+)", options.group(1)) + ] + entries.append(entry) + return entries + + +def read_option(entry: dict): + """What the compositor currently holds, in the shape the schema declares.""" + result = run(["hyprctl", "-j", "getoption", entry["option"]]) + if result.returncode != 0: + return None + try: + payload = json.loads(result.stdout) + except json.JSONDecodeError: + return None + field = {"bool": "bool", "int": "int", "float": "float", "str": "str"}.get(entry["readAs"]) + if field is None or field not in payload: + return None + return payload[field] + + +def stored_from_option(entry: dict, raw): + """The value a preference would hold for this compositor reading.""" + if entry["readAs"] == "bool": + value = bool(raw) + return (not value) if entry["invert"] else value + if entry["readAs"] == "int" and entry["type"] == "bool": + value = bool(raw) + return (not value) if entry["invert"] else value + return raw + + +def pick_target(entry: dict, current): + """A valid value this setting does not currently hold.""" + if entry["type"] == "bool": + return not bool(current) + if entry["type"] == "enum": + for candidate in entry.get("values", []): + if candidate != current: + return candidate + return None + if entry["type"] in ("int", "real"): + low = entry.get("min", 0.0) + high = entry.get("max", max(low + 1.0, float(current or 0) + 1.0)) + step = entry.get("step", 1.0) + candidate = float(current or 0) + step + if candidate > high: + candidate = float(current or 0) - step + if candidate < low: + candidate = low if low != current else high + if entry["type"] == "int": + candidate = int(round(candidate)) + if candidate == current: + return None + return candidate + return None + + +def ipc(config_home: str, harness: str, *arguments: str) -> subprocess.CompletedProcess: + environment = dict(os.environ, XDG_CONFIG_HOME=config_home) + return subprocess.run(["qs", "-p", harness, "ipc", "call", "settings-system-test", *arguments], + capture_output=True, text=True, timeout=30, check=False, + env=environment) + + +def main() -> int: + config_home = sys.argv[1] + harness = sys.argv[2] + + entries = [entry for entry in schema_entries() if entry["key"] not in SKIP_KEYS] + verified, skipped, failures, originals = [], [], [], {} + + for entry in entries: + raw = read_option(entry) + if raw is None or entry["readAs"] not in COMPARABLE: + skipped.append((entry["key"], f"read-back shape {entry['readAs']}")) + continue + + current = stored_from_option(entry, raw) + originals[entry["key"]] = (entry, raw) + target = pick_target(entry, current) + if target is None: + skipped.append((entry["key"], "no second valid value")) + continue + + # Whatever happens below, this setting goes back to where it was + # before the next one is touched. A sweep that dies holding sixty + # settings at test values would be worse than no sweep. + try: + answer = ipc(config_home, harness, "commit", entry["key"], json.dumps(target)) + if answer.returncode != 0: + failures.append((entry["key"], f"commit failed: {answer.stderr.strip()[:80]}")) + continue + if answer.stdout.strip().lower() not in ("true", ""): + failures.append((entry["key"], f"commit refused the value ({answer.stdout.strip()[:40]})")) + continue + + # The compositor is asked again for up to a second. commitPreference + # verifies before storing, so a correct write is usually visible + # immediately -- but "usually" makes a flaky test, and a flaky test + # is worse than none. A write that never happens never matches, no + # matter how long this waits. + after = None + matched = False + deadline = time.monotonic() + 1.0 + while True: + after = stored_from_option(entry, read_option(entry)) + if isinstance(target, float) or isinstance(after, float): + matched = after is not None and abs(float(after) - float(target)) < 0.02 + else: + matched = after == target + if matched or time.monotonic() > deadline: + break + time.sleep(0.05) + if not matched: + failures.append((entry["key"], + f"set to {target!r} but the compositor reports {after!r} " + f"({entry['option']})")) + continue + verified.append(entry["key"]) + finally: + ipc(config_home, harness, "commit", entry["key"], json.dumps(current)) + + # ── Settings Panama stores itself ──────────────────────────────────────── + local_verified, local_skipped, local_failures = [], [], [] + for entry in local_entries(): + raw = ipc(config_home, harness, "stored", entry["key"]) + if raw.returncode != 0: + local_failures.append((entry["key"], "could not be read from the store")) + continue + try: + current = json.loads(raw.stdout.strip() or "null") + except json.JSONDecodeError: + local_failures.append((entry["key"], f"stored value is not readable: {raw.stdout.strip()[:40]}")) + continue + + target = pick_target(entry, current) + if target is None: + local_skipped.append((entry["key"], "no second valid value")) + continue + + try: + answer = ipc(config_home, harness, "commit", entry["key"], json.dumps(target)) + if answer.returncode != 0 or answer.stdout.strip().lower() not in ("true", ""): + local_failures.append((entry["key"], "the store refused the value")) + continue + back = ipc(config_home, harness, "stored", entry["key"]) + try: + after = json.loads(back.stdout.strip() or "null") + except json.JSONDecodeError: + after = None + if isinstance(target, float) or isinstance(after, float): + matched = after is not None and abs(float(after) - float(target)) < 0.02 + else: + matched = after == target + if not matched: + local_failures.append((entry["key"], f"set to {target!r} but the store reports {after!r}")) + continue + local_verified.append(entry["key"]) + finally: + ipc(config_home, harness, "commit", entry["key"], json.dumps(current)) + + print(json.dumps({ + "verified": verified, + "skipped": skipped, + "failures": failures, + "total": len(entries), + "localVerified": local_verified, + "localSkipped": local_skipped, + "localFailures": local_failures, + })) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())