Skip to content

feat: add --fail-threshold option - #197

Merged
aconrad merged 6 commits into
aconrad:masterfrom
paveltsialnou:master
Jan 31, 2026
Merged

feat: add --fail-threshold option#197
aconrad merged 6 commits into
aconrad:masterfrom
paveltsialnou:master

Conversation

@paveltsialnou

Copy link
Copy Markdown
Contributor

Add new --fail-under option that allows to return non-zero exit code when pycobertura show cobertura.xml is used.

@aconrad

aconrad commented Oct 6, 2025

Copy link
Copy Markdown
Owner

Hi @paveltsialnou, thanks for this, I always appreciate contributions.

I'm a bit torn about this PR because it seems to provide a way to return a non-zero exit code for the show command if the coverage report falls below a threshold given in percentage. I have a whole paragraph about why I don't want to proliferate the use of coverage percentage as the metric. The differential coverage is the best tool for that (pycobertura diff), although I understand people might not want to use diff (more moving parts to setup), and just use show instead.

If the provided metric was the number of uncovered lines instead of the coverage percentage, would that work for you?

@paveltsialnou

Copy link
Copy Markdown
Contributor Author

@aconrad thanks for sharing your thoughts. I agree that in many cases that approach gives a more stable signal than percentage‑based checks, and your example highlights that well.

At the same time, I’ve seen a lot of teams that either aren’t ready to set up historical baselines for diffs or are required by policy to keep overall coverage above a set percentage.

The --fail-under flag here is entirely optional, so it doesn’t change the default behaviour, but it gives those teams a way to enforce a minimum % coverage gate while still being able to adopt diff‑coverage philosophy later.

I’m happy to make sure the docs clearly promote uncovered‑lines and diff as the stronger metric for regression prevention, and position percentage as a secondary tool for those who need it.

@aconrad

aconrad commented Oct 8, 2025

Copy link
Copy Markdown
Owner

At the same time, I’ve seen a lot of teams that either aren’t ready to set up historical baselines for diffs or are required by policy to keep overall coverage above a set percentage.

With the diff tool, the baseline is simply the target branch’s coverage. It tells you whether coverage has increased or decreased with the new changes (typically in a PR).

The --fail-under flag here is entirely optional, so it doesn’t change the default behaviour

Absolutely, agreed.

but it gives those teams a way to enforce a minimum % coverage gate while still being able to adopt diff-coverage philosophy later.

Let’s walk through a quick example.

Say you set --fail-under to 80%, and your codebase currently sits at 85%. Everything passes for now. Ideally, teams continue improving coverage to 86%, 87%, and so on. But if coverage drops to 84%, 83%, etc., the build won’t actually fail until it dips below 80%.

In practice, no one wants to wait for coverage to fall that far before getting feedback, right? So the next logical step is to automatically update the baseline whenever coverage changes — for instance, setting the new baseline to the last known 85%.

However, when coverage is constantly near the baseline (as happens when you auto-update it), it leads to more false positives — as explained in this paragraph. You might see PRs fail at 84.98% even though there’s no meaningful coverage regression.

One possible middle ground could be similar to what you’re suggesting: keeping a "fail under" threshold, but based on the number of uncovered lines instead of a percentage. Whether that threshold is 80 (percent) or 18426 (uncovered lines) doesn’t really matter for CI purposes — it’s just a numeric gate that determines whether to fail the build. The uncovered-lines metric also works better with automatic baseline updates and avoids those false positives.

Finally, if coverage moves from 18,426 → 18,449 uncovered lines in a PR, it’s not immediately clear where coverage dropped — maybe a function stopped being called or dead code was introduced. That’s exactly why the diff command exists: to show, visually, which lines lost coverage compared to the target branch. But, as you said, it's a next step. I just wanted to illustrate the full circle.

So, what do you think about making --fail-under operate on the number of uncovered lines instead? Would that work for you and your team’s workflow?

@paveltsialnou

Copy link
Copy Markdown
Contributor Author

This isn’t going to work for me right now, but it might be useful for others. So, is the plan to use cobertura.total_misses()?

@aconrad

aconrad commented Oct 14, 2025

Copy link
Copy Markdown
Owner

This isn’t going to work for me right now, but it might be useful for others. So, is the plan to use cobertura.total_misses()?

Correct. That represents uncovered lines.

Can you elaborate a little more on how you plan to use pycobertura in your workflow when pycobertura returns a non-zero exit status due to the "fail under" option? Is it for informational purposes only? Or do you expect to take action to fix the coverage check? It would help me understand your use case and maybe spur more ideas. :)

@paveltsialnou

Copy link
Copy Markdown
Contributor Author

Is it for informational purposes only?

Yes, since pycobertura relies on an existing report, the stage in which a fix could be applied has already been completed.

@paveltsialnou

Copy link
Copy Markdown
Contributor Author

@aconrad have you seen the latest update yet?

Comment thread README.md Outdated
@aconrad
aconrad requested a review from Copilot November 6, 2025 18:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a --fail-threshold option to the pycobertura show command that enables returning a non-zero exit code when the total number of uncovered statements exceeds a specified threshold.

  • Introduces new exit code TOTAL_MISSES_ABOVE_THRESHOLD (value 4) for threshold failures
  • Adds --fail-threshold CLI option with minimum value validation (must be >= 1)
  • Implements exit code logic comparing total misses against the threshold

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
pycobertura/cli.py Adds new exit code constant, --fail-threshold option definition, and threshold validation logic
tests/test_cli.py Adds comprehensive test coverage for the new option including valid/invalid values and type checking
README.md Documents usage example for the new --fail-threshold option

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_cli.py Outdated
Comment thread tests/test_cli.py Outdated
Comment thread pycobertura/cli.py Outdated
Comment thread README.md Outdated
@paveltsialnou
paveltsialnou requested a review from aconrad November 9, 2025 20:39
paveltsialnou and others added 4 commits January 11, 2026 16:23
Co-authored-by: Alexandre Conrad-Dormoy <alexandre.conrad@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@paveltsialnou paveltsialnou changed the title feat: add --fail-under option feat: add --fail-threshold option Jan 11, 2026
@aconrad

aconrad commented Jan 11, 2026

Copy link
Copy Markdown
Owner

Thanks for this. Can you add an entry in the CHANGES file please, under the "Unreleased" paragraph (follow other release note examples for formatting)

Add `--fail-threshold` option to return non-zero exit code for uncovered lines exceeding threshold.

@aconrad aconrad left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@aconrad
aconrad merged commit 033b821 into aconrad:master Jan 31, 2026
5 checks passed
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.

3 participants