Skip to content

Commit 8c8ea55

Browse files
vitor-de-araujovitor-de-araujo
authored
fix(ci_visibility): report retry number correctly with pytest-xdist (#13884)
Currently we use the `core` context API to record the test retry number. This does not play well with `pytest-xdist`, where the test report is generated in a subprocess, but logged in the main process (where the context is not available). This PR changes it to store the retry number in the test's user properties. Those are serialized by xdist as part of the report and sent back to the main process, so we have access to them at logging time. ## 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 eafb0af commit 8c8ea55

File tree

7 files changed

+60
-39
lines changed

7 files changed

+60
-39
lines changed

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from ddtrace.contrib.internal.pytest._retry_utils import UserProperty
99
from ddtrace.contrib.internal.pytest._retry_utils import _get_outcome_from_retry
1010
from ddtrace.contrib.internal.pytest._retry_utils import _get_retry_attempt_string
11-
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
@@ -117,8 +116,7 @@ def _atr_do_retries(item: pytest.Item, outcomes: RetryOutcomes) -> TestStatus:
117116
while InternalTest.atr_should_retry(test_id):
118117
retry_num = InternalTest.atr_add_retry(test_id, start_immediately=True)
119118

120-
with set_retry_num(item.nodeid, retry_num):
121-
retry_outcome = _get_outcome_from_retry(item, outcomes)
119+
retry_outcome = _get_outcome_from_retry(item, outcomes, retry_num)
122120

123121
InternalTest.atr_finish_retry(
124122
test_id, retry_num, retry_outcome.status, retry_outcome.skip_reason, retry_outcome.exc_info
@@ -276,19 +274,19 @@ def atr_get_teststatus(report: pytest_TestReport) -> _pytest_report_teststatus_r
276274
return (
277275
_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_PASSED,
278276
"r",
279-
(f"ATR RETRY {_get_retry_attempt_string(report.nodeid)}PASSED", {"green": True}),
277+
(f"ATR RETRY {_get_retry_attempt_string(report)}PASSED", {"green": True}),
280278
)
281279
if report.outcome == _ATR_RETRY_OUTCOMES.ATR_ATTEMPT_FAILED:
282280
return (
283281
_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_FAILED,
284282
"R",
285-
(f"ATR RETRY {_get_retry_attempt_string(report.nodeid)}FAILED", {"yellow": True}),
283+
(f"ATR RETRY {_get_retry_attempt_string(report)}FAILED", {"yellow": True}),
286284
)
287285
if report.outcome == _ATR_RETRY_OUTCOMES.ATR_ATTEMPT_SKIPPED:
288286
return (
289287
_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_SKIPPED,
290288
"s",
291-
(f"ATR RETRY {_get_retry_attempt_string(report.nodeid)}SKIPPED", {"yellow": True}),
289+
(f"ATR RETRY {_get_retry_attempt_string(report)}SKIPPED", {"yellow": True}),
292290
)
293291
return None
294292

@@ -298,19 +296,19 @@ def quarantine_atr_get_teststatus(report: pytest_TestReport) -> _pytest_report_t
298296
return (
299297
_QUARANTINE_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_PASSED,
300298
"q",
301-
(f"QUARANTINED RETRY {_get_retry_attempt_string(report.nodeid)}PASSED", {"blue": True}),
299+
(f"QUARANTINED RETRY {_get_retry_attempt_string(report)}PASSED", {"blue": True}),
302300
)
303301
if report.outcome == _QUARANTINE_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_FAILED:
304302
return (
305303
_QUARANTINE_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_FAILED,
306304
"Q",
307-
(f"QUARANTINED RETRY {_get_retry_attempt_string(report.nodeid)}FAILED", {"blue": True}),
305+
(f"QUARANTINED RETRY {_get_retry_attempt_string(report)}FAILED", {"blue": True}),
308306
)
309307
if report.outcome == _QUARANTINE_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_SKIPPED:
310308
return (
311309
_QUARANTINE_ATR_RETRY_OUTCOMES.ATR_ATTEMPT_SKIPPED,
312310
"q",
313-
(f"QUARANTINED RETRY {_get_retry_attempt_string(report.nodeid)}SKIPPED", {"blue": True}),
311+
(f"QUARANTINED RETRY {_get_retry_attempt_string(report)}SKIPPED", {"blue": True}),
314312
)
315313
if report.outcome == _QUARANTINE_ATR_RETRY_OUTCOMES.ATR_FINAL_PASSED:
316314
return (

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from ddtrace.contrib.internal.pytest._retry_utils import UserProperty
99
from ddtrace.contrib.internal.pytest._retry_utils import _get_outcome_from_retry
1010
from ddtrace.contrib.internal.pytest._retry_utils import _get_retry_attempt_string
11-
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
@@ -105,8 +104,7 @@ def _do_retries(item: pytest.Item, outcomes: RetryOutcomes) -> TestStatus:
105104
while InternalTest.attempt_to_fix_should_retry(test_id):
106105
retry_num = InternalTest.attempt_to_fix_add_retry(test_id, start_immediately=True)
107106

108-
with set_retry_num(item.nodeid, retry_num):
109-
retry_outcome = _get_outcome_from_retry(item, outcomes)
107+
retry_outcome = _get_outcome_from_retry(item, outcomes, retry_num)
110108

111109
InternalTest.attempt_to_fix_finish_retry(
112110
test_id, retry_num, retry_outcome.status, retry_outcome.skip_reason, retry_outcome.exc_info
@@ -120,19 +118,19 @@ def attempt_to_fix_get_teststatus(report: pytest_TestReport) -> _pytest_report_t
120118
return (
121119
_RETRY_OUTCOMES.ATTEMPT_PASSED,
122120
"r",
123-
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report.nodeid)}PASSED", {"green": True}),
121+
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report)}PASSED", {"green": True}),
124122
)
125123
if report.outcome == _RETRY_OUTCOMES.ATTEMPT_FAILED:
126124
return (
127125
_RETRY_OUTCOMES.ATTEMPT_FAILED,
128126
"R",
129-
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report.nodeid)}FAILED", {"yellow": True}),
127+
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report)}FAILED", {"yellow": True}),
130128
)
131129
if report.outcome == _RETRY_OUTCOMES.ATTEMPT_SKIPPED:
132130
return (
133131
_RETRY_OUTCOMES.ATTEMPT_SKIPPED,
134132
"s",
135-
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report.nodeid)}SKIPPED", {"yellow": True}),
133+
(f"ATTEMPT TO FIX RETRY {_get_retry_attempt_string(report)}SKIPPED", {"yellow": True}),
136134
)
137135
if get_user_property(report, UserProperty.RETRY_REASON) == RetryReason.ATTEMPT_TO_FIX:
138136
final_outcome = get_user_property(report, UserProperty.RETRY_FINAL_OUTCOME)

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from ddtrace.contrib.internal.pytest._retry_utils import UserProperty
99
from ddtrace.contrib.internal.pytest._retry_utils import _get_outcome_from_retry
1010
from ddtrace.contrib.internal.pytest._retry_utils import _get_retry_attempt_string
11-
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
@@ -129,8 +128,7 @@ def _efd_do_retries(item: pytest.Item) -> EFDTestStatus:
129128
while InternalTest.efd_should_retry(test_id):
130129
retry_num = InternalTest.efd_add_retry(test_id, start_immediately=True)
131130

