Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ci_visibility): handle pytest ATR retry failures in unittest classes [backport 2.19] #12047

Merged
merged 2 commits into from
Jan 28, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix(ci_visibility): handle pytest ATR retry failures in unittest clas…
…ses (#12030)

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)

(cherry picked from commit dc7037e)
vitor-de-araujo authored and github-actions[bot] committed Jan 23, 2025
commit 4a282a560d16bda7ca206e448e13dd16afb739d2
2 changes: 1 addition & 1 deletion ddtrace/contrib/pytest/_retry_utils.py
Original file line number Diff line number Diff line change
@@ -115,7 +115,7 @@ def _retry_run_when(item, when, outcomes: RetryOutcomes) -> t.Tuple[CallInfo, _p
)
else:
call = CallInfo.from_call(lambda: hook(item=item), when=when)
report = pytest.TestReport.from_item_and_call(item=item, call=call)
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":
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
fixes:
- |
CI Visibility: fixes an issue where Auto Test Retries with pytest would always consider retries of tests defined
inside unittest classes to be successful.
89 changes: 83 additions & 6 deletions tests/contrib/pytest/test_pytest_atr.py
Original file line number Diff line number Diff line change
@@ -24,27 +24,50 @@
)

_TEST_PASS_CONTENT = """
import unittest

def test_func_pass():
assert True

class SomeTestCase(unittest.TestCase):
def test_class_func_pass(self):
assert True
"""

_TEST_FAIL_CONTENT = """
import pytest
import unittest

def test_func_fail():
assert False

_test_func_retries_skip_count = 0

def test_func_retries_skip():
global _test_func_retries_skip_count
_test_func_retries_skip_count += 1
if _test_func_retries_skip_count > 1:
pytest.skip()
assert False

_test_class_func_retries_skip_count = 0

class SomeTestCase(unittest.TestCase):
def test_class_func_fail(self):
assert False

def test_class_func_retries_skip(self):
global _test_class_func_retries_skip_count
_test_class_func_retries_skip_count += 1
if _test_class_func_retries_skip_count > 1:
pytest.skip()
assert False
"""


_TEST_PASS_ON_RETRIES_CONTENT = """
import unittest

_test_func_passes_4th_retry_count = 0
def test_func_passes_4th_retry():
global _test_func_passes_4th_retry_count
@@ -56,10 +79,19 @@ def test_func_passes_1st_retry():
global _test_func_passes_1st_retry_count
_test_func_passes_1st_retry_count += 1
assert _test_func_passes_1st_retry_count == 2

class SomeTestCase(unittest.TestCase):
_test_func_passes_4th_retry_count = 0

def test_func_passes_4th_retry(self):
SomeTestCase._test_func_passes_4th_retry_count += 1
assert SomeTestCase._test_func_passes_4th_retry_count == 5

"""

_TEST_ERRORS_CONTENT = """
import pytest
import unittest

@pytest.fixture
def fixture_fails_setup():
@@ -79,13 +111,22 @@ def test_func_fails_teardown(fixture_fails_teardown):

_TEST_SKIP_CONTENT = """
import pytest
import unittest

@pytest.mark.skip
def test_func_skip_mark():
assert True

def test_func_skip_inside():
pytest.skip()

class SomeTestCase(unittest.TestCase):
@pytest.mark.skip
def test_class_func_skip_mark(self):
assert True

