Repository navigation
Add --reruns-on-exitfirst to override rerun count under -x/--maxfail - #377
LouisDeconinck wants to merge 2 commits into
Conversation
With -x/--exitfirst (or --maxfail), reruns previously still ran to completion before the session could exit on the first failure. The new option sets the rerun count used in that situation, e.g. --reruns-on-exitfirst 0 to fail fast. Closes pytest-dev#249. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
sylvesterkaczmarek
left a comment
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
--reruns-on-exitfirst can enable reruns under --pdb without tripping the existing incompatibility check. check_options() only considers --force-reruns / the global rerun setting, while get_reruns_count() now returns this new value whenever maxfail is active. Thus pytest -x --pdb --reruns-on-exitfirst 1 is accepted and will rerun failures even though the plugin otherwise rejects reruns with PDB. Please include the new option in the --pdb validation and add that no-global-reruns case to the tests.
The option enables reruns under -x/--maxfail even without a global rerun count, so it bypassed the existing --pdb incompatibility check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Fixed in 7da4ffb: |
|
Fixed — |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
icemac
left a comment
icemac
left a comment
There was a problem hiding this comment.
— Comment created by Claude
Code review findings, posted as inline comments.
| if item.session.config.option.maxfail: | ||
| reruns = item.session.config.getoption("reruns_on_exitfirst") | ||
| if reruns is not None: | ||
| # -x/--exitfirst or --maxfail given: override marker and global | ||
| # rerun counts so the run can exit on the first real failure. | ||
| return reruns |
There was a problem hiding this comment.
— Comment created by Claude
@pytest.mark.flaky(reruns=0) opt-out is overridden. This branch runs before the marker lookup, so --reruns 2 -x --reruns-on-exitfirst 1 gives a test marked flaky(reruns=0) one rerun, even though the README documents that marker as the way to opt a test out (#376). An option meant to limit reruns under -x should not raise a test's count. Possible fix: cap with min(reruns_on_exitfirst, <regular count>) instead of replacing it, or at least keep an explicit reruns=0 marker.
| if item.session.config.option.maxfail: | ||
| reruns = item.session.config.getoption("reruns_on_exitfirst") | ||
| if reruns is not None: | ||
| # -x/--exitfirst or --maxfail given: override marker and global | ||
| # rerun counts so the run can exit on the first real failure. | ||
| return reruns |
There was a problem hiding this comment.
— Comment created by Claude
Turns on reruns for every test when no rerun setting exists. With only -x --reruns-on-exitfirst 2 (no --reruns, no ini, no marker), get_reruns_count now returns 2 for unmarked tests that used to return None, so every test in the suite gets 2 reruns. This contradicts the help text ("Defaults to the regular rerun settings") and the README ("limit reruns when exiting early"). The comment in check_options says this is intended. If it is, the docs should say so; if not, only apply the override when a regular count exists.
| reruns = config.getoption("force_reruns") or _get_global_reruns(config) | ||
| if not reruns and config.option.maxfail: | ||
| # --reruns-on-exitfirst takes effect under -x/--maxfail even when | ||
| # no other rerun count is configured | ||
| reruns = config.getoption("reruns_on_exitfirst") |
There was a problem hiding this comment.
— Comment created by Claude
Precedence here does not match get_reruns_count. This uses force > global > exitfirst, but get_reruns_count uses force > exitfirst > marker > global. So --reruns 2 -x --reruns-on-exitfirst 0 --pdb still raises --reruns incompatible with --pdb (a warning after merging master), even though the effective count is 0 for every test. test_no_error_when_run_with_pdb_and_zero_reruns_on_exitfirst only covers the case without --reruns. Suggest one helper for the global effective count (with the exitfirst override applied first) that both places call.
| def test_error_when_run_with_pdb_and_reruns_on_exitfirst(testdir): | ||
| testdir.makepyfile("def test_pass(): pass") | ||
| result = testdir.runpytest("-x", "--reruns-on-exitfirst", "1", "--pdb") | ||
| result.stderr.fnmatch_lines_random("ERROR: --reruns incompatible with --pdb") |
There was a problem hiding this comment.
— Comment created by Claude
Conflicts with master; this test will fail after the merge. Master (#374) changed --pdb + reruns from a UsageError to a warning, and the branch currently does not merge cleanly. Once it is rebased or merged, ERROR: --reruns incompatible with --pdb is no longer emitted, so this assertion fails. The second commit ("Reject --reruns-on-exitfirst under --pdb") should be redone against the new _warn_pdb_disables_reruns path.
| group._addoption( | ||
| "--reruns-on-exitfirst", | ||
| action="store", | ||
| dest="reruns_on_exitfirst", | ||
| type=int, | ||
| help="Number of reruns to use when -x/--exitfirst or --maxfail is " | ||
| "given. Defaults to the regular rerun settings. Use " | ||
| "--reruns-on-exitfirst 0 to skip reruns entirely when exiting early.", | ||
| ) |
There was a problem hiding this comment.
— Comment created by Claude
Silently ignored without -x/--maxfail. pytest --reruns 2 --reruns-on-exitfirst 0 without -x does nothing and gives no feedback, so a user who expects it to work (e.g. -x set via addopts in one environment only) never notices. Consider a warning when the option is given but maxfail is 0.
| def test_reruns_on_exitfirst_applies_to_maxfail(testdir): | ||
| testdir.makepyfile("def test_fail(): assert False") | ||
| result = testdir.runpytest( | ||
| "--reruns", "2", "--maxfail", "2", "--reruns-on-exitfirst", "0" | ||
| ) | ||
| assert_outcomes(result, passed=0, failed=1, rerun=0) |
There was a problem hiding this comment.
— Comment created by Claude
Test does not exercise --maxfail > 1. With one test and --maxfail 2, early exit never triggers. The test only checks that maxfail is truthy, which -x already covers. A test with several failing tests and --maxfail 2 would check the documented behavior: the count also applies to the failures before the limit is reached.
| group._addoption( | ||
| "--reruns-on-exitfirst", | ||
| action="store", | ||
| dest="reruns_on_exitfirst", | ||
| type=int, | ||
| help="Number of reruns to use when -x/--exitfirst or --maxfail is " | ||
| "given. Defaults to the regular rerun settings. Use " | ||
| "--reruns-on-exitfirst 0 to skip reruns entirely when exiting early.", | ||
| ) |
There was a problem hiding this comment.
— Comment created by Claude
No ini counterpart. reruns, reruns_delay, only_rerun, etc. can be set in the ini file, but this option is CLI-only. Projects that put -x/reruns in config cannot set it the same way unless they use addopts. Consider adding parser.addini("reruns_on_exitfirst", ...) for consistency.
3 participants
Summary
--reruns-on-exitfirst N: when-x/--exitfirstor--maxfailis given, the rerun count for every test becomesNinstead of the global/marker setting--reruns-on-exitfirst 0makes-xexit on the first failure immediately, without waiting for reruns--force-rerunsstill wins; negative values are rejected with aUsageErrorCloses #249
Test plan
-x --reruns-on-exitfirst 0disables global and marker reruns-x--maxfailtoo; behavior unchanged without the option; negative value rejected