132-
with set_retry_num(item.nodeid, retry_num):
133-
retry_outcome = _get_outcome_from_retry(item, outcomes)
131+
retry_outcome = _get_outcome_from_retry(item, outcomes, retry_num)
134132

135133
InternalTest.efd_finish_retry(
136134
test_id, retry_num, retry_outcome.status, retry_outcome.skip_reason, retry_outcome.exc_info
@@ -323,19 +321,19 @@ def efd_get_teststatus(report: pytest_TestReport) -> _pytest_report_teststatus_r
323321
return (
324322
_EFD_RETRY_OUTCOMES.EFD_ATTEMPT_PASSED,
325323
"r",
326-
(f"EFD RETRY {_get_retry_attempt_string(report.nodeid)}PASSED", {"green": True}),
324+
(f"EFD RETRY {_get_retry_attempt_string(report)}PASSED", {"green": True}),
327325
)
328326
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_FAILED:
329327
return (
330328
_EFD_RETRY_OUTCOMES.EFD_ATTEMPT_FAILED,
331329
"R",
332-
(f"EFD RETRY {_get_retry_attempt_string(report.nodeid)}FAILED", {"yellow": True}),
330+
(f"EFD RETRY {_get_retry_attempt_string(report)}FAILED", {"yellow": True}),
333331
)
334332
if report.outcome == _EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED:
335333
return (
336334
_EFD_RETRY_OUTCOMES.EFD_ATTEMPT_SKIPPED,
337335
"s",
338-
(f"EFD RETRY {_get_retry_attempt_string(report.nodeid)}SKIPPED", {"yellow": True}),
336+
(f"EFD RETRY {_get_retry_attempt_string(report)}SKIPPED", {"yellow": True}),
339337
)
340338

341339
if get_user_property(report, UserProperty.RETRY_REASON) == RetryReason.EARLY_FLAKE_DETECTION:

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -692,16 +692,13 @@ def _process_result(item, result) -> _TestOutcome:
692692

693693

694694
def _pytest_runtest_makereport(item: pytest.Item, call: pytest_CallInfo, outcome: pytest_TestReport) -> None:
695+
original_result = outcome.get_result()
696+
695697
# When ATR or EFD retries are active, we do not want makereport to generate results
696-
if _pytest_version_supports_retries() and get_retry_num(item.nodeid) is not None:
698+
if _pytest_version_supports_retries() and get_retry_num(original_result) is not None:
697699
return
698700

699-
original_result = outcome.get_result()
700-
test_outcome = _process_result(item, original_result)
701-
702-
# A None value for test_outcome.status implies the test has not finished yet
703-
# Only continue to finishing the test if the test has finished, or if tearing down the test
704-
if test_outcome.status is None and call.when != TestPhase.TEARDOWN:
701+
if call.when != TestPhase.TEARDOWN:
705702
return
706703

707704
# Support for pytest-benchmark plugin

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,19 @@
55
from _pytest.runner import runtestprotocol
66
import pytest
77

8+
from ddtrace.contrib.internal.pytest._types import pytest_TestReport
89
from ddtrace.contrib.internal.pytest._utils import TestPhase
910
from ddtrace.contrib.internal.pytest._utils import _TestOutcome
1011
from ddtrace.contrib.internal.pytest._utils import excinfo_by_report
12+
from ddtrace.contrib.internal.pytest._utils import get_user_property
1113
from ddtrace.ext.test_visibility.api import TestExcInfo
1214
from ddtrace.ext.test_visibility.api import TestStatus
13-
from ddtrace.internal import core
1415

1516

1617
class UserProperty:
1718
RETRY_REASON = "dd_retry_reason"
1819
RETRY_FINAL_OUTCOME = "dd_retry_final_outcome"
20+
RETRY_NUMBER = "dd_retry_number"
1921

2022

2123
class RetryReason:
@@ -33,33 +35,37 @@ class RetryOutcomes:
3335
XPASS: str
3436

3537

36-
def get_retry_num(nodeid: str) -> t.Optional[int]:
37-
with core.context_with_data(f"dd-pytest-retry-{nodeid}") as ctx:
38-
return ctx.get_item("retry_num")
38+
def get_retry_num(report: pytest_TestReport) -> t.Optional[int]:
39+
return get_user_property(report, UserProperty.RETRY_NUMBER)
3940

4041

4142
@contextmanager
42-
def set_retry_num(nodeid: str, retry_num: int):
43-
with core.context_with_data(f"dd-pytest-retry-{nodeid}") as ctx:
44-
ctx.set_item("retry_num", retry_num)
43+
def set_retry_num(item: pytest.Item, retry_num: int):
44+
original_user_properties = item.user_properties
45+
try:
46+
item.user_properties = original_user_properties + [(UserProperty.RETRY_NUMBER, retry_num)]
4547
yield
48+
finally:
49+
item.user_properties = original_user_properties
4650

4751

48-
def _get_retry_attempt_string(nodeid) -> str:
49-
retry_number = get_retry_num(nodeid)
50-
return "ATTEMPT {} ".format(retry_number) if retry_number is not None else "INITIAL ATTEMPT "
52+
def _get_retry_attempt_string(report: pytest_TestReport) -> str:
53+
retry_number = get_retry_num(report)
54+
return "ATTEMPT {} ".format(retry_number) if retry_number else "INITIAL ATTEMPT "
5155

5256

5357
def _get_outcome_from_retry(
5458
item: pytest.Item,
5559
outcomes: RetryOutcomes,
60+
retry_number: int,
5661
) -> _TestOutcome:
5762
_outcome_status: t.Optional[TestStatus] = None
5863
_outcome_skip_reason: t.Optional[str] = None
5964
_outcome_exc_info: t.Optional[TestExcInfo] = None
6065

6166
item.ihook.pytest_runtest_logstart(nodeid=item.nodeid, location=item.location)
62-
reports = runtestprotocol(item, nextitem=None, log=False)
67+
with set_retry_num(item, retry_number):
68+
reports = runtestprotocol(item, nextitem=None, log=False)
6369

6470
if any(report.failed for report in reports):
6571
_outcome_status = TestStatus.FAIL

‎releasenotes/notes/ci_visibility-fix-pytest-xdist-retry-number-a6fee9bf3b14fd05.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
CI Visibility: This fix resolves an issue where test retry numbers were not reported correctly when tests were run
5+
with ``pytest-xdist``.

‎tests/contrib/pytest/test_pytest_xdist_atr.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,22 @@ def test_pytest_xdist_atr_junit_xml(self):
215215

216216
rec = self.inline_run("--ddtrace", "--junit-xml=out.xml")
217217
assert rec.ret == 1
218+
219+
def test_pytest_xdist_atr_retry_numbers_reported_correctly(self):
220+
self.testdir.makepyfile(
221+
test_fail="""
222+
def test_func_fail():
223+
assert False
224+
"""
225+
)
226+
result = self.subprocess_run("--ddtrace", "-v", "-n", "1")
227+
lines = [line for line in result.outlines if line.startswith("[gw0] [100%]")]
228+
assert lines == [
229+
"[gw0] [100%] ATR RETRY INITIAL ATTEMPT FAILED test_fail.py::test_func_fail ",
230+
"[gw0] [100%] ATR RETRY ATTEMPT 1 FAILED test_fail.py::test_func_fail ",
231+
"[gw0] [100%] ATR RETRY ATTEMPT 2 FAILED test_fail.py::test_func_fail ",
232+
"[gw0] [100%] ATR RETRY ATTEMPT 3 FAILED test_fail.py::test_func_fail ",
233+
"[gw0] [100%] ATR RETRY ATTEMPT 4 FAILED test_fail.py::test_func_fail ",
234+
"[gw0] [100%] ATR RETRY ATTEMPT 5 FAILED test_fail.py::test_func_fail ",
235+
"[gw0] [100%] FAILED test_fail.py::test_func_fail ",
236+
]

0 commit comments

Comments
 (0)