Skip to content

Commit 2ce46df

Browse files
fix(ci_visibility): handle pytest ATR retry failures in unittest classes [backport 2.20] (#12048)
Backport dc7037e from #12030 to 2.20. Auto Test Retries had a bug where retries of tests defined inside unittest classes would always succeed, even if the test failed. This was because for unittest classes, pytest saves the exception status in the [`pytest_runtest_makereport` hook](https://github.com/pytest-dev/pytest/blob/8.3.x/src/_pytest/unittest.py#L368), which was not called during retries. This PR fixes it so that the hook is called. ## 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 - [x] 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) Co-authored-by: Vítor De Araújo <[email protected]>
1 parent 07baf4c commit 2ce46df

File tree

3 files changed

+89
-7
lines changed

3 files changed

+89
-7
lines changed

ddtrace/contrib/internal/pytest/_retry_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def _retry_run_when(item, when, outcomes: RetryOutcomes) -> t.Tuple[CallInfo, _p
117117
)
118118
else:
119119
call = CallInfo.from_call(lambda: hook(item=item), when=when)
120-
report = pytest.TestReport.from_item_and_call(item=item, call=call)
120+
report = item.ihook.pytest_runtest_makereport(item=item, call=call)
121121
if report.outcome == "passed":
122122
report.outcome = outcomes.PASSED
123123
elif report.outcome == "failed" or report.outcome == "error":
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: fixes an issue where Auto Test Retries with pytest would always consider retries of tests defined
5+
inside unittest classes to be successful.

tests/contrib/pytest/test_pytest_atr.py

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,50 @@
2424
)
2525

2626
_TEST_PASS_CONTENT = """
27+
import unittest
28+
2729
def test_func_pass():
2830
assert True
31+
32+
class SomeTestCase(unittest.TestCase):
33+
def test_class_func_pass(self):
34+
assert True
2935
"""
3036

3137
_TEST_FAIL_CONTENT = """
3238
import pytest
39+
import unittest
3340
3441
def test_func_fail():
3542
assert False
3643
3744
_test_func_retries_skip_count = 0
45+
3846
def test_func_retries_skip():
3947
global _test_func_retries_skip_count
4048
_test_func_retries_skip_count += 1
4149
if _test_func_retries_skip_count > 1:
4250
pytest.skip()
4351
assert False
4452
53+
_test_class_func_retries_skip_count = 0
54+
55+
class SomeTestCase(unittest.TestCase):
56+
def test_class_func_fail(self):
57+
assert False
58+
59+
def test_class_func_retries_skip(self):
60+
global _test_class_func_retries_skip_count
61+
_test_class_func_retries_skip_count += 1
62+
if _test_class_func_retries_skip_count > 1:
63+
pytest.skip()
64+
assert False
4565
"""
4666

67+
4768
_TEST_PASS_ON_RETRIES_CONTENT = """
69+
import unittest
70+
4871
_test_func_passes_4th_retry_count = 0
4972
def test_func_passes_4th_retry():
5073
global _test_func_passes_4th_retry_count
@@ -56,10 +79,19 @@ def test_func_passes_1st_retry():
5679
global _test_func_passes_1st_retry_count
5780
_test_func_passes_1st_retry_count += 1
5881
assert _test_func_passes_1st_retry_count == 2
82+
83+
class SomeTestCase(unittest.TestCase):
84+
_test_func_passes_4th_retry_count = 0
85+
86+
def test_func_passes_4th_retry(self):
87+
SomeTestCase._test_func_passes_4th_retry_count += 1
88+
assert SomeTestCase._test_func_passes_4th_retry_count == 5
89+
5990
"""
6091

