diff --git a/changes/230.feature.rst b/changes/230.feature.rst new file mode 100644 index 0000000..17e91a3 --- /dev/null +++ b/changes/230.feature.rst @@ -0,0 +1 @@ +Allow flaky marker conditions to inspect the exception that caused a failed test phase, including when running with pytest-xdist. diff --git a/docs/mark.rst b/docs/mark.rst index 7755178..2447b15 100644 --- a/docs/mark.rst +++ b/docs/mark.rst @@ -42,8 +42,10 @@ This will retry the test 5 times with a 2-second pause between attempts. ``condition`` ^^^^^^^^^^^^^ -Re-run the test only if a specified condition is met. -The condition can be any expression that evaluates to ``True`` or ``False``. +Re-run the test only if a specified condition is met. The condition can be a +boolean, a string to be evaluated, or a callable. + +Boolean conditions are evaluated directly: .. code-block:: python @@ -56,6 +58,40 @@ The condition can be any expression that evaluates to ``True`` or ``False``. In this example, the test will only be re-run if the operating system is Windows. +A callable condition that accepts one argument receives the exception that +caused a failed test phase. Existing zero-argument callables remain supported. +This allows a re-run decision to use exception attributes rather than only its +type or message: + +.. code-block:: python + + class TemporaryError(Exception): + def __init__(self, status): + self.status = status + + @pytest.mark.flaky( + reruns=3, + condition=lambda error: error.status in {429, 503}, + ) + def test_service_request(): + raise TemporaryError(429) + +A string condition can inspect the same exception through the reserved +``error`` name. Its evaluation context also contains ``os``, ``sys``, +``platform``, ``config`` (the pytest config object), and the test function's +globals: + +.. code-block:: python + + @pytest.mark.flaky(reruns=3, condition="error.status in {429, 503}") + def test_service_request(): + raise TemporaryError(429) + +When more than one test phase fails in an attempt, the test is re-run if the +condition matches any of those failures. Each failure is evaluated at most +once. If a callable or string condition raises an exception, pytest emits a +warning and does not re-run for that failure. + ``only_rerun`` ^^^^^^^^^^^^^^ diff --git a/src/pytest_rerunfailures.py b/src/pytest_rerunfailures.py index c50fecd..627287f 100644 --- a/src/pytest_rerunfailures.py +++ b/src/pytest_rerunfailures.py @@ -1,5 +1,6 @@ import hashlib import importlib.metadata +import inspect import os import platform import re @@ -302,21 +303,75 @@ def get_reruns_delay_backoff_factor(item): return factor -def get_reruns_condition(item): +def get_reruns_condition(item, failures=()): rerun_marker = _get_marker(item) - condition = True - if rerun_marker is not None and "condition" in rerun_marker.kwargs: - condition = evaluate_condition( - item, rerun_marker, rerun_marker.kwargs["condition"] - ) + if rerun_marker is None or "condition" not in rerun_marker.kwargs: + return True - return condition + condition = rerun_marker.kwargs["condition"] + condition_results = getattr(item, "_rerun_condition_results", {}) + failures = list(failures) + if not failures: + failures = [("attempt", 0, None)] + if not callable(condition) and not isinstance(condition, str): + failures = failures[:1] + + for phase, index, excinfo in failures: + cache_key = (phase, index) + if cache_key not in condition_results: + condition_results[cache_key] = evaluate_condition( + item, rerun_marker, condition, excinfo + ) + item._rerun_condition_results = condition_results + if condition_results[cache_key]: + return True + return False -def evaluate_condition(item, mark, condition: object) -> bool: +def _warn_condition_error(msglines): + """Report a bad condition without letting warning filters abort pytest.""" + try: + warnings.warn("\n".join(msglines)) + except Warning: + pass + + +def evaluate_condition(item, mark, condition: object, excinfo=None) -> bool: # copy from python3.8 _pytest.skipping.py + error = excinfo.value if excinfo is not None else None + + # Callable condition. + if callable(condition): + try: + try: + signature = inspect.signature(condition) + except (TypeError, ValueError): + call_with_error = True + else: + try: + signature.bind(error) + except TypeError: + try: + signature.bind() + except TypeError: + _warn_condition_error([ + f"Error evaluating {mark.name!r} condition as a callable", + "Condition callable must accept zero or one argument", + ]) + return False + call_with_error = False + else: + call_with_error = True + return bool(condition(error) if call_with_error else condition()) + except Exception as exc: + _warn_condition_error([ + f"Error evaluating {mark.name!r} condition as a callable", + *traceback.format_exception_only(type(exc), exc), + ]) + return False + result = False # String condition. if isinstance(condition, str): @@ -328,6 +383,7 @@ def evaluate_condition(item, mark, condition: object) -> bool: } if hasattr(item, "obj"): globals_.update(item.obj.__globals__) # type: ignore[attr-defined] + globals_["error"] = error try: filename = f"<{mark.name} condition>" condition_code = compile(condition, filename, "eval") @@ -339,14 +395,16 @@ def evaluate_condition(item, mark, condition: object) -> bool: " " + " " * (exc.offset or 0) + "^", "SyntaxError: invalid syntax", ] - fail("\n".join(msglines), pytrace=False) + _warn_condition_error(msglines) + return False except Exception as exc: msglines = [ f"Error evaluating {mark.name!r} condition", " " + condition, *traceback.format_exception_only(type(exc), exc), ] - fail("\n".join(msglines), pytrace=False) + _warn_condition_error(msglines) + return False # Boolean condition. else: @@ -585,19 +643,20 @@ def _should_hard_fail_on_error(item, report, excinfo): return (not matches_rerun_only) or matches_rerun_except -def _should_not_rerun(item, report, reruns): +def _should_not_rerun(item, report, reruns, condition): xfail = hasattr(report, "wasxfail") is_terminal_error = any(item._terminal_errors.values()) - condition = get_reruns_condition(item) has_failed_subtests = report.when == "call" and _get_num_failed_subtests(item) > 0 - return ( + if ( item.execution_count > reruns or (not report.failed and not has_failed_subtests) or xfail or is_terminal_error - or not condition - ) + ): + return True + + return not condition def is_master(config): @@ -948,6 +1007,37 @@ def _is_rerun_path_excluded(item): ) +def _get_reruns_condition_failures(item): + """Return each failed phase exception recorded during this attempt.""" + failed_statuses = getattr(item, "_test_failed_statuses", {}) + excinfos = getattr(item, "_rerun_condition_excinfos", {}) + failures = [] + for phase in ("setup", "call", "teardown"): + phase_excinfos = excinfos.get(phase, ()) + if phase_excinfos: + failures.extend( + (phase, index, excinfo) for index, excinfo in enumerate(phase_excinfos) + ) + elif failed_statuses.get(phase): + failures.append((phase, 0, None)) + if not failures and _get_num_failed_subtests(item) > 0: + failures.append(("call", 0, None)) + return failures + + +def _reruns_condition_matches_phase(item, phase): + """Return whether a failed phase matched the attempt's condition.""" + rerun_marker = _get_marker(item) + if rerun_marker is None or "condition" not in rerun_marker.kwargs: + return True + condition_results = getattr(item, "_rerun_condition_results", {}) + return any( + result + for (result_phase, _), result in condition_results.items() + if result_phase == phase + ) + + def _teardown_suspended_finalizers(item, call, report): """Tear down the scopes held back for a re-run that will not happen. @@ -1009,7 +1099,6 @@ def pytest_runtest_teardown(item, nextitem): return _test_failed_statuses = getattr(item, "_test_failed_statuses", {}) - max_suite_reruns = item.session.config.option.max_suite_reruns if ( max_suite_reruns is not None @@ -1020,15 +1109,15 @@ def pytest_runtest_teardown(item, nextitem): # Only remove non-function level actions from the stack if the test is to be re-run # Exceeding re-run limits, being free of failue statuses, encountering - # allowable exceptions, and a falsy flaky condition indicate that the test is - # not to be re-ran. A failure can also be carried by failed subtests alone, - # which leaves the call phase itself passing. + # allowable exceptions indicate that the test may need to be re-run. The + # final condition decision is made after teardown, when every failed phase + # is known. A failure can also be carried by failed subtests alone, which + # leaves the call phase itself passing. if ( item.execution_count <= reruns and (any(_test_failed_statuses.values()) or _get_num_failed_subtests(item) > 0) and not any(item._test_xfailed.values()) and not any(item._terminal_errors.values()) - and get_reruns_condition(item) ): # clean cached results from any level of setups _remove_cached_results_from_failed_fixtures(item) @@ -1061,8 +1150,18 @@ def pytest_runtest_makereport(item, call): # create a dict to store xfail results for each stage setattr(item, "_test_xfailed", {}) + # Keep exception state on the worker-side item. TestReport attributes + # are serialized by pytest-xdist and ExceptionInfo is not serializable. + setattr(item, "_rerun_condition_excinfos", {}) + setattr(item, "_rerun_condition_results", {}) + + if call.excinfo is not None and result.failed: + item._rerun_condition_excinfos.setdefault(result.when, []).append(call.excinfo) + _test_failed_statuses = getattr(item, "_test_failed_statuses", {}) - _test_failed_statuses[result.when] = result.failed + _test_failed_statuses[result.when] = ( + _test_failed_statuses.get(result.when, False) or result.failed + ) item._test_failed_statuses = _test_failed_statuses item._terminal_errors[result.when] = _should_hard_fail_on_error( item, result, call.excinfo @@ -1072,8 +1171,10 @@ def pytest_runtest_makereport(item, call): result.when, False ) or hasattr(result, "wasxfail") - if result.when == "teardown" and item._terminal_errors["teardown"]: - result = _teardown_suspended_finalizers(item, call, result) + if result.when == "teardown" and getattr(item, "_finalizers_suspended", False): + condition = get_reruns_condition(item, _get_reruns_condition_failures(item)) + if item._terminal_errors["teardown"] or not condition: + result = _teardown_suspended_finalizers(item, call, result) return result @@ -1117,10 +1218,25 @@ def pytest_runtest_protocol(item, nextitem): item.ihook.pytest_runtest_logstart(nodeid=item.nodeid, location=item.location) reports = runtestprotocol(item, nextitem=nextitem, log=False) + condition = get_reruns_condition(item, _get_reruns_condition_failures(item)) rerun_triggered = False for report in reports: # 3 reports: setup, call, teardown report.rerun = item.execution_count - 1 - if rerun_triggered or _should_not_rerun(item, report, reruns): + if rerun_triggered: + item.ihook.pytest_runtest_logreport(report=report) + elif ( + condition + and not _reruns_condition_matches_phase(item, report.when) + and ( + report.failed + or (report.when == "call" and _get_num_failed_subtests(item) > 0) + ) + ): + # Another failed phase matched the condition and will carry + # this intermediate attempt's rerun report. Do not publish a + # nonmatching failure as a final result first. + continue + elif _should_not_rerun(item, report, reruns, condition): # no rerun needed or one already triggered, log normally item.ihook.pytest_runtest_logreport(report=report) else: @@ -1151,6 +1267,10 @@ def pytest_runtest_protocol(item, nextitem): rerun_triggered = True + # Do not retain ExceptionInfo tracebacks and their frame locals for the + # lifetime of the collected item/session. + item._rerun_condition_excinfos.clear() + item._rerun_condition_results.clear() need_to_run = rerun_triggered item.ihook.pytest_runtest_logfinish(nodeid=item.nodeid, location=item.location) diff --git a/tests/test_pytest_rerunfailures.py b/tests/test_pytest_rerunfailures.py index f77460b..67122fa 100644 --- a/tests/test_pytest_rerunfailures.py +++ b/tests/test_pytest_rerunfailures.py @@ -1416,6 +1416,335 @@ def test_fail_two(): assert_outcomes(result, passed=0, failed=1, rerun=2) +@pytest.mark.parametrize( + "condition", + ["lambda error: error.status == 429", "'error.status == 429'"], +) +def test_condition_can_inspect_exception_attributes(testdir, condition): + testdir.makepyfile( + f""" + import pytest + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + attempts = 0 + + @pytest.mark.flaky(reruns=1, condition={condition}) + def test_retry_rate_limit(): + global attempts + attempts += 1 + if attempts == 1: + raise ServiceError(429) + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=1, rerun=1) + + +@pytest.mark.parametrize( + "condition", + ["lambda error: error.status == 429", "'error.status == 429'"], +) +def test_condition_rejects_nonmatching_exception_attributes(testdir, condition): + testdir.makepyfile( + f""" + import pytest + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + @pytest.mark.flaky(reruns=1, condition={condition}) + def test_do_not_retry_bad_request(): + raise ServiceError(400) + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=0, failed=1, rerun=0) + + +@pytest.mark.skipif(not has_xdist, reason="requires xdist") +@pytest.mark.parametrize( + "condition", + ["lambda error: error.status == 429", "'error.status == 429'"], +) +def test_exception_condition_works_with_xdist(testdir, condition): + testdir.makepyfile( + f""" + import pytest + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + attempts = 0 + + @pytest.mark.flaky(reruns=1, condition={condition}) + def test_retry_rate_limit(): + global attempts + attempts += 1 + if attempts == 1: + raise ServiceError(429) + """ + ) + + result = testdir.runpytest("-p", "xdist", "-n", "1") + assert result.ret != pytest.ExitCode.INTERNAL_ERROR + assert_outcomes(result, passed=1, rerun=1) + + +def test_callable_condition_error_prevents_rerun(testdir): + testdir.makepyfile( + """ + import pytest + + def broken_condition(error): + raise ValueError("condition failed") + + @pytest.mark.flaky(reruns=1, condition=broken_condition) + def test_failure(): + assert False + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=0, failed=1, rerun=0) + result.stdout.fnmatch_lines([ + "*UserWarning: Error evaluating 'flaky' condition as a callable*", + "*ValueError: condition failed*", + ]) + + +def test_callable_condition_error_respects_filterwarnings_error(testdir): + testdir.makeini("[pytest]\nfilterwarnings = error") + testdir.makepyfile( + """ + import pytest + + def broken_condition(error): + raise ValueError("condition failed") + + @pytest.mark.flaky(reruns=1, condition=broken_condition) + def test_failure(): + assert False + """ + ) + + result = testdir.runpytest() + assert result.ret != pytest.ExitCode.INTERNAL_ERROR + assert_outcomes(result, passed=0, failed=1, rerun=0) + + +def test_string_condition_error_prevents_rerun(testdir): + testdir.makepyfile( + """ + import pytest + + @pytest.mark.flaky(reruns=1, condition="error.status == 429") + def test_failure(): + assert False + """ + ) + + result = testdir.runpytest() + assert result.ret != pytest.ExitCode.INTERNAL_ERROR + assert_outcomes(result, passed=0, failed=1, rerun=0) + result.stdout.fnmatch_lines([ + "*UserWarning: Error evaluating 'flaky' condition*", + "*AttributeError: 'AssertionError' object has no attribute 'status'*", + ]) + + +def test_error_name_cannot_be_shadowed_by_test_globals(testdir): + testdir.makepyfile( + """ + import pytest + + error = None + attempts = 0 + + class ServiceError(Exception): + pass + + @pytest.mark.flaky(reruns=1, condition="isinstance(error, ServiceError)") + def test_failure(): + global attempts + attempts += 1 + if attempts == 1: + raise ServiceError + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=1, rerun=1) + + +def test_zero_argument_callable_condition_remains_supported(testdir): + testdir.makepyfile( + """ + import pytest + + attempts = 0 + + @pytest.mark.flaky(reruns=1, condition=lambda: True) + def test_failure(): + global attempts + attempts += 1 + assert attempts > 1 + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=1, rerun=1) + + +def test_callable_condition_is_evaluated_once_per_failure(testdir): + testdir.makepyfile( + """ + import pytest + + condition_calls = 0 + attempts = 0 + + def retry_assertion(error): + global condition_calls + condition_calls += 1 + return isinstance(error, AssertionError) + + @pytest.mark.flaky(reruns=1, condition=retry_assertion) + def test_retry_once(): + global attempts + attempts += 1 + if attempts == 1: + assert False + assert condition_calls == 1 + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=1, rerun=1) + + +def test_condition_uses_one_decision_for_call_and_teardown_failures(testdir): + testdir.makepyfile( + """ + import pytest + + attempts = 0 + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + @pytest.fixture + def service(): + yield + if attempts == 1: + raise ServiceError(503) + + @pytest.mark.flaky( + reruns=1, + condition=lambda error: getattr(error, "status", None) == 503, + ) + def test_service(service): + global attempts + attempts += 1 + if attempts == 1: + assert False + """ + ) + + result = testdir.runpytest() + assert result.ret == 0 + assert_outcomes(result, passed=1, rerun=1) + + +def test_condition_exception_state_is_released_after_attempt(testdir): + testdir.makepyfile( + """ + import pytest + + attempts = 0 + + @pytest.mark.flaky(reruns=1, condition=lambda error: True) + def test_retry(): + global attempts + attempts += 1 + assert attempts > 1 + + def test_exception_state_released(request): + retry_item = request.session.items[0] + assert retry_item._rerun_condition_excinfos == {} + assert retry_item._rerun_condition_results == {} + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=2, rerun=1) + + +def test_exception_condition_receives_setup_error(testdir): + testdir.makepyfile( + """ + import pytest + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + attempts = 0 + + @pytest.fixture + def service(): + global attempts + attempts += 1 + if attempts == 1: + raise ServiceError(503) + return object() + + @pytest.mark.flaky(reruns=1, condition=lambda error: error.status == 503) + def test_service(service): + assert service is not None + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=1, rerun=1) + + +def test_exception_condition_receives_teardown_error(testdir): + testdir.makepyfile( + """ + import pytest + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + teardowns = 0 + + @pytest.fixture + def service(): + yield object() + global teardowns + teardowns += 1 + if teardowns == 1: + raise ServiceError(503) + + @pytest.mark.flaky(reruns=1, condition=lambda error: error.status == 503) + def test_service(service): + assert service is not None + """ + ) + + result = testdir.runpytest() + assert_outcomes(result, passed=2, rerun=1) + + def test_reruns_with_string_condition_with_global_var(testdir): testdir.makepyfile( """ @@ -2597,6 +2926,41 @@ def test_subtests(subtests): assert_outcomes(result, passed=1, rerun=1) +@pytest.mark.skipif(not has_subtests, reason="Only supported on pytest 9.0 and newer") +def test_failing_subtest_condition_receives_its_exception_once(testdir): + testdir.makepyfile( + """ + import pytest + + attempts = 0 + condition_calls = 0 + + class ServiceError(Exception): + def __init__(self, status): + self.status = status + + def retry_rate_limit(error): + global condition_calls + condition_calls += 1 + return isinstance(error, ServiceError) and error.status == 429 + + @pytest.mark.flaky(reruns=1, condition=retry_rate_limit) + def test_subtests(subtests): + global attempts + attempts += 1 + with subtests.test("Fails on first attempt"): + if attempts == 1: + raise ServiceError(429) + if attempts == 2: + assert condition_calls == 1 + """ + ) + + result = testdir.runpytest() + assert result.ret == 0 + assert_outcomes(result, passed=1, rerun=1) + + @pytest.mark.skipif(not has_subtests, reason="Only supported on pytest 9.0 and newer") def test_unrelated_report_id_does_not_prevent_failing_subtest_rerun(testdir): testdir.makeconftest(