Skip to content

Commit 978e257

Browse files
committed
qa: authoritative qmllint + a per-feature verification playbook
just lint now feeds qmllint the session $QML_IMPORT_PATH (-I) so QtQuick/Quickshell types resolve and filters the NixOS-unresolvable qs.* / singleton noise — validated 16->2 false positives per file, and it catches a real typo'd property the old noisy lint buried. QA.md documents the layered per-feature loop (hot-reload, lint, qs -p load gate, deterministic service parse tests via fake-bin/qmltestrunner, IPC liveness, visual+layer assertions, adversarial review, security) and the dev tools worth adding (qmltestrunner/qmlformat/qmlls, GammaRay, shellcheck, gitleaks, odiff).
1 parent 34982a9 commit 978e257

3 files changed

Lines changed: 129 additions & 2 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
!/SECURITY.md
2121
!/CHANGELOG.md
2222
!/ROADMAP.md
23+
!/QA.md
2324
!/.github/
2425

2526
# Migration/backup artifacts left by the dots installer

QA.md

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# Verification & QA
2+
3+
How every feature gets verified before it ships. Layered, fast → thorough; **the running shell is
4+
the source of truth** and rigor scales to risk (a service that shells out and parses CLI output gets
5+
the full treatment; a static widget gets the fast path). Built from a research pass on QML/Quickshell/
6+
Wayland test tooling (June 2026); see the tool table at the bottom.
7+
8+
## The per-feature QA loop
9+
10+
Apply top-to-bottom. Stop early for trivial widgets; run all of it for anything that parses output,
11+
shells out, handles IPC, or touches untrusted input.
12+
13+
### 0. While developing — hot-reload
14+
Edit in a worktree (`~/dev/dotfiles-dev`), watch the live instance:
15+
```sh
16+
qs -c ii log -f # follow; or: qs -c ii log | tail
17+
```
18+
**The running instance's log keeps STALE lines.** After a fix, don't trust the live buffer — confirm
19+
with a fresh isolated parse (step 2). This burned us once (the AndroidQuickToggleButton/HA spam).
20+
21+
### 1. Static gate — `just lint` (now authoritative)
22+
`just lint` feeds qmllint the session `$QML_IMPORT_PATH` (`-I …`) so QtQuick/Quickshell types resolve
23+
(false positives drop ~16→2 per file) and filters the categories that stay structurally unresolvable
24+
on NixOS (the `qs.*` config imports + project singletons). What survives is real — it catches a typo'd
25+
property (`heigth`) that the old noisy lint buried. Also grep the injection antipattern:
26+
```sh
27+
grep -rnE '"bash".*"-c".*\+' --include='*.qml' . # value concatenated into a shell string = injection
28+
```
29+
30+
### 2. Load gate — full-shell parse (authoritative "did it load")
31+
There is no parse-only flag and QML errors do NOT exit the process, so the gate is: load the whole
32+
shell in an isolated instance and assert it reaches "Configuration Loaded" with no errors in your files.
33+
```sh
34+
qs -p ~/dev/dotfiles-dev/quickshell/ii > /tmp/parse.log 2>&1 & # then grep, then kill
35+
grep -iE 'Configuration Loaded' /tmp/parse.log # must appear
36+
grep -iE '<YourFile>|error|is not a type|cannot assign' /tmp/parse.log # must be empty
37+
```
38+
Ignore the polkit / UPower / AiChat / KeyringStorage warnings — duplicate-instance artifacts.
39+
40+
### 3. Service logic — deterministic parse tests (the bug-catcher)
41+
For anything parsing `nmcli`/`pactl`/`playerctl`/leases output: **test the parse against canned output**,
42+
not live hardware. Two ways:
43+
- **Fake-bin PATH shim** — drop a fake `nmcli` printing a known line into a temp dir on `$PATH`, run the
44+
service, assert. (Quickshell `Process` calls `execve` and inherits `$PATH`, so this intercepts even
45+
`bash -c "nmcli …"`.)
46+
- **Pure-function unit test** — split the parse into a plain JS function and run it under
47+
`qmltestrunner -platform offscreen` (or even `node`/`python` for a quick check).
48+
49+
This is the layer that catches the substring-class bugs (`includes("activated")` matching
50+
`"deactivated"`; `includes("connected")` matching `"disconnected"`). When a feature depends on a tool's
51+
exact output, **run the tool and look** — don't assume (the `:activated` severity was over-rated because
52+
an agent assumed the nmcli output).
53+
54+
### 4. IPC liveness — `qs ipc`
55+
```sh
56+
qs ipc --pid $(pgrep -f 'quickshell -c ii') call <target> status # service alive + sane state
57+
qs ipc show # list all IpcHandler targets
58+
```
59+
Every service should expose a `status()` IpcHandler returning its key state.
60+
61+
### 5. Interactive + visual (UI features)
62+
```sh
63+
just test-ui <feature> # ydotool drives the keybind, grim screenshots, Read the PNG
64+
```
65+
Assert the layer surface actually exists with the right geometry (Hyprland IPC):
66+
```sh
67+
hyprctl -j layers | jq '.. | objects | select(.namespace? | startswith("quickshell:"))'
68+
hyprctl -j monitors | jq '.[] | select(.focused) | .reserved' # bar exclusive zone
69+
```
70+
Optional **visual regression**: `grim -g "<geom>" actual.png` then `odiff golden.png actual.png diff.png
71+
--aa --threshold 0.05 --fail-on-layout` (the `--aa` skips antialiased font noise).
72+
73+
### 6. Adversarial review (before ship)
74+
Run the multi-agent review on the diff (Sonnet, cost-controlled):
75+
the `qa-session-review` workflow — finders per dimension (bugs/vulns/races/redundancy) → each finding
76+
adversarially verified. It found the real hotspot bugs this layer is meant to catch.
77+
78+
### 7. Security
79+
- Injection: the grep in step 1; **always pass tainted values via the Process `environment` map**, never
80+
interpolate into `bash -c`.
81+
- `shellcheck` on any helper `.sh`.
82+
- `gitleaks detect --no-banner` for secrets (stronger than a hand grep); keep committed files
83+
publication-safe (no hostnames/IPs/tokens — `example.com` placeholders).
84+
- Any externally-controlled string in a `StyledText``textFormat: Text.PlainText` (StyledText defaults
85+
to AutoText = HTML; a DHCP hostname could inject markup).
86+
87+
## Tooling
88+
89+
Already in the loop: `qmllint` (now `-I`-fed), `qs -p` / `qs ipc` / `qs log`, `just test` (IPC selftest),
90+
`just test-ui` (ydotool + grim), `hyprctl -j layers/monitors`, `dbus-monitor`.
91+
92+
Worth adding (NixOS `home.packages` / a devShell):
93+
94+
| Tool | Nix attr | Use |
95+
|---|---|---|
96+
| qmltestrunner, qmlformat, qmlls, qmlprofiler | `kdePackages.qtdeclarative` | unit-test pure QML logic offscreen; format; editor LSP; perf |
97+
| **GammaRay** | `gammaray` | attach to the running shell, inspect the live QML object tree / bindings / signals — best bug-hunting tool |
98+
| **shellcheck** | `shellcheck` | lint the helper `.sh` scripts (none today) |
99+
| **gitleaks** | `gitleaks` | secret scanning, pre-commit + CI |
100+
| **odiff** | `odiff` (npm `odiff-bin` if not yet in channel) | visual-regression diffing, AA-tolerant |
101+
| statix, deadnix | `statix`, `deadnix` | lint the NixOS flake (separate repo) |
102+
103+
Input-injection note: `ydotool` (uinput, what we use) is compositor-agnostic and reliable; `wtype` is a
104+
zero-privilege wlroots alternative; `wlrctl` adds mouse + window-focus targeting for wlroots — handy if a
105+
test needs to focus a specific window before typing.
106+
107+
## Headless CI (future)
108+
Fully-automated UI tests can run in a NixOS VM with Hyprland headless: `HYPRLAND_HEADLESS_ONLY=1`, QEMU
109+
`-vga none -device virtio-gpu-pci` (LLVMpipe software GL), then `hyprctl output create headless`, drive
110+
with ydotool, capture with grim, assert with `hyprctl -j` + odiff. Hyprland's own `nix/tests` is the
111+
reference recipe. Involved — current practice is the local loop above; this is the upgrade path.
112+
113+
## Lessons baked in (hotspot QA cycle, 2026-06-30)
114+
- Verify a CLI's real output empirically before parsing it.
115+
- Field-exact parsing, never substring `includes()` on status strings.
116+
- Tainted values via `environment`, never shell-string interpolation.
117+
- `Text.PlainText` for any externally-controlled string.
118+
- The live log lies (stale lines) — confirm fixes with a fresh `qs -p`.

