Skip to content

Commit 310ca53

Browse files
gnufededubloom
authored andcommitted
ci: expand test optimization dogfooding (#17567)
## Description Removes all remaining `--no-ddtrace` flags from riotfile.py, expanding dogfooding of the new Test Optimization plugin (`ddtrace/testing`) to more test suites. Most of these `--no-ddtrace` flags were originally added when the old `_plugin_v2.py`-backed plugin was in use for outer sessions. Now that the new plugin (`ddtrace/testing`) is the default and is used for outer sessions, they are no longer needed. **Inner sessions** (spawned by tests such as pytest-bdd, pytest-benchmark, pytest-flaky, and selenium) are not affected: - `inline_run`-based inner sessions (pytest-bdd, pytest-benchmark, pytest-flaky) use `PytestTestCaseBase.inline_run()`, which suspends/resumes the outer CIVisibility instance and explicitly controls the `--ddtrace` flag per inner session. - Subprocess-based inner sessions (selenium) are isolated processes and unaffected by the outer session's plugin state. For now, all inner sessions use the new plugin (since `DD_PYTEST_USE_NEW_PLUGIN=false` in `_ci_override_env` has no effect on already-imported modules in in-process sessions). This is intentional — the new plugin is the focus of current efforts. --- Also addresses comment by codex: > In the GitLab riot workflow (.gitlab/tests.yml, line 43), every suite is invoked with --ddtrace, so removing --no-ddtrace here enables CI Visibility in the outer pytest process for this suite. That breaks assumptions in tests/integration/test_integration_civisibility.py (for example, test_civisibility_intake_with_missing_apikey expects CIVisibility to start disabled and asserts _instance is None after enable()), because a pre-existing singleton makes CIVisibility.enable() return early and bypasses the test’s patched initialization path. By removing those env vars in conftest.py with a fixture, and also pause/resume any CIVisibility instance (not needed for ddtrace/testing plugin really, but it is needed if we switched to v2). ## Testing Covered by the dogfooded test suites themselves running under the new plugin. ## Risks Low. The new plugin is already the default. Removing `--no-ddtrace` only affects the outer pytest session instrumentation; inner sessions already control their own plugin activation. ## Additional Notes Suites like `pytest_bdd` and `pytest_benchmark` have separate bdd/benchmark implementations in both plugins. Future work could add explicit coverage for the new plugin's code paths for those integrations. Co-authored-by: federico.mon <federico.mon@datadoghq.com>
1 parent 222a314 commit 310ca53

2 files changed

Lines changed: 37 additions & 13 deletions

File tree

riotfile.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
132132
Venv(
133133
name="meta-testing",
134134
pys=["3.10"],
135-
command="pytest {cmdargs} --no-ddtrace tests/meta",
135+
command="pytest {cmdargs} tests/meta",
136136
env={
137137
"DD_CIVISIBILITY_FLAKY_RETRY_ENABLED": "0",
138138
},
@@ -475,7 +475,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
475475
name="integration",
476476
# Enabling coverage for integration tests breaks certain tests in CI
477477
# Also, running two separate pytest sessions, the ``civisibility`` one with --no-ddtrace
478-
command="pytest -vv --no-ddtrace --no-cov --ignore-glob='*civisibility*' {cmdargs} tests/integration/",
478+
command="pytest -vv --no-cov --ignore-glob='*civisibility*' {cmdargs} tests/integration/",
479479
pkgs={"msgpack": [latest], "coverage": latest, "pytest-randomly": latest},
480480
pys=select_pys(),
481481
venvs=[
@@ -497,7 +497,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
497497
name="integration-civisibility",
498498
# Enabling coverage for integration tests breaks certain tests in CI
499499
# Also, running two separate pytest sessions, the ``civisibility`` one with --no-ddtrace
500-
command="pytest --no-cov --no-ddtrace {cmdargs} tests/integration/test_integration_civisibility.py",
500+
command="pytest --no-cov {cmdargs} tests/integration/test_integration_civisibility.py",
501501
pkgs={"msgpack": [latest], "coverage": latest, "pytest-randomly": latest},
502502
pys=select_pys(),
503503
venvs=[
@@ -2068,7 +2068,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
20682068
),
20692069
Venv(
20702070
name="unittest",
2071-
command="pytest --no-ddtrace {cmdargs} tests/contrib/unittest/",
2071+
command="pytest {cmdargs} tests/contrib/unittest/",
20722072
pkgs={
20732073
"msgpack": latest,
20742074
"pytest-randomly": latest,
@@ -2084,7 +2084,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
20842084
),
20852085
Venv(
20862086
name="asynctest",
2087-
command="pytest --no-ddtrace {cmdargs} tests/contrib/asynctest/",
2087+
command="pytest {cmdargs} tests/contrib/asynctest/",
20882088
pkgs={
20892089
"pytest-randomly": latest,
20902090
},
@@ -2105,7 +2105,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
21052105
),
21062106
Venv(
21072107
name="pytest_bdd",
2108-
command="pytest --no-ddtrace {cmdargs} tests/contrib/pytest_bdd/",
2108+
command="pytest {cmdargs} tests/contrib/pytest_bdd/",
21092109
pkgs={
21102110
"msgpack": latest,
21112111
"more_itertools": "<8.11.0",
@@ -2140,7 +2140,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
21402140
Venv(
21412141
name="pytest_benchmark",
21422142
pys=select_pys(),
2143-
command="pytest {cmdargs} --no-ddtrace --no-cov tests/contrib/pytest_benchmark/",
2143+
command="pytest {cmdargs} --no-cov tests/contrib/pytest_benchmark/",
21442144
pkgs={
21452145
"msgpack": latest,
21462146
"pytest-randomly": latest,
@@ -2161,7 +2161,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
21612161
Venv(
21622162
name="pytest:flaky",
21632163
pys=select_pys(),
2164-
command="pytest {cmdargs} --no-ddtrace --no-cov -p no:flaky tests/contrib/pytest_flaky/",
2164+
command="pytest {cmdargs} --no-cov -p no:flaky tests/contrib/pytest_flaky/",
21652165
pkgs={
21662166
"flaky": latest,
21672167
"pytest-randomly": latest,
@@ -3358,7 +3358,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
33583358
),
33593359
Venv(
33603360
name="aws_lambda",
3361-
command="pytest --no-ddtrace {cmdargs} tests/contrib/aws_lambda",
3361+
command="pytest {cmdargs} tests/contrib/aws_lambda",
33623362
pys=select_pys(min_version="3.9", max_version="3.13"),
33633363
pkgs={
33643364
"boto3": latest,
@@ -3874,7 +3874,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
38743874
"selenium": "~=4.0",
38753875
"webdriver-manager": latest,
38763876
},
3877-
command="pytest --no-cov {cmdargs} -c /dev/null --no-ddtrace tests/contrib/selenium",
3877+
command="pytest --no-cov {cmdargs} -c /dev/null tests/contrib/selenium",
38783878
env={
38793879
"DD_AGENT_PORT": "9126",
38803880
},

tests/integration/test_integration_civisibility.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import os
2+
from unittest import mock
23

3-
import mock
44
import pytest
55

66
from ddtrace.internal.ci_visibility import CIVisibility
@@ -18,6 +18,31 @@
1818
AGENT_VERSION = os.environ.get("AGENT_VERSION")
1919

2020

21+
@pytest.fixture(autouse=True)
22+
def _isolate_from_outer_session(monkeypatch):
23+
"""Strip DD_* env vars and suspend CIVisibility so each test starts with a clean slate.
24+
25+
When the outer pytest session runs with --ddtrace in CI, DD_API_KEY and other DD_* vars
26+
are present in the process environment. Tests asserting on specific env conditions (e.g.
27+
missing API key) must not see those vars. Each test sets its own DD_* vars explicitly via
28+
override_env(), so stripping them all here is safe.
29+
30+
The CIVisibility suspension is belt-and-suspenders for the old plugin path where a
31+
pre-existing singleton makes enable() return early.
32+
"""
33+
for key in list(os.environ):
34+
if key.startswith(("DD_", "_CI_DD_")):
35+
monkeypatch.delenv(key, raising=False)
36+
37+
suspended = CIVisibility._suspend()
38+
try:
39+
yield
40+
finally:
41+
if CIVisibility.enabled:
42+
CIVisibility.disable()
43+
CIVisibility._resume(suspended)
44+
45+
2146
@pytest.fixture(autouse=True, scope="module")
2247
def _dummy_check_enabled_features():
2348
"""By default, assume that _check_enabled_features() returns an ITR-disabled response.
@@ -77,8 +102,7 @@ def test_civisibility_intake_with_apikey():
77102
@pytest.mark.subprocess()
78103
def test_civisibility_intake_payloads():
79104
import gzip
80-
81-
import mock
105+
from unittest import mock
82106

83107
from ddtrace.internal.ci_visibility.constants import COVERAGE_TAG_NAME
84108
from ddtrace.internal.ci_visibility.recorder import CIVisibilityWriter

0 commit comments

Comments
 (0)