def test_class_func_skip_inside(self):
pytest.skip()
"""


@@ -109,7 +150,7 @@ def test_pytest_atr_no_ddtrace_does_not_retry(self):
self.testdir.makepyfile(test_pass_on_retries=_TEST_PASS_ON_RETRIES_CONTENT)
self.testdir.makepyfile(test_skip=_TEST_SKIP_CONTENT)
rec = self.inline_run()
rec.assertoutcome(passed=2, failed=6, skipped=2)
rec.assertoutcome(passed=3, failed=9, skipped=4)
assert len(self.pop_spans()) == 0

def test_pytest_atr_env_var_disables_retrying(self):
@@ -121,7 +162,7 @@ def test_pytest_atr_env_var_disables_retrying(self):

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

def test_pytest_atr_env_var_does_not_override_api(self):
@@ -137,7 +178,7 @@ def test_pytest_atr_env_var_does_not_override_api(self):
return_value=TestVisibilityAPISettings(flaky_test_retries_enabled=False),
):
rec = self.inline_run("--ddtrace", extra_env={"DD_CIVISIBILITY_FLAKY_RETRY_ENABLED": "1"})
rec.assertoutcome(passed=2, failed=6, skipped=2)
rec.assertoutcome(passed=3, failed=9, skipped=4)
assert len(self.pop_spans()) > 0

def test_pytest_atr_spans(self):
@@ -178,6 +219,15 @@ def test_pytest_atr_spans(self):
func_fail_retries += 1
assert func_fail_retries == 5

class_func_fail_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_fail")
assert len(class_func_fail_spans) == 6
class_func_fail_retries = 0
for class_func_fail_span in class_func_fail_spans:
assert class_func_fail_span.get_tag("test.status") == "fail"
if class_func_fail_span.get_tag("test.is_retry") == "true":
class_func_fail_retries += 1
assert class_func_fail_retries == 5

func_fail_skip_spans = _get_spans_from_list(spans, "test", "test_func_retries_skip")
assert len(func_fail_skip_spans) == 6
func_fail_skip_retries = 0
@@ -188,6 +238,18 @@ def test_pytest_atr_spans(self):
func_fail_skip_retries += 1
assert func_fail_skip_retries == 5

class_func_fail_skip_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_retries_skip")
assert len(class_func_fail_skip_spans) == 6
class_func_fail_skip_retries = 0
for class_func_fail_skip_span in class_func_fail_skip_spans:
class_func_fail_skip_is_retry = class_func_fail_skip_span.get_tag("test.is_retry") == "true"
assert class_func_fail_skip_span.get_tag("test.status") == (
"skip" if class_func_fail_skip_is_retry else "fail"
)
if class_func_fail_skip_is_retry:
class_func_fail_skip_retries += 1
assert class_func_fail_skip_retries == 5

func_pass_spans = _get_spans_from_list(spans, "test", "test_func_pass")
assert len(func_pass_spans) == 1
assert func_pass_spans[0].get_tag("test.status") == "pass"
@@ -205,7 +267,17 @@ def test_pytest_atr_spans(self):
assert func_skip_inside_spans[0].get_tag("test.status") == "skip"
assert func_skip_inside_spans[0].get_tag("test.is_retry") is None

assert len(spans) == 31
class_func_skip_mark_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_skip_mark")
assert len(class_func_skip_mark_spans) == 1
assert class_func_skip_mark_spans[0].get_tag("test.status") == "skip"
assert class_func_skip_mark_spans[0].get_tag("test.is_retry") is None

class_func_skip_inside_spans = _get_spans_from_list(spans, "test", "SomeTestCase::test_class_func_skip_inside")
assert len(class_func_skip_inside_spans) == 1
assert class_func_skip_inside_spans[0].get_tag("test.status") == "skip"
assert class_func_skip_inside_spans[0].get_tag("test.is_retry") is None

assert len(spans) == 51

def test_pytest_atr_fails_session_when_test_fails(self):
self.testdir.makepyfile(test_pass=_TEST_PASS_CONTENT)
@@ -216,7 +288,7 @@ def test_pytest_atr_fails_session_when_test_fails(self):
rec = self.inline_run("--ddtrace")
spans = self.pop_spans()
assert rec.ret == 1
assert len(spans) == 28
assert len(spans) == 48

def test_pytest_atr_passes_session_when_test_pass(self):
self.testdir.makepyfile(test_pass=_TEST_PASS_CONTENT)
@@ -226,9 +298,14 @@ def test_pytest_atr_passes_session_when_test_pass(self):
rec = self.inline_run("--ddtrace")
spans = self.pop_spans()
assert rec.ret == 0
assert len(spans) == 15
assert len(spans) == 23

def test_pytest_atr_does_not_retry_failed_setup_or_teardown(self):
# NOTE: This feature only works for regular pytest tests. For tests inside unittest classes, setup and teardown
# happens at the 'call' phase, and we don't have a way to detect that the error happened during setup/teardown,
# so tests will be retried as if they were failing tests.
# See <https://docs.pytest.org/en/8.3.x/how-to/unittest.html#pdb-unittest-note>.

self.testdir.makepyfile(test_errors=_TEST_ERRORS_CONTENT)
rec = self.inline_run("--ddtrace")
spans = self.pop_spans()