Skip to content

Commit ecc84b1

Browse files
vitor-de-araujovitor-de-araujo
authored
Revert "chore(ci_visibility): move retry logic to pytest_runtest_protocol (#13376)" (#13416)
This reverts commit 355694d. This change caused some errors with unittest (which did not run when the original PR was merged). ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [ ] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent 83dea4c commit ecc84b1

File tree

7 files changed

+186
-238
lines changed

7 files changed

+186
-238
lines changed

‎ddtrace/contrib/internal/pytest/_atr_utils.py

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,14 @@
44
import pytest
55

66
from ddtrace.contrib.internal.pytest._retry_utils import RetryOutcomes
7-
from ddtrace.contrib.internal.pytest._retry_utils import RetryReason
8-
from ddtrace.contrib.internal.pytest._retry_utils import UserProperty
97
from ddtrace.contrib.internal.pytest._retry_utils import _get_outcome_from_retry
108
from ddtrace.contrib.internal.pytest._retry_utils import _get_retry_attempt_string
119
from ddtrace.contrib.internal.pytest._retry_utils import set_retry_num
1210
from ddtrace.contrib.internal.pytest._types import _pytest_report_teststatus_return_type
1311
from ddtrace.contrib.internal.pytest._types import pytest_TestReport
1412
from ddtrace.contrib.internal.pytest._utils import _PYTEST_STATUS
15-
from ddtrace.contrib.internal.pytest._utils import TestPhase
1613
from ddtrace.contrib.internal.pytest._utils import _get_test_id_from_item
1714
from ddtrace.contrib.internal.pytest._utils import _TestOutcome
18-
from ddtrace.contrib.internal.pytest._utils import get_user_property
1915
from ddtrace.ext.test_visibility.api import TestStatus
2016
from ddtrace.internal.logger import get_logger
2117
from ddtrace.internal.test_visibility._internal_item_ids import InternalTestId
@@ -54,14 +50,11 @@ class _QUARANTINE_ATR_RETRY_OUTCOMES(_ATR_RETRY_OUTCOMES):
5450
def atr_handle_retries(
5551
test_id: InternalTestId,
5652
item: pytest.Item,
57-
test_reports: t.Dict[str, pytest_TestReport],
53+
when: str,
54+
original_result: pytest_TestReport,
5855
test_outcome: _TestOutcome,
5956
is_quarantined: bool = False,
6057
):
61-
setup_report = test_reports.get(TestPhase.SETUP)
62-
call_report = test_reports.get(TestPhase.CALL)
63-
teardown_report = test_reports.get(TestPhase.TEARDOWN)
64-
6558
if is_quarantined:
6659
retry_outcomes = _QUARANTINE_ATR_RETRY_OUTCOMES
6760
final_outcomes = _QUARANTINE_FINAL_OUTCOMES
@@ -77,14 +70,11 @@ def atr_handle_retries(
7770
XPASS=retry_outcomes.ATR_ATTEMPT_FAILED,
7871
)
7972

80-
item.ihook.pytest_runtest_logreport(report=setup_report)
81-
8273
# Overwrite the original result to avoid double-counting when displaying totals in final summary
83-
if call_report:
74+
if when == "call":
8475
if test_outcome.status == TestStatus.FAIL:
85-
call_report.outcome = outcomes.FAILED
86-
87-
item.ihook.pytest_runtest_logreport(report=call_report)
76+
original_result.outcome = outcomes.FAILED
77+
return
8878

8979
atr_outcome = _atr_do_retries(item, outcomes)
9080
longrepr = InternalTest.stash_get(test_id, "failure_longrepr")
@@ -93,14 +83,19 @@ def atr_handle_retries(
9383
nodeid=item.nodeid,
9484
location=item.location,
9585
keywords={k: 1 for k in item.keywords},
96-
when=TestPhase.CALL,
86+
when="call",
9787
longrepr=longrepr,
9888
outcome=final_outcomes[atr_outcome],
99-
user_properties=item.user_properties + [(UserProperty.RETRY_REASON, RetryReason.AUTO_TEST_RETRY)],
89+
user_properties=item.user_properties + [("dd_retry_reason", "auto_test_retry")],
10090
)
10191
item.ihook.pytest_runtest_logreport(report=final_report)
10292

103-
item.ihook.pytest_runtest_logreport(report=teardown_report)
93+
94+
def get_user_property(report, key, default=None):
95+
for k, v in report.user_properties:
96+
if k == key:
97+
return v
98+
return default
10499

105100

106101
def atr_get_failed_reports(terminalreporter: _pytest.terminal.TerminalReporter) -> t.List[pytest_TestReport]:
@@ -132,12 +127,12 @@ def _atr_write_report_for_status(
132127
markedup_strings: t.List[str],
133128
color: str,
134129
delete_reports: bool = True,
135-
retry_reason: str = RetryReason.AUTO_TEST_RETRY,
130+
retry_reason: str = "auto_test_retry",
136131
):
137132
reports = [
138133
report
139134
for report in terminalreporter.getreports(report_outcome)
140-
if get_user_property(report, UserProperty.RETRY_REASON) == retry_reason
135+
if get_user_property(report, "dd_retry_reason") == retry_reason
141136
]
142137
markup_kwargs = {color: True}
143138
if reports:

‎ddtrace/contrib/internal/pytest/_attempt_to_fix.py

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
from ddtrace.contrib.internal.pytest._retry_utils import set_retry_num
1111
from ddtrace.contrib.internal.pytest._types import _pytest_report_teststatus_return_type
1212
from ddtrace.contrib.internal.pytest._types import pytest_TestReport
13-
from ddtrace.contrib.internal.pytest._utils import TestPhase
1413
from ddtrace.contrib.internal.pytest._utils import _get_test_id_from_item
1514
from ddtrace.contrib.internal.pytest._utils import _TestOutcome
1615
from ddtrace.contrib.internal.pytest.constants import USER_PROPERTY_QUARANTINED
@@ -42,14 +41,10 @@ class _RETRY_OUTCOMES:
4241
def attempt_to_fix_handle_retries(
4342
test_id: InternalTestId,
4443
item: pytest.Item,
45-
test_reports: t.Dict[str, pytest_TestReport],
44+
when: str,
45+
original_result: pytest_TestReport,
4646
test_outcome: _TestOutcome,
47-
is_quarantined: bool = False,
4847
):
49-
setup_report = test_reports.get(TestPhase.SETUP)
50-
call_report = test_reports.get(TestPhase.CALL)
51-
teardown_report = test_reports.get(TestPhase.TEARDOWN)
52-
5348
retry_outcomes = _RETRY_OUTCOMES
5449
final_outcomes = _FINAL_OUTCOMES
5550

@@ -61,33 +56,28 @@ def attempt_to_fix_handle_retries(
6156
XPASS=retry_outcomes.ATTEMPT_FAILED,
6257
)
6358

64-
item.ihook.pytest_runtest_logreport(report=setup_report)
65-
6659
# Overwrite the original result to avoid double-counting when displaying totals in final summary
67-
if call_report:
60+
if when == "call":
6861
if test_outcome.status == TestStatus.FAIL:
69-
call_report.outcome = outcomes.FAILED
62+
original_result.outcome = outcomes.FAILED
7063
elif test_outcome.status == TestStatus.SKIP:
71-
call_report.outcome = outcomes.SKIPPED
72-
73-
item.ihook.pytest_runtest_logreport(report=call_report)
64+
original_result.outcome = outcomes.SKIPPED
65+
return
7466

7567
retries_outcome = _do_retries(item, outcomes)
7668
longrepr = InternalTest.stash_get(test_id, "failure_longrepr")
7769

7870
final_report = RetryTestReport(
7971
nodeid=item.nodeid,
8072
location=item.location,
81-
keywords={k: 1 for k in item.keywords},
82-
when=TestPhase.CALL,
73+
keywords=item.keywords,
74+
when="call",
8375
longrepr=longrepr,
8476
outcome=final_outcomes[retries_outcome],
8577
user_properties=item.user_properties,
8678
)
8779
item.ihook.pytest_runtest_logreport(report=final_report)
8880

89-
item.ihook.pytest_runtest_logreport(report=teardown_report)
90-
9181

9282
def _do_retries(item: pytest.Item, outcomes: RetryOutcomes) -> TestStatus:
9383
test_id = _get_test_id_from_item(item)

‎ddtrace/contrib/internal/pytest/_efd_utils.py

Lines changed: 39 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,15 @@
44
import pytest
55

66
from ddtrace.contrib.internal.pytest._retry_utils import RetryOutcomes
7-
from ddtrace.contrib.internal.pytest._retry_utils import RetryReason
8-
from ddtrace.contrib.internal.pytest._retry_utils import UserProperty
7+
from ddtrace.contrib.internal.pytest._retry_utils import RetryTestReport
98
from ddtrace.contrib.internal.pytest._retry_utils import _get_outcome_from_retry
109
from ddtrace.contrib.internal.pytest._retry_utils import _get_retry_attempt_string
1110
from ddtrace.contrib.internal.pytest._retry_utils import set_retry_num
1211
from ddtrace.contrib.internal.pytest._types import _pytest_report_teststatus_return_type
1312
from ddtrace.contrib.internal.pytest._types import pytest_TestReport
1413
from ddtrace.contrib.internal.pytest._utils import PYTEST_STATUS
15-
from ddtrace.contrib.internal.pytest._utils import TestPhase
1614
from ddtrace.contrib.internal.pytest._utils import _get_test_id_from_item
1715
from ddtrace.contrib.internal.pytest._utils import _TestOutcome
18-
from ddtrace.contrib.internal.pytest._utils import get_user_property
1916
from ddtrace.ext.test_visibility.api import TestStatus
2017
from ddtrace.internal.logger import get_logger
2118
from ddtrace.internal.test_visibility._efd_mixins import EFDTestStatus
@@ -40,76 +37,67 @@ class _EFD_RETRY_OUTCOMES:
4037
_EFD_FLAKY_OUTCOME = "flaky"
4138

4239
_FINAL_OUTCOMES: t.Dict[EFDTestStatus, str] = {
43-
EFDTestStatus.ALL_PASS: PYTEST_STATUS.PASSED,
44-
EFDTestStatus.ALL_FAIL: PYTEST_STATUS.FAILED,
45-
EFDTestStatus.ALL_SKIP: PYTEST_STATUS.SKIPPED,
46-
EFDTestStatus.FLAKY: PYTEST_STATUS.PASSED,
40+
EFDTestStatus.ALL_PASS: _EFD_RETRY_OUTCOMES.EFD_FINAL_PASSED,
41+
EFDTestStatus.ALL_FAIL: _EFD_RETRY_OUTCOMES.EFD_FINAL_FAILED,
42+
EFDTestStatus.ALL_SKIP: _EFD_RETRY_OUTCOMES.EFD_FINAL_SKIPPED,
43+
EFDTestStatus.FLAKY: _EFD_RETRY_OUTCOMES.EFD_FINAL_FLAKY,
4744
}
4845

4946

5047
def efd_handle_retries(
5148
test_id: InternalTestId,
5249
item: pytest.Item,
53-
test_reports: t.Dict[str, pytest_TestReport],
50+
when: str,
51+
original_result: pytest_TestReport,
5452
test_outcome: _TestOutcome,
55-
is_quarantined: bool = False,
5653
):
57-
setup_report = test_reports.get(TestPhase.SETUP)
58-
call_report = test_reports.get(TestPhase.CALL)
59-
teardown_report = test_reports.get(TestPhase.TEARDOWN)
60-
6154
# Overwrite the original result to avoid double-counting when displaying totals in final summary
62-
if call_report:
55+
if when == "call":
6356
if test_outcome.status == TestStatus.FAIL:
64-
call_report.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_FAILED
57+
original_result.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_FAILED
6558
elif test_outcome.status == TestStatus.PASS:
66-
call_report.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_PASSED
59+
original_result.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_PASSED
6760
elif test_outcome.status == TestStatus.SKIP:
68-
call_report.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED
69-
61+
original_result.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED
62+
return
7063
if InternalTest.get_tag(test_id, "_dd.ci.efd_setup_failed"):
7164
log.debug("Test item %s failed during setup, will not be retried for Early Flake Detection")
7265
return
73-
7466
if InternalTest.get_tag(test_id, "_dd.ci.efd_teardown_failed"):
7567
# NOTE: tests that passed their call but failed during teardown are not retried
7668
log.debug("Test item %s failed during teardown, will not be retried for Early Flake Detection")
7769
return
7870

7971
# If the test skipped (can happen either in setup or call depending on mark vs calling .skip()), we set the original
8072
# status as skipped and then continue handling retries because we may not return
81-
if test_outcome.status == TestStatus.SKIP:
82-
if call_report:
83-
call_report.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED
84-
else:
85-
# When skip happens during setup, we don't have a call report.
86-
setup_report.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED
87-
88-
item.ihook.pytest_runtest_logreport(report=setup_report)
89-
90-
if call_report:
91-
item.ihook.pytest_runtest_logreport(report=call_report)
73+
if test_outcome.status == TestStatus.SKIP and when in ["setup", "call"]:
74+
original_result.outcome = _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED
75+
# We don't return for when == call when skip happens during setup, so we need to log it and make sure the status
76+
# of the test is set
77+
if when == "setup":
78+
item.ihook.pytest_runtest_logreport(
79+
nodeid=item.nodeid,
80+
locationm=item.location,
81+
keywords=item.keywords,
82+
when="setup",
83+
longrepr=None,
84+
outcome=_EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED,
85+
)
86+
InternalTest.mark_skip(test_id)
9287

9388
efd_outcome = _efd_do_retries(item)
9489
longrepr = InternalTest.stash_get(test_id, "failure_longrepr")
9590

96-
final_report = pytest_TestReport(
91+
final_report = RetryTestReport(
9792
nodeid=item.nodeid,
9893
location=item.location,
99-
keywords={k: 1 for k in item.keywords},
100-
when=TestPhase.CALL,
94+
keywords=item.keywords,
95+
when="call",
10196
longrepr=longrepr,
10297
outcome=_FINAL_OUTCOMES[efd_outcome],
103-
user_properties=item.user_properties
104-
+ [
105-
(UserProperty.RETRY_REASON, RetryReason.EARLY_FLAKE_DETECTION),
106-
(UserProperty.RETRY_FINAL_OUTCOME, efd_outcome.value),
107-
],
10898
)
10999
item.ihook.pytest_runtest_logreport(report=final_report)
110100

111-
item.ihook.pytest_runtest_logreport(report=teardown_report)
112-
113101

114102
def efd_get_failed_reports(terminalreporter: _pytest.terminal.TerminalReporter) -> t.List[pytest_TestReport]:
115103
return terminalreporter.getreports(_EFD_RETRY_OUTCOMES.EFD_ATTEMPT_FAILED)
@@ -337,17 +325,14 @@ def efd_get_teststatus(report: pytest_TestReport) -> _pytest_report_teststatus_r
337325
"s",
338326
(f"EFD RETRY {_get_retry_attempt_string(report.nodeid)}SKIPPED", {"yellow": True}),
339327
)
340-
341-
if get_user_property(report, UserProperty.RETRY_REASON) == RetryReason.EARLY_FLAKE_DETECTION:
342-
efd_outcome = get_user_property(report, UserProperty.RETRY_FINAL_OUTCOME)
343-
if efd_outcome == "passed":
344-
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_PASSED, ".", ("EFD FINAL STATUS: PASSED", {"green": True}))
345-
if efd_outcome == "failed":
346-
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_FAILED, "F", ("EFD FINAL STATUS: FAILED", {"red": True}))
347-
if efd_outcome == "skipped":
348-
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_SKIPPED, "S", ("EFD FINAL STATUS: SKIPPED", {"yellow": True}))
349-
if efd_outcome == "flaky":
350-
# Flaky tests are the only one that have a pretty string because they are intended to be displayed in the
351-
# final count of terminal summary
352-
return (_EFD_FLAKY_OUTCOME, "K", ("EFD FINAL STATUS: FLAKY", {"yellow": True}))
328+
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_FINAL_PASSED:
329+
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_PASSED, ".", ("EFD FINAL STATUS: PASSED", {"green": True}))
330+
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_FINAL_FAILED:
331+
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_FAILED, "F", ("EFD FINAL STATUS: FAILED", {"red": True}))
332+
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_FINAL_SKIPPED:
333+
return (_EFD_RETRY_OUTCOMES.EFD_FINAL_SKIPPED, "S", ("EFD FINAL STATUS: SKIPPED", {"yellow": True}))
334+
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_FINAL_FLAKY:
335+
# Flaky tests are the only one that have a pretty string because they are intended to be displayed in the final
336+
# count of terminal summary
337+
return (_EFD_FLAKY_OUTCOME, "K", ("EFD FINAL STATUS: FLAKY", {"yellow": True}))
353338
return None

0 commit comments

Comments
 (0)