Skip to content

fix: Ensure the first failure of a flaky test is annotated when using in tandem with pytest-rerunfailures#144

Merged
edgarrmondragon merged 2 commits into
mainfrom
132-annotate-all-failures-reruns
Jun 18, 2026
Merged

fix: Ensure the first failure of a flaky test is annotated when using in tandem with pytest-rerunfailures#144
edgarrmondragon merged 2 commits into
mainfrom
132-annotate-all-failures-reruns

Conversation

@edgarrmondragon

@edgarrmondragon edgarrmondragon commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

pytest-rerunfailures sets TestReport.outcome to "rerun" instead of "failed" (TestReport.failed is just self.outcome == "failed"), so we were skipping annotations for flaky test reruns and only annotating the final failure.

Related

… in tandem with `pytest-rerunfailures`

Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
@edgarrmondragon edgarrmondragon force-pushed the 132-annotate-all-failures-reruns branch from fc1793b to 8415dfa Compare June 17, 2026 23:21
@henryiii

Copy link
Copy Markdown
Contributor

Haven't had a chance to review this yet, dumping this here until I can review:

🤖 AI text below 🤖

Both new tests pass. Here's my review.


Code Review — PR #144: Annotate intermediate flaky-test failures with pytest-rerunfailures

Overview

pytest-rerunfailures reports intermediate failures with TestReport.outcome == "rerun" rather than "failed". Since report.failed is defined as outcome == "failed", the plugin silently skipped every rerun and only annotated the final attempt. This PR widens the guard to also act on the "rerun" outcome, adds two subprocess tests, wires pytest-rerunfailures into the dev deps, and loads it in CI. Tidy, well-scoped fix that does exactly what the title says.

Correctness ✅

  • The one-line change report.failed or report.outcome == "rerun" is the right place — it sits before all the longrepr extraction logic, which is outcome-agnostic, so rerun reports flow through the existing path unchanged.
  • when == "call" is preserved, so setup/teardown reruns stay unannotated — consistent with the plugin's existing call-phase-only policy.
  • Verified locally: both new tests pass (all-fail → 3 annotations, eventually-passes → 1).

Conventions ✅

  • Matches project style: behavior asserted on subprocess stderr (result.errlines), tests in top-level plugin_test.py, from __future__ import annotations present, version guards untouched.
  • The string literal "rerun" is the documented public outcome from pytest-rerunfailures; hardcoding it (vs. importing a constant) is the pragmatic norm and keeps rerunfailures a soft/optional dependency. Good call — the condition short-circuits safely when the plugin isn't installed (outcome is never "rerun").

Minor observations (non-blocking)

  • CI scope of PYTEST_PLUGINS: adding rerunfailures to the workflow's PYTEST_PLUGINS activates it across the entire outer suite, not just the two new tests (which already declare pytest_plugins/--reruns themselves in their generated subprocess files). It's harmless since --reruns defaults to 0, but strictly speaking it isn't required for the new tests to pass. Leaving it is fine — it does exercise that the plugins coexist.
  • xdist + rerunfailures interaction untested: the suite has an xdist dedup path (-n auto), and there's no test covering reruns under xdist. The controller re-emits worker reports, and rerun reports presumably ride the same path, so I'd expect it to work — but it's an unverified combination. Optional follow-up, not a blocker for this PR.
  • global _attempt test pattern: relies on module-level state persisting across reruns within one process. That's correct for rerunfailures (the module stays imported), and it's the simplest way to express "flaky" — no concern, just noting the assumption is load-bearing.

Risks

Low. The change only broadens when annotations fire, and only for a new outcome value that the plugin previously ignored entirely. No behavior change for users not running pytest-rerunfailures.

Verdict: approve. Clean, tested, minimal. The only thing I'd consider is an xdist+reruns test, but that's optional.

Signed-off-by: Edgar Ramírez Mondragón <edgarrm358@gmail.com>
@edgarrmondragon

Copy link
Copy Markdown
Collaborator Author

Addressed the "xdist + rerunfailures interaction" test suggestion.

@edgarrmondragon edgarrmondragon merged commit 8fa8b07 into main Jun 18, 2026
15 checks passed
@edgarrmondragon edgarrmondragon deleted the 132-annotate-all-failures-reruns branch June 18, 2026 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Changed behaviour when used with pytest-rerunfailures

2 participants