Skip to content

Fix #21: Prevent pytest from re-running full suite on coverage failure#175

Open
TheDevCodingKen wants to merge 1 commit intoansible:develfrom
TheDevCodingKen:fix/issue-21-coverage-rerun
Open

Fix #21: Prevent pytest from re-running full suite on coverage failure#175
TheDevCodingKen wants to merge 1 commit intoansible:develfrom
TheDevCodingKen:fix/issue-21-coverage-rerun

Conversation

@TheDevCodingKen
Copy link

@TheDevCodingKen TheDevCodingKen commented Mar 11, 2026

Fixes #21

Summary

When pytest-cov triggers a failure due to the coverage threshold not being met, the pytest --lf command in the tox-rerun-posargs list evaluates an empty failure cache. By default, pytest responds to an empty --lf cache by re-running the entire test suite, which leads to inefficient use of CI resources.

This PR adds the --last-failed-no-failures none flag to the ci-cd.yml workflow. This overrides the default behavior, instructing pytest to fail fast and skip test execution if there are no actual functional test failures in the cache.

Local Verification

Reproduced the issue locally by temporarily raising the fail_under threshold in .coveragerc to force a coverage failure, then executing the rerun logic.

Before (Simulating coverage drop):

.tox/py/bin/python -m pytest --no-cov --lf

After (With flag applied):

.tox/py/bin/python -m pytest --no-cov --lf --last-failed-no-failures none

Result:
Pytest correctly identified no test failures and deselected the suite.

run-last-failure: no previously failed tests, deselecting all items.
========================= 132 deselected in 0.91s ==========================

Summary by CodeRabbit

  • Chores
    • Updated CI/CD configuration to improve test rerun behavior so retrying tests won’t error when there are no prior failures.

@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86dea13d-ec80-4bb9-a1d7-00609cc56c3c

📥 Commits

Reviewing files that changed from the base of the PR and between c757698 and 509615a.

📒 Files selected for processing (1)
  • .github/workflows/ci-cd.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/ci-cd.yml

📝 Walkthrough

Walkthrough

A single command-line argument --last-failed-no-failures none was added to the tox rerun positional arguments in the CI/CD workflow to avoid re-running the full test suite when no tests actually failed (e.g., coverage-only failures).

Changes

Cohort / File(s) Summary
CI/CD Workflow Configuration
.github/workflows/ci-cd.yml
Added --last-failed-no-failures none to tox-run-posargs to change tox rerun behavior when there are no previous test failures.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: preventing pytest from re-running the full suite on coverage failure, which directly addresses the issue being fixed.
Linked Issues check ✅ Passed The code change successfully implements the requirement from issue #21 by adding the --last-failed-no-failures none flag to prevent full suite reruns when coverage fails but no functional tests fail.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to the ci-cd.yml workflow file, adding only the necessary pytest flag to address the coverage failure rerun issue without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

Tip

CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.

Add a Ruff configuration file to your project to customize how CodeRabbit runs ruff.

@TheDevCodingKen TheDevCodingKen force-pushed the fix/issue-21-coverage-rerun branch from c757698 to 509615a Compare March 12, 2026 20:40
@webknjaz
Copy link
Member

I'm not sure we were in agreement regarding what the behavior should be in the end.

@webknjaz
Copy link
Member

I self-assigned that to myself because I wanted to do some more thinking. I'm not entirely convinced if we should be addressing this at all — this patch makes the step run and result in "hey, I ran but not really, and also here's your failure — guess why". I'm worried it'd make the UX even more confusing.
I vaguely recall thinking of improving the comms, though, outputting the explanation of why and how this step may fail. And I've done that in tox-dev/workflow@702b8ea + tox-dev/workflow@ab43dd1 + tox-dev/workflow@0b1e326.


This is mainly about the UX of the CI so local testing won't demonstrate anything interesting to us. If you want to see how this changes the experience for real — you can simulate the situation within your fork (make an additional PR with the simulation against devel in your fork, not the upstream repo).

@webknjaz webknjaz self-assigned this Mar 18, 2026
@webknjaz webknjaz self-requested a review March 18, 2026 08:55
@TheDevCodingKen
Copy link
Author

I self-assigned that to myself because I wanted to do some more thinking. I'm not entirely convinced if we should be addressing this at all — this patch makes the step run and result in "hey, I ran but not really, and also here's your failure — guess why". I'm worried it'd make the UX even more confusing. I vaguely recall thinking of improving the comms, though, outputting the explanation of why and how this step may fail. And I've done that in tox-dev/workflow@702b8ea + tox-dev/workflow@ab43dd1 + tox-dev/workflow@0b1e326.

This is mainly about the UX of the CI so local testing won't demonstrate anything interesting to us. If you want to see how this changes the experience for real — you can simulate the situation within your fork (make an additional PR with the simulation against devel in your fork, not the upstream repo).

Thanks for looking into this, @webknjaz. That makes complete sense. Optimizing CI resource usage is great, but I completely agree that if it comes at the cost of DevEx and confusing CI logs, it defeats the purpose.

I really appreciate the context on the UX side of things. I'll run a simulation in my personal fork as you suggested to see exactly what the GitHub Actions output looks like with this patch applied. Let me know if you decide to go a different route with the communication output, happy to pivot or close this out based on what's best for the project's DevEx.

@webknjaz
Copy link
Member

Dunno. Perhaps, --exitfirst in the first run makes sense instead, provided that everything will run in the rerun.

FWIW, #21 seems mainly about the DX, not the resources, though. cc @chrismeyersfsu

@TheDevCodingKen
Copy link
Author

Dunno. Perhaps, --exitfirst in the first run makes sense instead, provided that everything will run in the rerun.

FWIW, #21 seems mainly about the DX, not the resources, though. cc @chrismeyersfsu

That makes total sense regarding the DX focus. Shifting the fail-fast behavior to the initial run using --exitfirst is an interesting idea—it would definitely save the developer from waiting through a long initial run if a test breaks early.

I’ll hold off on running the simulation or pushing any alternative commits for now so you and @chrismeyersfsu can sync up on the best architectural approach for the pipeline's DevEx. Happy to implement whatever workflow you both decide is best.

@webknjaz
Copy link
Member

That makes total sense regarding the DX focus. Shifting the fail-fast behavior to the initial run using --exitfirst is an interesting idea—it would definitely save the developer from waiting through a long initial run if a test breaks early.

But this also means that such runs may result in reporting heavily reduced coverage metrics. So I've no definitive opinion on this thus far.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

re-run failed tests triggers on failed coverage

2 participants