Skip to content

Commit 3ff7046

Browse files
chore(ci): fix test spans being incorrectly marked as failed (#13574)
The repo `tests/conftest.py` defines a `pytest_runtest_protocol` which was overriding the `pytest_runtest_protocol` hook from the Test Optimization pytest plugin, with a number of consequences: - Tests marked with `skip`, `skipif` or `subprocess` markers would not run the Test Optimization hook, causing their spans not to be finished (which in turned caused their module/suite/session not to be finished) and left in a `failed` stated. - The way the conftest.py hook invoked the builtin `pytest_runtest_protocol` directly with `nextitem=None` forced some session fixtures to be torn down and set up again in the next test, which masked a problem with a database fixture in the Django tests. This PR: - Changes the conftest.py hook to return `None` instead of calling the builtin `pytest_runtest_protocol` directly. This causes the next hook (Test Optimization) to run instead of the builtin one. - The Django test that uses a database in a subprocess was changed to not drop the database at the end of the subprocess, so it will still be available to the main process. Future work: - The tests with the `subprocess` marker are still being incorrectly sent as failed to Test Optimization (the pytest session passes normally). This will be fixed in a subsequent PR. ## 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)
1 parent d2e32b9 commit 3ff7046

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

tests/conftest.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import warnings
2626

2727
from _pytest.runner import call_and_report
28-
from _pytest.runner import pytest_runtest_protocol as default_pytest_runtest_protocol
2928
import pytest
3029

3130
import ddtrace
@@ -415,11 +414,11 @@ def pytest_collection_modifyitems(session, config, items):
415414
@pytest.hookimpl(tryfirst=True)
416415
def pytest_runtest_protocol(item):
417416
if item.get_closest_marker("skip"):
418-
return default_pytest_runtest_protocol(item, None)
417+
return None
419418

420419
skipif = item.get_closest_marker("skipif")
421420
if skipif and skipif.args[0]:
422-
return default_pytest_runtest_protocol(item, None)
421+
return None
423422

424423
marker = item.get_closest_marker("subprocess")
425424
if marker:

tests/contrib/django/test_django.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1556,7 +1556,8 @@ def test_connection(client, test_spans):
15561556
assert span.get_tag("django.db.alias") == "default"
15571557
15581558
if __name__ == "__main__":
1559-
sys.exit(pytest.main(["-x", __file__]))
1559+
# --reuse-db needed so the subprocess will not delete the main process database.
1560+
sys.exit(pytest.main(["-x", "--reuse-db", __file__]))
15601561
""".format(
15611562
expected_service_name
15621563
)

0 commit comments

Comments
 (0)