♻️ refactor(coverage): key exclusions on probed capability#679
Open
gaborbernat wants to merge 19 commits into
Open
♻️ refactor(coverage): key exclusions on probed capability#679gaborbernat wants to merge 19 commits into
gaborbernat wants to merge 19 commits into
Conversation
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.
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
force-pushed
the
feat/coverage-pragmas
branch
from
July 18, 2026 19:52
ce55c3d to
dcc77bf
Compare
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
force-pushed
the
feat/coverage-pragmas
branch
2 times, most recently
from
July 18, 2026 21:21
bdb19bc to
fc94fb4
Compare
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
force-pushed
the
feat/coverage-pragmas
branch
from
July 18, 2026 21:32
fc94fb4 to
7838c32
Compare
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.
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.
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.
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 coversat on theos.linkfallback forfollow_symlinksand 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,sqlite3andlink-follow-symlinks. Theskipifgates read the sameCAPABILITIESmapping the plugin excludes from, so a gate and its exclusion cannot drift apart.hard-linkstays required on Windows, which has hard links and whichwin32 no covercould 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_dirresolves a parent withabspathon Windows to avoid following junctions and reparse points, which keeps that parent a distinct key. Asymlinkjob runs it both ways, and denying it takes both doors, since Developer Mode andSeCreateSymbolicLinkPrivilegeeach grant symlink creation on their own. The job clears the registry value and removes the privilege, whichAdjustTokenPrivilegesdocuments as irreversible with checks then returningSTATUS_PRIVILEGE_NOT_HELD.report.fail_underrises from 95 to 99, and the combined matrix reports 100% with no missed statement or partial branch. ✅ No public behavior changes, since the edits undersrc/filelock/are markers and one reflowed call.