Skip to content

♻️ refactor(coverage): key exclusions on probed capability#679

Open
gaborbernat wants to merge 19 commits into
tox-dev:mainfrom
gaborbernat:feat/coverage-pragmas
Open

♻️ refactor(coverage): key exclusions on probed capability#679
gaborbernat wants to merge 19 commits into
tox-dev:mainfrom
gaborbernat:feat/coverage-pragmas

Conversation

@gaborbernat

@gaborbernat gaborbernat commented Jul 18, 2026

Copy link
Copy Markdown
Member

A pragma that names a platform is standing in for the reason the code cannot run there, and the stand-in goes stale. One win32 no cover sat on the os.link fallback for follow_symlinks and demanded a branch modern Windows never takes, while others excluded platform-agnostic code and hid Windows gaps behind it. Of the markers carrying no reason at all, most were per-line copies of one block-level fact.

A configurer plugin in tasks/coverage_pragmas.py, registered after covdefaults, replaces the platform name with the capability the code needs. 🔍 # pragma: needs <capability> drops out only where that capability is absent and # pragma: lacks <capability> only where it is present, across sixteen capabilities probed at runtime: fork, dir-fd, hard-link, symlink, fcntl, unlink-open-file, posix-signals, file-mode, fd-directory, fifo, af-unix, o-nofollow, utime-nofollow, utime-fd, sqlite3 and link-follow-symlinks. The skipif gates read the same CAPABILITIES mapping the plugin excludes from, so a gate and its exclusion cannot drift apart. hard-link stays required on Windows, which has hard links and which win32 no cover could not express.

Each remaining marker sits on the enclosing clause header or decorator, which coverage carries across the whole body, so the per-line copies are gone and no marker has to fit inside a line already past the width limit.

Naming the capability separates cases the platform name had merged. Windows does support symlinks, so the cases needing one now run there for the first time; the cases asserting that a symlinked parent collapses into one key stay gated, because _resolve_dir resolves a parent with abspath on Windows to avoid following junctions and reparse points, which keeps that parent a distinct key. A symlink job runs it both ways, and denying it takes both doors, since Developer Mode and SeCreateSymbolicLinkPrivilege each grant symlink creation on their own. The job clears the registry value and removes the privilege, which AdjustTokenPrivileges documents as irreversible with checks then returning STATUS_PRIVILEGE_NOT_HELD.

report.fail_under rises from 95 to 99, and the combined matrix reports 100% with no missed statement or partial branch. ✅ No public behavior changes, since the edits under src/filelock/ are markers and one reflowed call.

Every env in the matrix runs coverage with the same fail_under, but a line
guarded by sys.platform only runs on one OS, so the others counted it missing.
Windows sat at ~79% because the fork and POSIX-only code and their tests never
run there.

Mark those lines with covdefaults win32/linux/darwin pragmas, derived from the
per-platform .coverage data the CI matrix already uploads: a line is excluded on
a platform only when it is proven to run on the other two, so no real gap is
hidden. Marks land on the clause header (def/if/with) so a whole block carries
one pragma; a few headers already past 120 characters are left unmarked rather
than wrap in a way ruff would reflow.

The per-(os, py) floor rises from 79% to 97% (win32 on 3.10), so fail_under
moves to 97. pypy runs without coverage and is unaffected.
@gaborbernat gaborbernat added the skip news Internal change; exempt from the news fragment check label Jul 18, 2026
Termux skips the os.link-backed strict tests, so its total coverage (~87%)
cannot meet the matrix fail_under of 97. Run the suite with empty posargs, the
same way pypy does, so Termux stays a functional platform check.
@gaborbernat
gaborbernat force-pushed the feat/coverage-pragmas branch from ce55c3d to dcc77bf Compare July 18, 2026 19:52
The pragma sweep marked 42 source lines win32 no cover that are platform-
agnostic (async rollback/registration, SoftFileLease heartbeat, strict-lock
cleanup, exception-group traversal). They only lacked Windows coverage because
the tests reaching them are @_UNIX_FLOCK_ONLY / dir_fd / fork -- a Windows CI
run of those tests confirmed 35 genuinely need Unix (fcntl, umask, symlink,
flock semantics), so the skips stay, but the source they cover is reachable on
Windows and untested there: a real gap, not intrinsic-platform code.

