chore(ci_visibility): refactor test retry logic

ddtrace/contrib/internal/pytest/_plugin_v2.py

@@ -37,6 +37,7 @@
 from ddtrace.contrib.internal.pytest._utils import _pytest_version_supports_itr
 from ddtrace.contrib.internal.pytest._utils import _pytest_version_supports_retries
 from ddtrace.contrib.internal.pytest._utils import _TestOutcome
+from ddtrace.contrib.internal.pytest._utils import get_user_property
 from ddtrace.contrib.internal.pytest.constants import FRAMEWORK
 from ddtrace.contrib.internal.pytest.constants import USER_PROPERTY_QUARANTINED
 from ddtrace.contrib.internal.pytest.constants import XFAIL_REASON
@@ -49,7 +50,10 @@
 from ddtrace.ext.test_visibility.api import disable_test_visibility
 from ddtrace.ext.test_visibility.api import enable_test_visibility
 from ddtrace.ext.test_visibility.api import is_test_visibility_enabled
+from ddtrace.internal.ci_visibility.api._retry import ATRRetryManager
+from ddtrace.internal.ci_visibility.api._retry import EFDRetryManager
 from ddtrace.internal.ci_visibility.constants import SKIPPED_BY_ITR_REASON
+from ddtrace.internal.ci_visibility.recorder import CIVisibility
 from ddtrace.internal.ci_visibility.telemetry.coverage import COVERAGE_LIBRARY
 from ddtrace.internal.ci_visibility.telemetry.coverage import record_code_coverage_empty
 from ddtrace.internal.ci_visibility.telemetry.coverage import record_code_coverage_finished
@@ -81,10 +85,8 @@
 if _pytest_version_supports_atr():
     from ddtrace.contrib.internal.pytest._atr_utils import atr_get_failed_reports
     from ddtrace.contrib.internal.pytest._atr_utils import atr_get_teststatus
-    from ddtrace.contrib.internal.pytest._atr_utils import atr_handle_retries
-    from ddtrace.contrib.internal.pytest._atr_utils import atr_pytest_terminal_summary_post_yield
-    from ddtrace.contrib.internal.pytest._atr_utils import quarantine_atr_get_teststatus
-    from ddtrace.contrib.internal.pytest._atr_utils import quarantine_pytest_terminal_summary_post_yield
+    from ddtrace.contrib.internal.pytest._atr_utils import handle_retries
+    from ddtrace.contrib.internal.pytest._atr_utils import retry_pytest_terminal_summary_post_yield
 
 if _pytest_version_supports_attempt_to_fix():
     from ddtrace.contrib.internal.pytest._attempt_to_fix import attempt_to_fix_get_teststatus
