test: fail closed on skipped pytest evidence - #1733
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughPytest session finishing now treats skipped, xfailed, xpassed, and missing terminal statistics as failures. Focused tests cover status preservation, fail-closed behavior, zero-count filtering, and stable diagnostics. The changelog records the policy. ChangesPytest outcome governance
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to The PR correctly makes skipped and other non-executed tests fail the repository gate, but its failure handling can replace an existing interruption or internal-error status with a generic test failure, making diagnosis less precise. The change is mergeable with explicit owner follow-up to preserve existing non-success codes and add regression coverage. Sequence Diagram(s)sequenceDiagram
participant PytestSession
participant ConftestHook
participant TerminalReporter
participant OutcomePolicy
PytestSession->>ConftestHook: invoke pytest_sessionfinish
ConftestHook->>TerminalReporter: retrieve terminal reporter
ConftestHook->>OutcomePolicy: enforce outcome policy
OutcomePolicy->>TerminalReporter: read skip, xfail, and xpass counts
OutcomePolicy-->>PytestSession: retain OK or set TESTS_FAILED
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes implement fail-closed handling for skipped, xfailed, and xpassed outcomes, preserve clean-run status, handle missing terminal reporting, and add focused policy tests without fabricating capability evidence [ Full details: Out of Scope Changes checkExplanation All listed changes support issue Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. (1 skipped: 1 unsupported.) Full details: Title checkExplanation The title clearly describes the main change: pytest evidence now fails closed when skipped outcomes occur. It does not mention xfail, xpass, or missing reporter handling, but it remains concise and relevant. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Single-writer repair: fresh changed-file inventory found two portability edits that belong to active canonical owner PRs. Population-label extended-precision fail-closed evidence is now adopted by #1522 at #1733 is intentionally Draft until those canonical owner deltas integrate and the mirrored paths can be removed without losing test evidence. Do not land this PR ahead of those owner paths. No valid delta was discarded. |
Closes #1732.
Defect
Repository pytest invocations could report GREEN while owned scientific, Rust/GPU parity, security, filesystem, or platform tests emitted
skip/importorskip/skipif/xfail-family outcomes. Missing capability can be a truthful platform fact, but non-execution is not successful evidence.A second integration-order defect allowed an ordinary later
pytest_sessionfinish(trylast=True)plugin to resetsession.exitstatustoOKafter this repository had classified skipped evidence asTESTS_FAILED, producing a real process exit 0 despite the fail-closed policy.RED → GREEN lineage
33ce5563dfd1a5bfe8a744214a3303b58161f789exposed the original hidden non-execution surface; GREEN3131f27d15ed4951e3a4fa5775b0effde80dca15madeskipped,xfailed, andxpassedterminal buckets non-passing and failed closed without terminal accounting.1adf41e946a21dc4e9c53d08ac3693770bcf0ef2proved stronger pytest exit classifications could be overwritten; GREENcbe0d836af8083f46f310e0d50788e0c88b88c1cpreserves every pre-existing non-success exit and exercises a real child pytest process.5dc238d25cd10e6889c9900d49714c793c19e01eproved missing descriptor-relative atomic-write prerequisites produced false passes; GREEN3ab1ebf3a3e73632bd46d49998a009dee5f0a728makes those prerequisites failing evidence.1a75be806190b3e30784ced0419ddda79dadbf76proved a later ordinary session-finish plugin could reset skip-derived failure to exit 0; GREEN6430de94e78f6f65af2af47e90c8cd9bf321d163makes the repository verdict an outer hook wrapper while preserving stronger non-success classifications.Repository-owned non-execution repairs
The hook is not used as a substitute for repairing owned tests. This branch also converts concrete hidden-success paths while preserving model/numerical ownership:
longdoublecannot supply the wider precision required by the test instead of recording a passing skip.The latest source sequence for that portability sweep is
10b1e5dbc0df9947a8beacec112f7fef0b4ff188(population labels),3c1ed4e0d5c3707e10e49ae12cc986f0f29d0170(G-theory cut),584d213e9610b741bfe0937de4ad67d0e83258b8(WLE),e169e935dea792c7598c5c8cee6223038ea78363(CDM), and102e116d7c0183ed77ab4e0ffaac0cc9cb551ccd(2PL). Current governed changelog/head is07cc43803736500df145f8a597cf1b9f1ef142d4.Files owned by active model/stack PRs remain with their canonical writers; for example RSM lossless-tolerance evidence remains #1699-owned and Rasch CML evidence remains #1516-owned rather than being edited here.
Review state
Visible review submissions remain COMMENTED only; no qualifying
APPROVEDreview exists. All returned inline threads are resolved. Earlier exit-status and atomic-write findings are repaired; the GPU-fallback comment is informational. No self-approval or predecessor-review transfer is used.Live governance and landing
Current exact source head is
07cc43803736500df145f8a597cf1b9f1ef142d4on protectedmain@b5a3a0c1057d4b53d7a4bb18e0de69f630c2b45c. Every predecessor check is non-transferable after these source moves.The inherited organization ruleset
18156473requires one approval, resolved review threads, and nine central required workflows. Central CodeQL is intentionally not one of those nine after.github#1719established thatgithub/codeql-actioncannot execute as a ruleset-required workflow. fast-mlsirm repository branch protection nevertheless still independently requires status contextAnalyze (actions).Fresh exact-current CodeQL evidence remains non-passing before source execution:
33754522958, job100645612648Analyze (actions):status=queued,labels=[ubuntu-latest],runner_id=0, empty runner/group identity,steps=[]; siblingAnalyze (python)is terminalskipped;33754512403, job100645576780Analyze (actions):status=queued,runner_id=0, empty runner/group identity,steps=[].Other current-head checks have materialized but are not a landing GREEN; changed-scope work and the required
scan-pr-queuelane are still queued. The superseding cross-layer branch-protection/workflow and runner-acquisition evidence is recorded on canonical owner pathContextualWisdomLab/.github#712comment5525678933.Normal protected merge requires all applicable current-head required contexts terminal success on this unchanged exact head, zero valid unresolved findings, and the qualifying independent approval. No no-op retrigger commit, predecessor-success transfer, administrator bypass, self-approval, force update, or gate weakening is authorized.
Context Fabric / EA read-only boundary
ContextualWisdomLab/context-graph-contractsandContextualWisdomLab/enterprise-architecture-coreremain foreign-owner repositories on live defaultdevelop, exact tips99cb5468ba3c15c5e79688f53dee74724fae2d13and1c0fa8b15ceb9e72186274aeb255d6777eb84ef4. This PR writes neither repository and does not treat open PR heads as released authority.Boundary
No psychometric formula, estimator, model contract, public API, LLM/provider routing, temporal/event composition, Context Graph contract, or EA authority changes. This lane owns repository-local test-evidence governance and portability only.