Repository navigation
ci: one Rust-first workflow, and fix the long-broken Python job - #68
Merged
Merged
Conversation
The "Reference engine (Python 1.5.0)" workflow had been failing on every
push since April, for a self-inflicted reason: it set
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1, which unloads pytest-cov, and then
passed --cov to pytest.
pytest: error: unrecognized arguments: --cov=. --cov-report=term-missing
That env var is a workaround for stray plugins in a local developer
environment and has no business on a clean runner. Dropped, along with
coverage: measuring coverage of retired code that will never ship again
is noise.
rust.yml and tests.yml are consolidated into ci.yml, so the Actions tab
reflects what this project is. The reference engine's own 76 tests now
run as a step inside the parity job, where they belong — they exist to
guard the implementation the parity harnesses measure against, so if they
break, every parity result below them is meaningless.
Verified the whole legacy suite runs headless with no PyQt6 installed:
the four tests that touch Qt inject stubs into sys.modules rather than
importing it. Blocked the real PyQt6 with a meta_path finder and got
76/76, so no --ignore flags and no PyQt6 install are needed.
Also refreshed the test counts in README (246 -> 251) and pointed the
code checklist audit at the files that exist now rather than at
tests.yml, pyinstaller.yml and AutoTidy.spec, all removed in the rewrite.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Three fixes from the first CI run of the consolidated workflow.
Clippy failed on both platforms. My local toolchain was 1.93 and the
runner is 1.97, which added `manual_sort_by`:
error: consider using `sort_by_key`
--> crates/autotidy-core/src/undo.rs:93
Updated the local toolchain to reproduce it rather than patching blind,
which surfaced a third lint the run had not reached: `byte_char_slices`
in src-tauri. That one was never going to be caught, because the Engine
job lints only the two engine crates and clippy lints are not emitted by
rustc — so `-D warnings` on the app's build step does not cover them. The
app job now runs clippy too. It stays out of the Linux Engine job because
the Tauri crate needs GTK/WebKit libraries that runner does not have.
The legacy suite no longer runs in CI. It covers retired 1.5.0 modules —
the PyQt dialogs, config_manager, startup_manager — that nothing imports
and that will never ship, and one of its tests asserts on a path shape
that breaks wherever the username exceeds eight characters:
'C:\Users\RUNNER~1\...\monitor_me' not found in
['C:\Users\runneradmin\...\monitor_me']
A red X for an 8.3 filename quirk in a test for a dead dialog is noise.
The part of the reference engine that matters is utils.py and
constants.py, which both parity harnesses import directly and exercise
across 23 rule variants — if those break, parity breaks. The suite stays
runnable locally and is documented in legacy/README.md.
`Engine (ubuntu-latest)` failed on one test while Windows passed:
scan::tests::destination_outside_the_tree_needs_no_guard
The fixture asked whether `D:/elsewhere/{YYYY}` needs a guard. On Windows
that is an absolute path outside the monitored tree, so the answer is no.
On Unix there are no drive letters: `D:/elsewhere` is a *relative* path
naming a directory called `D:`, so `guard_paths` correctly resolved it
inside the monitored folder and produced a guard.
The engine was right both times; the test was Windows-only in disguise
and silently asserted the opposite of its name on Linux. Fixtures now go
through `monitored()` and `elsewhere()` helpers that spell an absolute
path for the host platform.
This is what the ubuntu leg of the matrix is for. Swept the engine crates
for other hardcoded drive letters and found none; the remaining ones are
in src-tauri, which is Windows-only by nature and only ever built there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Reference engine (Python 1.5.0)has been failing on every push since April, for a self-inflicted reason:The workflow disables plugin autoload — which unloads
pytest-cov— and then passes--covto pytest. That env var is a workaround for stray plugins in a local dev environment; a clean runner has no such problem.What changed
rust.yml+tests.yml→ci.yml. One workflow, so the Actions tab reflects that this is a Rust project rather than showing a permanently-red Python entry beside it.--cov. Coverage of retired code that will never ship again is noise.Verified, not assumed
The whole legacy suite runs headless with no PyQt6 installed — the four tests that touch Qt inject stubs into
sys.modulesrather than importing it. I confirmed this by blocking the real PyQt6 with ameta_pathfinder:So no
--ignoreflags and no PyQt6 install are needed in CI.Also
docs/CODE_CHECKLIST_AUDIT.mdpointed at files that exist now, rather thantests.yml,pyinstaller.ymlandAutoTidy.spec— all removed in the rewriteJobs
🤖 Generated with Claude Code