@@ -515,10 +517,11 @@ def _pytest_runtest_makereport(item: pytest.Item, call: pytest_CallInfo, outcome
     original_result = outcome.get_result()
 
     test_id = _get_test_id_from_item(item)
+    test = CIVisibility.get_test_by_id(test_id)
 
-    is_quarantined = InternalTest.is_quarantined_test(test_id)
-    is_disabled = InternalTest.is_disabled_test(test_id)
-    is_attempt_to_fix = InternalTest.is_attempt_to_fix(test_id)
+    is_quarantined = test.is_quarantined()
+    is_disabled = test.is_disabled()
+    is_attempt_to_fix = test.is_attempt_to_fix()
 
     test_outcome = _process_result(item, call, original_result)
 
@@ -548,12 +551,15 @@ def _pytest_runtest_makereport(item: pytest.Item, call: pytest_CallInfo, outcome
     if InternalTest.stash_get(test_id, "setup_failed") or InternalTest.stash_get(test_id, "teardown_failed"):
         log.debug("Test %s failed during setup or teardown, skipping retries", test_id)
         return
-    if is_attempt_to_fix and _pytest_version_supports_attempt_to_fix():
-        return attempt_to_fix_handle_retries(test_id, item, call.when, original_result, test_outcome)
-    if InternalTestSession.efd_enabled() and InternalTest.efd_should_retry(test_id):
-        return efd_handle_retries(test_id, item, call.when, original_result, test_outcome)
-    if InternalTestSession.atr_is_enabled() and InternalTest.atr_should_retry(test_id):
-        return atr_handle_retries(test_id, item, call.when, original_result, test_outcome, is_quarantined)
+    # if is_attempt_to_fix and _pytest_version_supports_attempt_to_fix():
+    #     return attempt_to_fix_handle_retries(test_id, item, call.when, original_result, test_outcome)
+    # if InternalTestSession.efd_enabled() and InternalTest.efd_should_retry(test_id):
+    #     return efd_handle_retries(test_id, item, call.when, original_result, test_outcome)
+
+    for retry_class in [EFDRetryManager, ATRRetryManager]:
+        if retry_class.should_apply(test):
+            retry_manager = retry_class(test_id)
+            return handle_retries(retry_manager, item, call.when, original_result, test_outcome, is_quarantined)
 
 
 @pytest.hookimpl(hookwrapper=True)
@@ -607,14 +613,13 @@ def _pytest_terminal_summary_post_yield(terminalreporter, failed_reports_initial
         ]
 
     # IMPORTANT: terminal summary functions mutate terminalreporter.stats
-    if _pytest_version_supports_efd() and InternalTestSession.efd_enabled():
-        efd_pytest_terminal_summary_post_yield(terminalreporter)
+    # if _pytest_version_supports_efd() and InternalTestSession.efd_enabled():
+    #     efd_pytest_terminal_summary_post_yield(terminalreporter)
 
-    if _pytest_version_supports_atr() and InternalTestSession.atr_is_enabled():
-        atr_pytest_terminal_summary_post_yield(terminalreporter)
+    # if _pytest_version_supports_atr() and InternalTestSession.atr_is_enabled():
+    retry_pytest_terminal_summary_post_yield(terminalreporter)
 
-    quarantine_pytest_terminal_summary_post_yield(terminalreporter)
-    attempt_to_fix_pytest_terminal_summary_post_yield(terminalreporter)
+    # attempt_to_fix_pytest_terminal_summary_post_yield(terminalreporter)
 
     return
 
@@ -656,8 +661,8 @@ def _pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None:
 
     if InternalTestSession.efd_enabled() and InternalTestSession.efd_has_failed_tests():
         session.exitstatus = pytest.ExitCode.TESTS_FAILED
-    if InternalTestSession.atr_is_enabled() and InternalTestSession.atr_has_failed_tests():
-        session.exitstatus = pytest.ExitCode.TESTS_FAILED
+    # if InternalTestSession.atr_is_enabled() and InternalTestSession.atr_has_failed_tests():
+    #     session.exitstatus = pytest.ExitCode.TESTS_FAILED
     if InternalTestSession.attempt_to_fix_has_failed_tests():
         session.exitstatus = pytest.ExitCode.TESTS_FAILED
 
@@ -701,7 +706,7 @@ def pytest_report_teststatus(
             return test_status
 
     if _pytest_version_supports_atr() and InternalTestSession.atr_is_enabled():
-        test_status = atr_get_teststatus(report) or quarantine_atr_get_teststatus(report)
+        test_status = atr_get_teststatus(report)
         if test_status is not None:
             return test_status
 

ddtrace/contrib/internal/pytest/_retry_utils.py

@@ -16,13 +16,8 @@
 from ddtrace.internal import core
 
 
-@dataclass(frozen=True)
-class RetryOutcomes:
-    PASSED: str
-    FAILED: str
-    SKIPPED: str
-    XFAIL: str
-    XPASS: str
+RetryOutcomes = "OBSOLETE"
+RetryTestReport = "OBSOLETE"
 
 
 def get_retry_num(nodeid: str) -> t.Optional[int]:
@@ -44,8 +39,15 @@ def _get_retry_attempt_string(nodeid) -> str:
 
 def _get_outcome_from_retry(
     item: pytest.Item,
-    outcomes: RetryOutcomes,
+    retry_number: int,
 ) -> _TestOutcome:
+    class outcomes:
+        PASSED = "passed"
+        FAILED = "failed"
+        SKIPPED = "skipped"
+        XFAIL = "xfail"
+        XPASS = "xpass"
+
     _outcome_status: t.Optional[TestStatus] = None
     _outcome_skip_reason: t.Optional[str] = None
     _outcome_exc_info: t.Optional[TestExcInfo] = None
@@ -57,38 +59,39 @@ def _get_outcome_from_retry(
     item._report_sections = []
 
     # Setup
-    setup_call, setup_report = _retry_run_when(item, "setup", outcomes)
-    if setup_report.outcome == outcomes.FAILED:
+    setup_call, setup_report, setup_outcome = _retry_run_when(item, "setup", retry_number)
+    if setup_outcome == outcomes.FAILED:
         _outcome_status = TestStatus.FAIL
         if setup_call.excinfo is not None:
             _outcome_exc_info = TestExcInfo(setup_call.excinfo.type, setup_call.excinfo.value, setup_call.excinfo.tb)
             item.stash[caplog_records_key] = {}
             item.stash[caplog_handler_key] = {}
             if tmppath_result_key is not None:
                 item.stash[tmppath_result_key] = {}
-    if setup_report.outcome == outcomes.SKIPPED:
+    if setup_outcome == outcomes.SKIPPED:
         _outcome_status = TestStatus.SKIP
 
     # Call
-    if setup_report.outcome == outcomes.PASSED:
-        call_call, call_report = _retry_run_when(item, "call", outcomes)
-        if call_report.outcome == outcomes.FAILED:
+    if setup_outcome == outcomes.PASSED:
+        call_call, call_report, call_outcome = _retry_run_when(item, "call", retry_number)
+        if call_outcome == outcomes.FAILED:
             _outcome_status = TestStatus.FAIL
             if call_call.excinfo is not None:
                 _outcome_exc_info = TestExcInfo(call_call.excinfo.type, call_call.excinfo.value, call_call.excinfo.tb)
                 item.stash[caplog_records_key] = {}
                 item.stash[caplog_handler_key] = {}
                 if tmppath_result_key is not None:
                     item.stash[tmppath_result_key] = {}
-        elif call_report.outcome == outcomes.SKIPPED:
+        elif call_outcome == outcomes.SKIPPED:
             _outcome_status = TestStatus.SKIP
-        elif call_report.outcome == outcomes.PASSED:
+        elif call_outcome == outcomes.PASSED:
             _outcome_status = TestStatus.PASS
+
     # Teardown does not happen if setup skipped
-    if not setup_report.skipped:
-        teardown_call, teardown_report = _retry_run_when(item, "teardown", outcomes)
+    if not setup_outcome == outcomes.SKIPPED:
+        teardown_call, teardown_report, teardown_outcome = _retry_run_when(item, "teardown", retry_number)
         # Only override the outcome if the teardown failed, otherwise defer to either setup or call outcome
-        if teardown_report.outcome == outcomes.FAILED:
+        if teardown_outcome == outcomes.FAILED:
             _outcome_status = TestStatus.FAIL
             if teardown_call.excinfo is not None:
                 _outcome_exc_info = TestExcInfo(
@@ -104,58 +107,31 @@ def _get_outcome_from_retry(
     return _TestOutcome(status=_outcome_status, skip_reason=_outcome_skip_reason, exc_info=_outcome_exc_info)
 
 
-def _retry_run_when(item, when, outcomes: RetryOutcomes) -> t.Tuple[CallInfo, _pytest.reports.TestReport]:
+def _retry_run_when(item, when, retry_number: int) -> t.Tuple[CallInfo, _pytest.reports.TestReport]:
     hooks = {
         "setup": item.ihook.pytest_runtest_setup,
         "call": item.ihook.pytest_runtest_call,
         "teardown": item.ihook.pytest_runtest_teardown,
     }
     hook = hooks[when]
     # NOTE: we use nextitem=item here to make sure that logs don't generate a new line
+    #  ^ ꙮ I don't understand what this means. nextitem is not item here.
     if when == "teardown":
         call = CallInfo.from_call(
             lambda: hook(item=item, nextitem=pytest.Class.from_parent(item.session, name="forced_teardown")), when=when
         )
     else:
         call = CallInfo.from_call(lambda: hook(item=item), when=when)
     report = item.ihook.pytest_runtest_makereport(item=item, call=call)
-    if report.outcome == "passed":
-        report.outcome = outcomes.PASSED
-    elif report.outcome == "failed" or report.outcome == "error":
-        report.outcome = outcomes.FAILED
-    elif report.outcome == "skipped":
-        report.outcome = outcomes.SKIPPED
+    report.user_properties += [
+        ("dd_retry_reason", "auto_test_retry"),
+        ("dd_retry_outcome", report.outcome),
+        ("dd_retry_number", retry_number),
+    ]
+    original_outcome = report.outcome
+    report.outcome = "retry"
+
     # Only log for actual test calls, or failures
-    if when == "call" or "passed" not in report.outcome:
+    if when == "call" or "passed" not in original_outcome:
         item.ihook.pytest_runtest_logreport(report=report)
-    return call, report
-
-
-class RetryTestReport(pytest_TestReport):
-    """
-    A RetryTestReport behaves just like a normal pytest TestReport, except that the the failed/passed/skipped
-    properties are aware of retry final states (dd_efd_final_*, etc). This affects the test counts in JUnit XML output,
-    for instance.
-
-    The object should be initialized with the `longrepr` of the _initial_ test attempt. A `longrepr` set to `None` means
-    the initial attempt either succeeded (which means it was already counted by pytest) or was quarantined (which means
-    we should not count it at all), so we don't need to count it here.
-    """
-
-    @property
-    def failed(self):
-        if self.longrepr is None:
-            return False
-        return "final_failed" in self.outcome
-
-    @property
-    def passed(self):
-        if self.longrepr is None:
-            return False
-        return "final_passed" in self.outcome or "final_flaky" in self.outcome
-
-    @property
-    def skipped(self):
-        if self.longrepr is None:
-            return False
-        return "final_skipped" in self.outcome
+    return call, report, original_outcome

ddtrace/contrib/internal/pytest/_utils.py

@@ -230,3 +230,10 @@ class _TestOutcome(t.NamedTuple):
     status: t.Optional[TestStatus] = None
     skip_reason: t.Optional[str] = None
     exc_info: t.Optional[TestExcInfo] = None
+
+
+def get_user_property(report, key, default=None):
+    for k, v in report.user_properties:
+        if k == key:
+            return v
+    return default