Skip to content

Make the fail-under summary agree with the exit code - #751

Open
OnourImpram wants to merge 2 commits into
pytest-dev:masterfrom
OnourImpram:fix/fail-under-precision
Open

Make the fail-under summary agree with the exit code#751
OnourImpram wants to merge 2 commits into
pytest-dev:masterfrom
OnourImpram:fix/fail-under-precision

Conversation

@OnourImpram

Copy link
Copy Markdown

The defect

--cov-fail-under is decided twice, with two different comparisons.

Where Predicate
plugin.py pytest_sessionfinish should_fail_under(self.cov_total, cov_fail_under, cov_precision) — coverage.py's own, compares the total rounded to [report] precision
plugin.py pytest_terminal_summary failed = self.cov_total < self.options.cov_fail_under — the raw total

When the raw total is below the threshold but rounds up to meet it, the two disagree. The run exits 0 and prints a red FAIL banner at the same time.

Reproduced on pytest 9.1.1 / pytest-cov 7.1.0 / coverage 7.15.2, with the suite's own SCRIPT fixture (88.888…% covered) at the default precision of 0:

$ pytest --cov=... --cov-fail-under=88.9
FAIL Required test coverage of 88.9% not reached. Total coverage: 88.89%
============================= 10 passed in 0.20s ==============================
$ echo $?
0

round(88.888…, 0) is 89, so should_fail_under says pass and the exit code is 0 — correctly, given the configured precision. Only the banner disagrees.

The change

One line: the summary uses the predicate the exit-code path already uses.

The exit code does not change. Only the banner text and its colour move, so no build that passes today starts failing.

On the direction of the fix

The inconsistency is not in dispute, but the two open issues want it resolved in opposite directions, so this is a maintainer call and I want to be explicit about which way I went.

I chose to align the banner rather than the exit code because:

  1. should_fail_under is coverage.py's canonical predicate, and pytest_sessionfinish already delegates to it deliberately — the semantic choice has been made in this codebase.
  2. [report] precision / --cov-precision exists to declare the granularity that matters; at precision 0, 88.888…% is 89%.
  3. It is the non-breaking direction. Switching the exit code to the raw comparison would start failing CI runs that pass today.

If you prefer #728's reading, the change belongs on the other line and I am happy to redo it that way.

Not changed

The banner still hardcodes Total coverage: {actual:.2f}% rather than using cov_precision, so at precision 3 it can still print a differently-rounded number than the table above it. Fixing that changes an expected string in test_cov_min_float_value_not_reached, so I have left it alone rather than fold an unrelated behaviour change into this PR. Happy to follow up.

Tests

Two regression tests alongside the existing test_cov_min_float_value group — one at default precision, one mirroring #638 with precision = 1. Both fail on master and pass with the change.

Full suite on Windows / Python 3.13: 205 passed, 6 skipped (all pre-existing platform skips). ruff check and ruff format --diff clean.

The remaining fail-under tests are unaffected: their thresholds (88.88 @ p0, 88.89 @ p3, 100 @ p0, 50 @ p0) do not sit in the divergent band, so both predicates return the same boolean.

pytest-cov decides the --cov-fail-under verdict twice with two different
comparisons. pytest_sessionfinish uses coverage.py's should_fail_under,
which compares the total rounded to [report] precision. The terminal
summary compares the raw total with a bare `<`.

When the raw total is below the threshold but rounds up to meet it, the
two disagree: the run exits 0 while the summary prints a red
"FAIL Required test coverage of N% not reached".

Use the same predicate in both places. The exit code is unchanged; only
the banner and its colour move, so no currently-passing build starts
failing.

Adds two regression tests alongside the existing test_cov_min_float_value
group. Both fail on master and pass with this change; the rest of the
fail-under tests are unaffected because their thresholds do not sit in
the divergent band.

Refs pytest-dev#638. pytest-dev#728 reports the same inconsistency from the opposite
direction -- see the PR description.
@ionelmc

ionelmc commented Jul 27, 2026

Copy link
Copy Markdown
Member

Why does this pr show a huge removal/add change on plugin.py? Is that a display bug in github or what's going on?

The file was committed with CRLF endings, which rewrote all 477 lines and made
this pull request look like a full-file rewrite (+477 -471) instead of the
seven-line change it is. That is what the review question was about.

Endings are back to LF, matching upstream. The diff against master is now only
the fail-under predicate change and its two comments; nothing else in this file
differs.
@OnourImpram

Copy link
Copy Markdown
Author

Line endings, not a rewrite. The file had been committed with CRLF, so all 477 lines of plugin.py registered as changed. Restored to LF in f3121a1; the pull request now shows +43 −1.

The actual change in plugin.py is seven lines. pytest_sessionfinish decides the exit code with should_fail_under(cov_total, cov_fail_under, cov_precision), while the summary banner compared cov_total < cov_fail_under directly. At a precision boundary the two disagree and the run prints that required coverage was not reached while exiting 0, or the reverse. The banner now calls the same predicate, so it cannot contradict the exit status. The remaining diff is one regression test and a changelog entry.

Thanks for looking at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants