change: convert the test scripts to pytest

mysticalsoap commented

2026-08-17 21:21:41 -04:00

Owner

First half of #25 (the conversion; the CI job needs a runner decision first, see below). Closes #30 -- the raw-string commit is that issue's exact fix. Stacked on #37 -- the first four commits here are that PR and will disappear from this diff when it merges.

Problem

Five standalone scripts, each with its own copy-pasted check() harness (one had already drifted), each named after the bug that prompted it -- so the directory grows with the bug history, and there is no single runner, no selection, no fixtures.

Fix

pytest, with files named after the module under test: test_update.py (import guard, credentials, ovpn_quote), test_tunnel.py, test_aqomui_gui.py, plus one conftest.py. Same coverage, same rationale docstrings, shared fixtures instead of five harness copies. The OpenVPN-parser cases keep their skip when the binary is absent (skipif). tests/manual/ stays uncollected. CONTRIBUTING gains a Testing section stating the layout rule and the no-root/network/account/display line.

Also fixes the three '\d' regex literals to raw strings -- a SyntaxWarning today, a SyntaxError in a future Python, and the thing a -W error::SyntaxWarning CI step would trip on.

Verification

pytest -q: 23 passed. python -W error::SyntaxWarning -m compileall aqomui tests: clean. ruff check --select E9,F63,F7,F82: clean -- so the future CI lint gate can start strict with no code changes.

Notes

PR #16 adds tests/test_scripts_button.py; once it lands, that script needs the same mechanical conversion (its cases belong in test_aqomui_gui.py).
The CI half of #25 is blocked on infrastructure: the only registered runner is the deploy one, host-type, deliberately without Docker access and pinned to a single sudoers command. Running untrusted PR code on it would undo that containment, so CI wants a second, container-capable runner first.

🤖 Generated with Claude Code

First half of #25 (the conversion; the CI job needs a runner decision first, see below). Closes #30 -- the raw-string commit is that issue's exact fix. Stacked on #37 -- the first four commits here are that PR and will disappear from this diff when it merges. **Problem** Five standalone scripts, each with its own copy-pasted `check()` harness (one had already drifted), each named after the bug that prompted it -- so the directory grows with the bug history, and there is no single runner, no selection, no fixtures. **Fix** pytest, with files named after the module under test: `test_update.py` (import guard, credentials, `ovpn_quote`), `test_tunnel.py`, `test_aqomui_gui.py`, plus one `conftest.py`. Same coverage, same rationale docstrings, shared fixtures instead of five harness copies. The OpenVPN-parser cases keep their skip when the binary is absent (`skipif`). `tests/manual/` stays uncollected. CONTRIBUTING gains a Testing section stating the layout rule and the no-root/network/account/display line. Also fixes the three `'\d'` regex literals to raw strings -- a SyntaxWarning today, a SyntaxError in a future Python, and the thing a `-W error::SyntaxWarning` CI step would trip on. **Verification** `pytest -q`: 23 passed. `python -W error::SyntaxWarning -m compileall aqomui tests`: clean. `ruff check --select E9,F63,F7,F82`: clean -- so the future CI lint gate can start strict with no code changes. **Notes** - PR #16 adds `tests/test_scripts_button.py`; once it lands, that script needs the same mechanical conversion (its cases belong in `test_aqomui_gui.py`). - The CI half of #25 is blocked on infrastructure: the only registered runner is the deploy one, host-type, deliberately without Docker access and pinned to a single sudoers command. Running untrusted PR code on it would undo that containment, so CI wants a second, container-capable runner first. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

mysticalsoap added 6 commits

2026-08-17 21:21:41 -04:00

change: move the import guard onto a shared thread base 9f10fabe0a

14dd13a guarded AddServers.run in place. The same abort-on-escape rule
applies to any QThread the root service owns, so the guard becomes a base
class: subclasses implement work() and report_failure(), and GuardedThread
guarantees nothing reaches the top of run(). No behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix: keep an unhandled tunnel error from killing the service d0bee2fd15

TunnelThread.run caught only KeyError, and that except was the OpenVPN
dispatch fallback, not a guard -- anything else raised while establishing
a tunnel reached the top of the QThread and PyQt5 aborted the root
service, dropping every active tunnel. The fallback also rerouted a
KeyError raised anywhere inside wireguard() into openvpn() with a
WireGuard server dict.

TunnelThread now runs on GuardedThread: the dispatch keys on
server_dict.get("tunnel"), so a missing key still means OpenVPN, and an
escaped error ends as conn_attempt_failed -- the status the GUI already
answers with cleanup and a notification. report_failure also deletes the
outbound allow rule(s) the attempt inserted, which nothing else takes
back on this path.

tests/test_tunnel_failure.py covers it (no root, network or account) and
fails against the unfixed code, including the wireguard-to-openvpn reroute.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix: let a WireGuard setup error reach the guard's report 569dc42525

wg()'s own except emitted "fail", a status the GUI cannot dispatch --
it resolves status strings to methods of the same name via getattr and
has none called fail, so reporting the failure raised AttributeError in
the GUI instead of telling the user anything. It also logged nothing, so
the log the notification points at had no cause in it.

GuardedThread.report_failure already does this right: the cause is
logged, the attempt's firewall rules come back out, and the GUI gets
conn_attempt_failed. Delete the local handler and let it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

change: stop mislabeling what broad exception handlers swallow 0b6f2b67b5

allow_dest_ip caught BaseException and reported every failure as an
invalid ip address -- a missing iptables binary, or SystemExit, got the
same line. It now catches Exception and logs what actually happened.

The bare except around the hop ip is a real fallback (no hop configured),
so it stays one, narrowed to the KeyError/TypeError that case produces.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

change: convert the test scripts to pytest 75b6c29aa4

Five standalone scripts, each with its own copy of the same check()
harness -- one already drifted. pytest replaces the harness with plain
asserts and shared fixtures, and buys collection, selection and a real
exit code.

Files are now named after the module under test, one file per module, so
the directory grows with the package rather than with the bug history:
update (import guard, credentials, ovpn_quote), tunnel, aqomui_gui. The
openvpn-parser cases keep their skip when the binary is absent, as
pytest.mark.skipif. tests/manual/ stays uncollected -- it needs root.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

fix: make the ip regexes raw strings c48284cffd

'\d' in a plain string is an invalid escape sequence -- a SyntaxWarning
today, a SyntaxError in a future Python.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

mysticalsoap merged commit c2bc5506da into trunk

2026-08-17 21:25:26 -04:00

mysticalsoap deleted branch change/tests-to-pytest

2026-08-17 21:25:26 -04:00

Rows
Columns

change: convert the test scripts to pytest #38