6192
_TEST_ERRORS_CONTENT = """
6293
import pytest
94+
import unittest
6395
6496
@pytest.fixture
6597
def fixture_fails_setup():
@@ -79,13 +111,22 @@ def test_func_fails_teardown(fixture_fails_teardown):
79111

80112
_TEST_SKIP_CONTENT = """
81113
import pytest
114+
import unittest
82115
83116
@pytest.mark.skip
84117
def test_func_skip_mark():
85118
assert True
86119
87120
def test_func_skip_inside():
88121
pytest.skip()
122+
123+
class SomeTestCase(unittest.TestCase):
124+
@pytest.mark.skip
125+
def test_class_func_skip_mark(self):
126+
assert True
127+
128+
def test_class_func_skip_inside(self):
129+
pytest.skip()
89130
"""
90131

91132

@@ -105,7 +146,7 @@ def test_pytest_atr_no_ddtrace_does_not_retry(self):
105146
self.testdir.makepyfile(test_pass_on_retries=_TEST_PASS_ON_RETRIES_CONTENT)
106147
self.testdir.makepyfile(test_skip=_TEST_SKIP_CONTENT)
107148
rec = self.inline_run()
108-
rec.assertoutcome(passed=2, failed=6, skipped=2)
149+
rec.assertoutcome(passed=3, failed=9, skipped=4)
109150
assert len(self.pop_spans()) == 0
110151

111152
def test_pytest_atr_env_var_disables_retrying(self):
@@ -117,7 +158,7 @@ def test_pytest_atr_env_var_disables_retrying(self):
117158

118159
with mock.patch("ddtrace.internal.ci_visibility.recorder.ddconfig", _get_default_civisibility_ddconfig()):
119160
rec = self.inline_run("--ddtrace", "-s", extra_env={"DD_CIVISIBILITY_FLAKY_RETRY_ENABLED": "0"})
120-
rec.assertoutcome(passed=2, failed=6, skipped=2)
161+
rec.assertoutcome(passed=3, failed=9, skipped=4)
121162
assert len(self.pop_spans()) > 0
122163

123164
def test_pytest_atr_env_var_does_not_override_api(self):
@@ -133,7 +174,7 @@ def test_pytest_atr_env_var_does_not_override_api(self):
133174
return_value=TestVisibilityAPISettings(flaky_test_retries_enabled=False),
134175
):
135176
rec = self.inline_run("--ddtrace", extra_env={"DD_CIVISIBILITY_FLAKY_RETRY_ENABLED": "1"})
136-
rec.assertoutcome(passed=2, failed=6, skipped=2)
177+
rec.assertoutcome(passed=3, failed=9, skipped=4)
137178
assert len(self.pop_spans()) > 0
138179

139180
def test_pytest_atr_spans(self):
@@ -174,6 +215,15 @@ def test_pytest_atr_spans(self):
174215
func_fail_retries += 1
175216
assert func_fail_retries == 5
176217

218+
class_func_fail_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_fail")
219+
assert len(class_func_fail_spans) == 6
220+
class_func_fail_retries = 0
221+
for class_func_fail_span in class_func_fail_spans:
222+
assert class_func_fail_span.get_tag("test.status") == "fail"
223+
if class_func_fail_span.get_tag("test.is_retry") == "true":
224+
class_func_fail_retries += 1
225+
assert class_func_fail_retries == 5
226+
177227
func_fail_skip_spans = _get_spans_from_list(spans, "test", "test_func_retries_skip")
178228
assert len(func_fail_skip_spans) == 6
179229
func_fail_skip_retries = 0
@@ -184,6 +234,18 @@ def test_pytest_atr_spans(self):
184234
func_fail_skip_retries += 1
185235
assert func_fail_skip_retries == 5
186236

237+
class_func_fail_skip_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_retries_skip")
238+
assert len(class_func_fail_skip_spans) == 6
239+
class_func_fail_skip_retries = 0
240+
for class_func_fail_skip_span in class_func_fail_skip_spans:
241+
class_func_fail_skip_is_retry = class_func_fail_skip_span.get_tag("test.is_retry") == "true"
242+
assert class_func_fail_skip_span.get_tag("test.status") == (
243+
"skip" if class_func_fail_skip_is_retry else "fail"
244+
)
245+
if class_func_fail_skip_is_retry:
246+
class_func_fail_skip_retries += 1
247+
assert class_func_fail_skip_retries == 5
248+
187249
func_pass_spans = _get_spans_from_list(spans, "test", "test_func_pass")
188250
assert len(func_pass_spans) == 1
189251
assert func_pass_spans[0].get_tag("test.status") == "pass"
@@ -201,7 +263,17 @@ def test_pytest_atr_spans(self):
201263
assert func_skip_inside_spans[0].get_tag("test.status") == "skip"
202264
assert func_skip_inside_spans[0].get_tag("test.is_retry") is None
203265

204-
assert len(spans) == 31
266+
class_func_skip_mark_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_skip_mark")
267+
assert len(class_func_skip_mark_spans) == 1
268+
assert class_func_skip_mark_spans[0].get_tag("test.status") == "skip"
269+
assert class_func_skip_mark_spans[0].get_tag("test.is_retry") is None
270+
271+
class_func_skip_inside_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_skip_inside")
272+
assert len(class_func_skip_inside_spans) == 1
273+
assert class_func_skip_inside_spans[0].get_tag("test.status") == "skip"
274+
assert class_func_skip_inside_spans[0].get_tag("test.is_retry") is None
275+
276+
assert len(spans) == 51
205277

206278
def test_pytest_atr_fails_session_when_test_fails(self):
207279
self.testdir.makepyfile(test_pass=_TEST_PASS_CONTENT)
@@ -212,7 +284,7 @@ def test_pytest_atr_fails_session_when_test_fails(self):
212284
rec = self.inline_run("--ddtrace")
213285
spans = self.pop_spans()
214286
assert rec.ret == 1
215-
assert len(spans) == 28
287+
assert len(spans) == 48
216288

217289
def test_pytest_atr_passes_session_when_test_pass(self):
218290
self.testdir.makepyfile(test_pass=_TEST_PASS_CONTENT)
@@ -222,9 +294,14 @@ def test_pytest_atr_passes_session_when_test_pass(self):
222294
rec = self.inline_run("--ddtrace")
223295
spans = self.pop_spans()
224296
assert rec.ret == 0
225-
assert len(spans) == 15
297+
assert len(spans) == 23
226298

227299
def test_pytest_atr_does_not_retry_failed_setup_or_teardown(self):
300+
# NOTE: This feature only works for regular pytest tests. For tests inside unittest classes, setup and teardown
301+
# happens at the 'call' phase, and we don't have a way to detect that the error happened during setup/teardown,
302+
# so tests will be retried as if they were failing tests.
303+
# See <https://docs.pytest.org/en/8.3.x/how-to/unittest.html#pdb-unittest-note>.
304+
228305
self.testdir.makepyfile(test_errors=_TEST_ERRORS_CONTENT)
229306
rec = self.inline_run("--ddtrace")
230307
spans = self.pop_spans()

0 commit comments

Comments
 (0)