justfile

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,22 @@ set shell := ["bash", "-uc"]
44
default:
55
@just --list
66

7-
# qmllint every Quickshell QML changed on this branch (+ uncommitted/untracked)
7+
# qmllint every Quickshell QML changed on this branch (+ uncommitted/untracked).
8+
# Authoritative mode: feed qmllint the session's $QML_IMPORT_PATH (-I) so QtQuick/Quickshell
9+
# types resolve (cuts the false-positive flood ~16->2 per file), then filter the categories that
10+
# stay structurally unresolvable on NixOS — the `qs.*` config-relative imports and the project's
11+
# own singletons (Translation/Config/Appearance/...), plus the Process.exited signal-param noise.
12+
# What survives the filter is real signal (e.g. a typo'd property). See QA.md.
813
lint:
914
@changed=$( { git diff --name-only main...HEAD -- 'quickshell/**/*.qml'; \
1015
git ls-files -m -o --exclude-standard -- 'quickshell/**/*.qml'; } \
1116
| sort -u | grep . || true ); \
1217
[ -z "$changed" ] && { echo "no changed QML to lint"; exit 0; }; \
18+
iflags=""; for p in $(echo "${QML_IMPORT_PATH:-}" | tr ':' ' '); do [ -n "$p" ] && iflags="$iflags -I $p"; done; \
19+
[ -z "$iflags" ] && echo "⚠ QML_IMPORT_PATH unset — lint runs in noisy mode (run from the graphical session, or export it in CI)"; \
20+
filt='\[import\]|\[unqualified\]|\[signal-handler-parameters\]|Warnings occurred while importing|not found on type "(Translation|Config|Appearance|GlobalStates|Directories|FileUtils|ColorUtils|MaterialThemeLoader)"'; \
1321
fail=0; for f in $changed; do [ -f "$f" ] || continue; \
14-
out="$(qmllint "$f" 2>&1 || true)"; \
22+
out="$(qmllint $iflags "$f" 2>&1 | grep -iE 'warning:|error:' | grep -vE "$filt" || true)"; \
1523
if [ -n "${out//[[:space:]]/}" ]; then echo "✗ $f"; echo "$out" | sed 's/^/ /'; fail=1; \
1624
else echo "✓ $f"; fi; done; \
1725
exit $fail

0 commit comments

Comments
 (0)