Drop those masks so the gap is counted honestly. Windows still rises from 79%
to 96.3% from the legitimate platform pragmas (sys.platform branches, fork,
dir_fd, O_NOFOLLOW, flock). fail_under moves to 95, the honest per-(os,py)
floor (win32 on 3.10).
@gaborbernat
gaborbernat force-pushed the feat/coverage-pragmas branch 2 times, most recently from bdb19bc to fc94fb4 Compare July 18, 2026 21:21
Add fault-injection tests that drive the rollback, cancellation, and
error-grouping paths through the code's own os/close layer, so they run
on every platform instead of only POSIX. This closes coverage holes
that platform pragmas would otherwise have masked as Windows gaps.

Mark the branches that are genuinely unreachable off-platform — the
win32 defensive OS-error handlers and the sqlite3-absent import
fallback — with precise pragmas carrying their reason.

Raises darwin coverage to 98.95%; the honest per-platform floor gets
ratcheted once CI reports the Windows rise.
@gaborbernat
gaborbernat force-pushed the feat/coverage-pragmas branch from fc94fb4 to 7838c32 Compare July 18, 2026 21:32
Cover the deterministic branches that were simply untested — optional
marker fields, default-parameter paths, no-callback and cyclic-chain
guards, re-entrant undo, fork-safety PID guards, and the async acquire
that returns without the lock — with real tests through the public API,
rather than papering over them.

Reserve pragmas for code that genuinely cannot run under coverage: the
win32 GetProcessTimes failure path, the follow_symlinks fallback that
only PyPy takes, forked-child and pypy-backend test bodies, and the
posix-only fork/mkfifo/symlink/dir_fd tests (marked per platform). Fix
_strict.py's link pragma, which wrongly required the fallback branch on
win32 that modern os.link never takes there.

Darwin coverage rises to 99.71%; the per-platform floor is ratcheted
once CI reports the win32 rise.
The cleanup helper sends SIGTERM and joins briefly; on a loaded
free-threaded runner the reap can land just after that window, so join
again generously before asserting the worker is gone.

With the branch and platform gaps now covered, the honest per-cell
floor across the CI matrix is 99.55% (win32, 3.10). Raise fail_under
from 95 to 99 so a real regression trips it while leaving margin for
per-run fluctuation.
A pragma naming a platform stands in for the reason the code cannot run
there, and the stand-in goes stale. One win32 no cover sat on os.link's
follow_symlinks fallback and demanded a branch modern Windows never
takes; others hid real Windows gaps behind platform-agnostic code. The
711 that carried no reason at all were mostly per-line copies of a
single block-level fact.

Add a coverage configurer plugin defining needs/lacks pragmas over 16
probed capabilities: fork, dir-fd, hard-link, symlink, fcntl,
unlink-open-file, posix-signals, file-mode, fd-directory, fifo, af-unix,
o-nofollow, utime-nofollow, utime-fd, sqlite3 and link-follow-symlinks.
A needs line drops out only where the capability is absent, a lacks line
only where it is present, and both read the same probes the tests gate
their skipif on, so a test cannot skip while coverage still demands its
lines. hard-link stays required on Windows, which has it.

Collapse the per-line copies onto the enclosing clause header or
decorator, which coverage carries across the whole body, retiring the
pragmas that no longer fit inside the line limit.

Drive Windows symlink support from Developer Mode in both directions,
since Windows does support symlinks; five symlink tests now run there.
Assert the flock fallback warning instead of letting it through, so the
suite reports no warnings.
filelock resolves a lock's parent with abspath on Windows so junctions
and reparse points are never followed, which also keeps a symlinked
parent a distinct key there. The cases asserting that two spellings
collapse into one key were gated on symlink creation, so once the
capability probe found the privilege on a Windows runner they ran and
blocked until the timeout killed the cell. Gate them on the collapsing
they actually need.

Prepend to PYTHONPATH in the old-client fixture rather than replace it,
so the coverage plugin still resolves in the child coverage restarts.

State the non-Windows invariant the fcntl gate enforces, so ty can see
flock's members when it checks on Windows, and read the capability
mapping under its public name in the symlink workflow.
Windows grants symlink creation two independent ways, so clearing one
proves nothing: Developer Mode lets an unprivileged process create them,
and SeCreateSymbolicLinkPrivilege lets any holder create them whatever
Developer Mode says. The runner's account holds the privilege, so both
variants probed symlink: True and the denied job was a duplicate of the
granted one.

Remove the privilege from the token before launching the suite.
AdjustTokenPrivileges cannot add one back, and SE_PRIVILEGE_REMOVED is
documented as irreversible, with privilege checks for removed privileges
returning STATUS_PRIVILEGE_NOT_HELD.

