From 1b095284a4ae262aaa1846c2d5bdb91e6db8471a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20De=20Ara=C3=BAjo?= Date: Mon, 3 Feb 2025 13:47:13 +0000 Subject: [PATCH] feat(ci_visibility): make pytest v2 plugin the default version (#12066) Previously, the pytest v2 plugin was disabled by default, and could be enabled by setting the `DD_PYTEST_USE_NEW_PLUGIN_BETA` variable to true. In ddtrace 3.0, the pytest v2 plugin will be *enabled* by default, and `DD_PYTEST_USE_NEW_PLUGIN_BETA` will not be used anymore. For now I'm not removing the code for the old plugin version. Instead, I'm adding an internal environment variable `_DD_PYTEST_USE_LEGACY_PLUGIN` that allows running pytest with the old plugin, and using that in the tests that depend on it. We can remove the code (and the variable) after the 3.x release, since this is not part of the public interface. ## 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) --- .gitlab/tests.yml | 1 - ddtrace/contrib/internal/pytest/_plugin_v1.py | 5 ++-- ddtrace/contrib/internal/pytest/_plugin_v2.py | 18 +++++++------- ddtrace/contrib/internal/pytest/_utils.py | 2 +- docs/configuration.rst | 16 ------------- hatch.toml | 8 ++----- ...at-pytest-v2-default-db160906c0ba26dc.yaml | 19 +++++++++++++++ riotfile.py | 24 ++++++++----------- ...st_appsec_flask_pytest_iast_no_snapshot.py | 6 ++--- tests/contrib/pytest/test_pytest_snapshot.py | 3 +++ .../contrib/pytest/test_pytest_snapshot_v2.py | 3 --- .../contrib/selenium/test_selenium_chrome.py | 6 ++--- 12 files changed, 52 insertions(+), 59 deletions(-) create mode 100644 releasenotes/notes/ci_visibility-feat-pytest-v2-default-db160906c0ba26dc.yaml diff --git a/.gitlab/tests.yml b/.gitlab/tests.yml index b8c9a3d9897..80c08f543d9 100644 --- a/.gitlab/tests.yml +++ b/.gitlab/tests.yml @@ -6,7 +6,6 @@ stages: variables: RIOT_RUN_CMD: riot -P -v run --exitfirst --pass-env -s REPO_LANG: python # "python" is used everywhere rather than "py" - DD_PYTEST_USE_NEW_PLUGIN_BETA: "true" PYTEST_ADDOPTS: "-s" # CI_DEBUG_SERVICES: "true" diff --git a/ddtrace/contrib/internal/pytest/_plugin_v1.py b/ddtrace/contrib/internal/pytest/_plugin_v1.py index fc4982bdc67..d45a925f773 100644 --- a/ddtrace/contrib/internal/pytest/_plugin_v1.py +++ b/ddtrace/contrib/internal/pytest/_plugin_v1.py @@ -452,9 +452,8 @@ def pytest_load_initial_conftests(early_config, parser, args): def pytest_configure(config): deprecate( - "this version of the pytest ddtrace plugin is slated for deprecation", - message="set DD_PYTEST_USE_NEW_PLUGIN_BETA=true in your environment to preview the next version of the plugin.", - removal_version="3.0.0", + "this version of the pytest ddtrace plugin is deprecated", + message="remove _DD_PYTEST_USE_LEGACY_PLUGIN from your environment to use the currently supported version.", category=DDTraceDeprecationWarning, ) unpatch_unittest() diff --git a/ddtrace/contrib/internal/pytest/_plugin_v2.py b/ddtrace/contrib/internal/pytest/_plugin_v2.py index 0b25af62e51..79435f94576 100644 --- a/ddtrace/contrib/internal/pytest/_plugin_v2.py +++ b/ddtrace/contrib/internal/pytest/_plugin_v2.py @@ -1,3 +1,4 @@ +import os from pathlib import Path import re import typing as t @@ -182,7 +183,6 @@ def pytest_load_initial_conftests(early_config, parser, args): try: take_over_logger_stream_handler() - log.warning("This version of the ddtrace pytest plugin is currently in beta.") # Freezegun is proactively patched to avoid it interfering with internal timing patch(freezegun=True) dd_config.test_visibility.itr_skipping_level = ITR_SKIPPING_LEVEL.SUITE @@ -199,14 +199,14 @@ def pytest_load_initial_conftests(early_config, parser, args): def pytest_configure(config: pytest_Config) -> None: - # The only way we end up in pytest_configure is if the environment variable is being used, and logging the warning - # now ensures it shows up in output regardless of the use of the -s flag - deprecate( - "the DD_PYTEST_USE_NEW_PLUGIN_BETA environment variable is deprecated", - message="this preview version of the pytest ddtrace plugin will become the only version.", - removal_version="3.0.0", - category=DDTraceDeprecationWarning, - ) + if os.getenv("DD_PYTEST_USE_NEW_PLUGIN_BETA"): + # Logging the warning at this point ensures it shows up in output regardless of the use of the -s flag. + deprecate( + "the DD_PYTEST_USE_NEW_PLUGIN_BETA environment variable is deprecated", + message="the new pytest plugin is now the default version. No additional configurations are required.", + removal_version="3.0.0", + category=DDTraceDeprecationWarning, + ) try: if is_enabled(config): diff --git a/ddtrace/contrib/internal/pytest/_utils.py b/ddtrace/contrib/internal/pytest/_utils.py index 7e8b2bc2714..f2af0bd47bd 100644 --- a/ddtrace/contrib/internal/pytest/_utils.py +++ b/ddtrace/contrib/internal/pytest/_utils.py @@ -30,7 +30,7 @@ _NODEID_REGEX = re.compile("^(((?P.*)/)?(?P[^/]*?))::(?P.*?)$") -_USE_PLUGIN_V2 = asbool(os.environ.get("DD_PYTEST_USE_NEW_PLUGIN_BETA", "false")) +_USE_PLUGIN_V2 = not asbool(os.environ.get("_DD_PYTEST_USE_LEGACY_PLUGIN", "false")) class _PYTEST_STATUS: diff --git a/docs/configuration.rst b/docs/configuration.rst index 853050195fe..276656cc1d6 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -616,22 +616,6 @@ Test Visibility version_added: v2.16.0: - DD_PYTEST_USE_NEW_PLUGIN_BETA: - type: Boolean - default: False - - description: | - Configures the ``CIVisibility`` service to use a beta release of the new version of the pytest plugin, - supporting `Auto Test Retries `_, - `Early Flake Detection `_, and - improved coverage collection for `Test Impact Analysis - `_. This version of the plugin will become the default in - the future. See the `release notes for v2.18.0 `_ - for more information. - - version_added: - v2.18.0: - DD_CIVISIBILITY_RUM_FLUSH_WAIT_MILLIS: type: Integer default: 500 diff --git a/hatch.toml b/hatch.toml index d0063aaa751..554db588865 100644 --- a/hatch.toml +++ b/hatch.toml @@ -558,7 +558,6 @@ dependencies = [ ] [envs.pytest_plugin_v2.env-vars] -DD_PYTEST_USE_NEW_PLUGIN_BETA = "true" DD_AGENT_PORT = "9126" [envs.pytest_plugin_v2.scripts] @@ -599,9 +598,6 @@ dependencies = [ "hypothesis", ] -[envs.freezegun.env-vars] -DD_PYTEST_USE_NEW_PLUGIN_BETA = "true" - [envs.freezegun.scripts] test = [ "pytest tests/contrib/freezegun {args:}", @@ -637,6 +633,6 @@ tested_pytest_plugin_version = ["v1", "v2"] [envs.selenium.overrides] matrix.tested_pytest_plugin_version.env-vars = [ - { key = "_TESTED_PYTEST_PLUGIN_VERSION", value = "false", if = ["v1"]}, - { key = "_TESTED_PYTEST_PLUGIN_VERSION", value = "true", if = ["v2"]} + { key = "_TESTED_PYTEST_LEGACY_PLUGIN", value = "true", if = ["v1"]}, + { key = "_TESTED_PYTEST_LEGACY_PLUGIN", value = "false", if = ["v2"]} ] diff --git a/releasenotes/notes/ci_visibility-feat-pytest-v2-default-db160906c0ba26dc.yaml b/releasenotes/notes/ci_visibility-feat-pytest-v2-default-db160906c0ba26dc.yaml new file mode 100644 index 00000000000..f2987f4c4fd --- /dev/null +++ b/releasenotes/notes/ci_visibility-feat-pytest-v2-default-db160906c0ba26dc.yaml @@ -0,0 +1,19 @@ +--- +upgrade: + - | + CI Visibility: Official release of the new version of the pytest plugin, introducing the following features: + - `Auto Test Retries `_ + - `Early Flake Detection `_ + - Improved coverage collection for `Test Impact Analysis `_ + (formerly Intelligent Test Runner), using an internal collection method instead of `coverage.py + `_, with improved dependency discovery. + + **NOTE:** this new version of the plugin introduces breaking changes: + - ``module``, ``suite``, and ``test`` names are now parsed from the ``item.nodeid`` attribute + - test names now include the class for class-based tests + - Test skipping by Test Impact Analysis (formerly Intelligent Test Runner) is now done at the suite level, instead + of at the test level + + A beta version of the plugin had been available since v2.18.0, and could be enabled via the + ``DD_PYTEST_USE_NEW_PLUGIN_BETA`` environment variable. The new version is now the default, and the environment + variable is not used anymore. diff --git a/riotfile.py b/riotfile.py index dac8b01fc0c..23ba3e78465 100644 --- a/riotfile.py +++ b/riotfile.py @@ -103,7 +103,6 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT "DD_INJECT_FORCE": "1", "DD_PATCH_MODULES": "unittest:false", "CMAKE_BUILD_PARALLEL_LEVEL": "12", - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "true", }, venvs=[ Venv( @@ -1604,7 +1603,6 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT }, env={ "DD_AGENT_PORT": "9126", - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", }, venvs=[ Venv( @@ -1634,12 +1632,12 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT venvs=[ Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "0", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "true", }, ), Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "false", }, ), ], @@ -1662,12 +1660,12 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT venvs=[ Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "0", + "DD_PYTEST_LEGACY_PLUGIN": "true", }, ), Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "false", }, ), ], @@ -1734,12 +1732,12 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT venvs=[ Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "0", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "true", }, ), Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "false", }, ), ], @@ -1755,12 +1753,12 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT venvs=[ Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "0", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "true", }, ), Venv( env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "false", }, ), ], @@ -1783,7 +1781,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT ] }, env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "0", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "true", }, ), Venv( @@ -1793,7 +1791,7 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT ] }, env={ - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", + "_DD_PYTEST_USE_LEGACY_PLUGIN": "false", }, ), ], @@ -3072,8 +3070,6 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT env={ "DD_PROFILING_ENABLE_ASSERTS": "1", "DD_PROFILING_EXPORT_LIBDD_ENABLED": "1", - # Enable pytest v2 plugin to handle pytest-cpp items in the test suite - "DD_PYTEST_USE_NEW_PLUGIN_BETA": "1", "CPUCOUNT": "12", }, pkgs={ diff --git a/tests/contrib/flask/test_appsec_flask_pytest_iast_no_snapshot.py b/tests/contrib/flask/test_appsec_flask_pytest_iast_no_snapshot.py index 801cffa4b8a..07f95a55933 100644 --- a/tests/contrib/flask/test_appsec_flask_pytest_iast_no_snapshot.py +++ b/tests/contrib/flask/test_appsec_flask_pytest_iast_no_snapshot.py @@ -9,8 +9,8 @@ @pytest.mark.parametrize("iast_enabled", ["true", "false"]) @pytest.mark.parametrize("iast_request_sampling", ["100.0", "0.0"]) -@pytest.mark.parametrize("pytest_use_new_plugin", ["true", "false"]) -def test_flask_pytest_iast(iast_enabled, iast_request_sampling, pytest_use_new_plugin): +@pytest.mark.parametrize("pytest_use_legacy_plugin", ["false", "true"]) +def test_flask_pytest_iast(iast_enabled, iast_request_sampling, pytest_use_legacy_plugin): from tests.utils import _build_env env = _build_env() @@ -21,7 +21,7 @@ def test_flask_pytest_iast(iast_enabled, iast_request_sampling, pytest_use_new_p "DD_TRACE_SQLITE_ENABLED": "0", "DD_IAST_ENABLED": iast_enabled, "DD_TRACE_DEBUG": "true", - "DD_PYTEST_USE_NEW_PLUGIN_BETA": pytest_use_new_plugin, + "_DD_PYTEST_USE_LEGACY_PLUGIN": pytest_use_legacy_plugin, "DD_IAST_REQUEST_SAMPLING": iast_request_sampling, # "DD_API_KEY": "invalidapikey", # "DD_CIVISIBILITY_AGENTLESS_ENABLED": "1", diff --git a/tests/contrib/pytest/test_pytest_snapshot.py b/tests/contrib/pytest/test_pytest_snapshot.py index 8b298c28c95..17876eea2f0 100644 --- a/tests/contrib/pytest/test_pytest_snapshot.py +++ b/tests/contrib/pytest/test_pytest_snapshot.py @@ -85,6 +85,7 @@ def test_add_two_number_list(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", + _DD_PYTEST_USE_LEGACY_PLUGIN="true", ) ), ) @@ -130,6 +131,7 @@ def test_add_two_number_list(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", + _DD_PYTEST_USE_LEGACY_PLUGIN="true", ) ), ) @@ -164,6 +166,7 @@ def test_call_urllib(): DD_CIVISIBILITY_ITR_ENABLED="false", CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", + _DD_PYTEST_USE_LEGACY_PLUGIN="true", DD_PATCH_MODULES="httpx:true", ) ), diff --git a/tests/contrib/pytest/test_pytest_snapshot_v2.py b/tests/contrib/pytest/test_pytest_snapshot_v2.py index dad546df23b..f7a123129a6 100644 --- a/tests/contrib/pytest/test_pytest_snapshot_v2.py +++ b/tests/contrib/pytest/test_pytest_snapshot_v2.py @@ -85,7 +85,6 @@ def test_add_two_number_list(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", - DD_PYTEST_USE_NEW_PLUGIN_BETA="true", ) ), ) @@ -131,7 +130,6 @@ def test_add_two_number_list(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", - DD_PYTEST_USE_NEW_PLUGIN_BETA="true", ) ), ) @@ -167,7 +165,6 @@ def test_call_urllib(): CI_PROJECT_DIR=str(self.testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", DD_PATCH_MODULES="httpx:true", - DD_PYTEST_USE_NEW_PLUGIN_BETA="true", ) ), ) diff --git a/tests/contrib/selenium/test_selenium_chrome.py b/tests/contrib/selenium/test_selenium_chrome.py index c8f9c9145c8..d820e384113 100644 --- a/tests/contrib/selenium/test_selenium_chrome.py +++ b/tests/contrib/selenium/test_selenium_chrome.py @@ -116,7 +116,7 @@ def test_selenium_local_pass(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", - DD_PYTEST_USE_NEW_PLUGIN_BETA=os.environ.get("_TESTED_PYTEST_PLUGIN_VERSION"), + _DD_PYTEST_USE_LEGACY_PLUGIN=os.environ.get("_TESTED_PYTEST_LEGACY_PLUGIN"), ) ), ) @@ -167,7 +167,7 @@ def test_selenium_local_pass(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", - DD_PYTEST_USE_NEW_PLUGIN_BETA=os.environ.get("_TESTED_PYTEST_PLUGIN_VERSION"), + _DD_PYTEST_USE_LEGACY_PLUGIN=os.environ.get("_TESTED_PYTEST_LEGACY_PLUGIN"), ) ), ) @@ -221,7 +221,7 @@ def test_selenium_local_unpatch(): DD_PATCH_MODULES="sqlite3:false", CI_PROJECT_DIR=str(testdir.tmpdir), DD_CIVISIBILITY_AGENTLESS_ENABLED="false", - DD_PYTEST_USE_NEW_PLUGIN_BETA=os.environ.get("_TESTED_PYTEST_PLUGIN_VERSION"), + _DD_PYTEST_USE_LEGACY_PLUGIN=os.environ.get("_TESTED_PYTEST_LEGACY_PLUGIN"), ) ), )