Skip to content

test: fail closed on skipped pytest evidence - #1733

Draft
seonghobae wants to merge 36 commits into
mainfrom
fix/no-hidden-pytest-outcomes
Draft

test: fail closed on skipped pytest evidence#1733
seonghobae wants to merge 36 commits into
mainfrom
fix/no-hidden-pytest-outcomes

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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 reset session.exitstatus to OK after this repository had classified skipped evidence as TESTS_FAILED, producing a real process exit 0 despite the fail-closed policy.

RED → GREEN lineage

  • RED 33ce5563dfd1a5bfe8a744214a3303b58161f789 exposed the original hidden non-execution surface; GREEN 3131f27d15ed4951e3a4fa5775b0effde80dca15 made skipped, xfailed, and xpassed terminal buckets non-passing and failed closed without terminal accounting.
  • Review RED 1adf41e946a21dc4e9c53d08ac3693770bcf0ef2 proved stronger pytest exit classifications could be overwritten; GREEN cbe0d836af8083f46f310e0d50788e0c88b88c1c preserves every pre-existing non-success exit and exercises a real child pytest process.
  • Capability RED 5dc238d25cd10e6889c9900d49714c793c19e01e proved missing descriptor-relative atomic-write prerequisites produced false passes; GREEN 3ab1ebf3a3e73632bd46d49998a009dee5f0a728 makes those prerequisites failing evidence.
  • Hook-order RED 1a75be806190b3e30784ced0419ddda79dadbf76 proved a later ordinary session-finish plugin could reset skip-derived failure to exit 0; GREEN 6430de94e78f6f65af2af47e90c8cd9bf321d163 makes 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:

  • descriptor-relative atomic-write evidence fails with the named missing POSIX primitive;
  • Rust objective/MMLE parity, multigroup/multilevel M2 ownership, marginal MMLE parity, CLI backend resolution, marginal ABI capability, Rust fit-pipeline smoke, public JMLE ownership, and public production/reference backend resolution require the compiled native core where Rust ownership is asserted;
  • explicit GPU requests on CPU-only hosts prove deterministic documented CPU fallback rather than returning a passing skip; usable-GPU hosts retain GPU-vs-CPU parity assertions;
  • extended-precision population-label, Brennan-Kane mastery-cut, WLE control-admission, CDM response-admission, and compensatory 2PL response-admission evidence now fails explicitly when the host longdouble cannot 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), and 102e116d7c0183ed77ab4e0ffaac0cc9cb551ccd (2PL). Current governed changelog/head is 07cc43803736500df145f8a597cf1b9f1ef142d4.

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 APPROVED review 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 07cc43803736500df145f8a597cf1b9f1ef142d4 on protected main@b5a3a0c1057d4b53d7a4bb18e0de69f630c2b45c. Every predecessor check is non-transferable after these source moves.

The inherited organization ruleset 18156473 requires one approval, resolved review threads, and nine central required workflows. Central CodeQL is intentionally not one of those nine after .github#1719 established that github/codeql-action cannot execute as a ruleset-required workflow. fast-mlsirm repository branch protection nevertheless still independently requires status context Analyze (actions).

Fresh exact-current CodeQL evidence remains non-passing before source execution:

  • repository CodeQL run 33754522958, job 100645612648 Analyze (actions): status=queued, labels=[ubuntu-latest], runner_id=0, empty runner/group identity, steps=[]; sibling Analyze (python) is terminal skipped;
  • PR/dynamic CodeQL run 33754512403, job 100645576780 Analyze (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-queue lane are still queued. The superseding cross-layer branch-protection/workflow and runner-acquisition evidence is recorded on canonical owner path ContextualWisdomLab/.github#712 comment 5525678933.

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-contracts and ContextualWisdomLab/enterprise-architecture-core remain foreign-owner repositories on live default develop, exact tips 99cb5468ba3c15c5e79688f53dee74724fae2d13 and 1c0fa8b15ceb9e72186274aeb255d6777eb84ef4. 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.


Devin Review

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: bf4b16e3-f57d-4404-a936-7b7b00a8074c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Pytest 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.

Changes

Pytest outcome governance

Layer / File(s) Summary
Outcome policy and session enforcement
tests/_outcome_policy.py, tests/conftest.py
The session-finish hook reads terminal statistics, reports non-execution counts, and sets TESTS_FAILED when prohibited outcomes or missing reporting occur.
Policy validation and changelog
tests/test_no_hidden_pytest_outcomes.py, docs/changelog.d/1732-fail-closed-pytest-outcomes.md
Tests verify clean-run status preservation, failure for each prohibited bucket, missing-reporter handling, zero-count filtering, and stable diagnostic ordering. The changelog documents the policy.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to 83801

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
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive 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 ca… Provide evidence from the required CI, package, and scientific jobs that the exact PR head has zero skipped, xfailed, and xpassed outcomes, or provide equivalent automated verification tied to that commit.
✅ Passed checks (4 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed All listed changes support issue #1732. The policy module, pytest session hook, focused tests, and changelog entry are directly related to fail-closed pytest outcome governance. No unrelated productio…
Docstring Coverage ✅ Passed 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 …
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 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 re…
Full details: Linked Issues check

Explanation

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 [#1732]. The provided context does not confirm that all required CI, package, and scientific jobs reported zero prohibited outcomes on the exact PR head.

Full details: Out of Scope Changes check

Explanation

All listed changes support issue #1732. The policy module, pytest session hook, focused tests, and changelog entry are directly related to fail-closed pytest outcome governance. No unrelated production, scientific, routing, or foreign-owner changes are present.

Full details: Docstring Coverage

Explanation

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 check

Explanation

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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/no-hidden-pytest-outcomes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae marked this pull request as draft September 3, 2026 12:40

Copy link
Copy Markdown
Contributor Author

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 64427a640983e5f849aad16c326697a688b646f6. Compensatory 2PL response-admission fail-closed evidence is adopted by #1646, whose stale ancestry was also repaired non-force onto protected main@b5a3a0c1057d4b53d7a4bb18e0de69f630c2b45c; current exact head is 7adde29f0e1214f539047632536be67f578ce377, behind_by=0.

#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.

@seonghobae seonghobae added maintenance priority: medium Normal-priority or P2 work type: maintenance Maintenance, build, dependency, or operational upkeep labels Sep 7, 2026 — with ChatGPT Codex Connector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance priority: medium Normal-priority or P2 work type: maintenance Maintenance, build, dependency, or operational upkeep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test governance: fail closed on skipped and expected-failure outcomes

1 participant