Two things the documentation does not settle decide whether this works:
whether a child inherits the removal, and whether clearing Developer
Mode takes effect without a reboot. Report the token privilege, the
registry value, and whether a symlink still succeeds in this process and
in a child, so the log names the mechanism that governs.
@gaborbernat gaborbernat changed the title 👷 ci(coverage): pragma platform-specific lines ♻️ refactor(coverage): key exclusions on probed capability Jul 19, 2026
report.fail_under tracks the weakest env, because a single one cannot
reach 100: the old-client suite runs only where FILELOCK_OLD_CLIENT_PATH
is set, and the escrow paths only below 3.12, which leaves win32 on 3.10
at 99.35. Every line is still covered by some env, so the combined
report reaches 100 and gating it there catches a line no env exercises,
which 99 let through.
An env sat below 100 wherever code it never runs carried no conditional
marker, which the version, implementation and capability pragmas already
exist to express. The if half of the BaseExceptionGroup import went
unmarked while only its else carried one, so 3.10 counted it missing.
The compat suite counted as missed wherever FILELOCK_OLD_CLIENT_PATH is
unset, so gate it on a new old-client capability that its skipif reads
too. The strict modules cannot run without os.link at all, so let the
plugin drop a whole module a missing capability makes unrunnable rather
than restate one module-level gate on every line.

Cover the sentinel private-record reclaim with a case that ages a record
on any platform, which only the dir_fd reaper cases reached before, and
drop the win32 pragma from the scan that runs everywhere.

Darwin now reports 100% with no missed statement and no partial branch.
Seven envs already reached 100; the rest still ran code they never
marked. The escrow early return only runs from 3.12, the exception-group
cases are gated at 3.11 with no matching marker, and the symlink helpers
claimed to need a symlink when their only callers are the cases Windows
does not run.

Cover the marker unlink retry rather than mark it: the EACCES path
exists for the handle Windows holds after close, so deny every attempt
and prove it stops without the denial escaping.

Escalate the worker cleanup to kill after a terminate that does not
land. It exists so a worker cannot outlive its test, which a SIGTERM
alone did not guarantee on a saturated runner.
The capability names and their probes already say what each one checks,
so the per-entry notes only restated them; the same went for notes that
repeated a test's own name and for the needs/lacks explainer the module
docstring already carries. Keep the ones a reader cannot recover from
the code: the 64-bit handle truncation, typeshed hiding fcntl off POSIX,
and why a line needs two exclusions.

A named pragma states its own reason, so drop the prose trailing
forked child.
Nothing exercised either deliberately, so which env covered them came
down to GC timing and to whichever path happened to run, holding from
3.12 and slipping on 3.10 and 3.11. Both are reachable behaviour rather
than something to exclude: a dropped lock must close the connection it
still held, and a foreign pid must leave an inherited connection open
for the parent that owns it.
Every env in the matrix now reaches 100, so the gate no longer tracks a
weakest one and the combined report needs no separate threshold.

Termux runs under coverage with the rest: it skips the strict tests for
want of os.link, and the plugin now drops those modules where the
capability is missing, so the suite it does run has to hold 100 too
rather than passing as a functional check.

Drive the finalizer directly rather than a collection, whose timing left
one env covering the connection close by luck.
Running Termux under coverage showed the strict tests were dropped while
the backend they exercise was not, leaving its 316 lines counted where
nothing can call them. Without os.link the whole feature is out of
reach, so drop the module too and mark what it reaches elsewhere: the
error only it raises, the sentinel only it leaves behind, and the
transition gate and poll backoff no other backend turns on.
The no-loop return in __del__ came and went between runs because only
collection timing ever reached it, which a 100 gate turns into a flaky
build. Drive it from a lock built outside any loop instead.

Mark the finally clause of the transition gate as well: it belongs to
the same statement as the try, but coverage treats it as its own clause,
so the exclusion did not carry.
Only a heartbeat thread catching the eviction mid-tick ever reached the
token mismatch, so whether it counted came down to timing. Overwrite the
marker with a peer's token and ask the refresh directly.
The denied job printed the capability from a process the removal never
touched, so its log read symlink: True while the suite it launched ran
denied. Print it from under the launcher instead.

PyPy stays unmeasured, but for its own reason rather than the one the
Termux comment used to borrow: coverage has no JIT path there, so the
suite takes 12m rather than 2m and roughly 60 of its lock, heartbeat and
poll deadlines lapse under the latency, though they pass measured in
isolation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement skip news Internal change; exempt from the news fragment check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant