From 173a261d021ee2b608d0a4aff686a7eb1af972b6 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 11:12:28 +0100 Subject: [PATCH 01/13] add safety check for Windows paths --- ddtrace/contrib/pytest/plugin.py | 17 ++++++++++++----- ddtrace/contrib/unittest/patch.py | 23 ++++++++++++++++++----- ddtrace/internal/ci_visibility/utils.py | 8 +++++--- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index a569c95d8ac..a4ae46e6107 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -11,14 +11,14 @@ expected failures. """ -from doctest import DocTest import json import os import re +from doctest import DocTest from typing import Dict -from _pytest.nodes import get_fslocation_from_item import pytest +from _pytest.nodes import get_fslocation_from_item import ddtrace from ddtrace.constants import SPAN_KIND @@ -50,7 +50,6 @@ from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger - PATCH_ALL_HELP_MSG = "Call ddtrace.patch_all before running tests." log = get_logger(__name__) @@ -331,7 +330,9 @@ def _start_test_suite_span(item, test_module_span, should_enable_coverage=False) test_suite_span.set_tag_str(test.MODULE, test_module_span.get_tag(test.MODULE)) test_module_path = test_module_span.get_tag(test.MODULE_PATH) test_suite_span.set_tag_str(test.MODULE_PATH, test_module_path) - test_suite_span.set_tag_str(test.SUITE, item.config.hook.pytest_ddtrace_get_item_suite_name(item=item)) + test_suite_name = item.config.hook.pytest_ddtrace_get_item_suite_name(item=item) + if test_suite_name: + test_suite_span.set_tag_str(test.SUITE, test_suite_name) _store_span(pytest_module_item, test_suite_span) if should_enable_coverage: @@ -806,7 +807,13 @@ def pytest_ddtrace_get_item_suite_name(item): if test_module_path: if not pytest_module_item.nodeid.startswith(test_module_path): log.warning("Suite path is not under module path: '%s' '%s'", pytest_module_item.nodeid, test_module_path) - suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) + try: + suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) + except TypeError: + log.warning( + "Tried to collect suite path but it is using different paths on Windows", + ) + return "" return suite_path return pytest_module_item.nodeid diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 374b020d997..185c30e1a22 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -258,7 +258,11 @@ def _extract_test_file_name(item) -> str: def _extract_module_file_path(item) -> str: if _is_test(item): - return os.path.relpath(inspect.getfile(item.__class__)) + try: + module_file_path = os.path.relpath(inspect.getfile(item.__class__)) + return module_file_path + except ValueError: + log.debug("Tried to collect module file path but it is on different paths on Windows") return "" @@ -537,14 +541,23 @@ def add_xpass_test_wrapper(func, instance, args: tuple, kwargs: dict): def _mark_test_as_unskippable(obj): test_name = obj.__name__ test_suite_name = str(obj).split(".")[0].split()[1] - test_module_path = os.path.relpath(obj.__code__.co_filename) - test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) - _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) + test_module_path = None + try: + test_module_path = os.path.relpath(obj.__code__.co_filename) + except ValueError: + log.debug("Tried to collect unskippable decorator but it is on different paths on Windows") + if test_module_path: + test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) + _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) return obj +def _using_unskippable_decorator(args, kwargs): + return args[0] is False and _extract_skip_if_reason(args, kwargs) == ITR_UNSKIPPABLE_REASON + + def skip_if_decorator(func, instance, args: tuple, kwargs: dict): - if args[0] is False and _extract_skip_if_reason(args, kwargs) == ITR_UNSKIPPABLE_REASON: + if _using_unskippable_decorator(args, kwargs): return _mark_test_as_unskippable return func(*args, **kwargs) diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index 8c1f658ff69..d681cb8a069 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -6,14 +6,13 @@ from ddtrace.ext import test from ddtrace.internal.logger import get_logger - log = get_logger(__name__) def get_source_file_path_for_test_method(test_method_object) -> typing.Union[str, None]: try: source_file_path = os.path.relpath(inspect.getfile(test_method_object)) - except TypeError: + except (TypeError, ValueError): return None return source_file_path @@ -39,7 +38,10 @@ def _add_start_end_source_file_path_data_to_span(span: ddtrace.Span, test_method return source_file_path = get_source_file_path_for_test_method(test_method_object) if not source_file_path: - log.debug("Tried to collect file path for test %s but it is a built-in Python function", test_name) + log.debug( + "Tried to collect file path for test %s but it is a built-in Python function or using different paths on Windows", + test_name, + ) return start_line, end_line = get_source_lines_for_test_method(test_method_object) if not start_line or not end_line: From a769fb106673271f0dfb127cc49ee550c787acc2 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 11:33:00 +0100 Subject: [PATCH 02/13] add release note --- ddtrace/contrib/unittest/patch.py | 4 ++-- .../notes/fix-path-issue-windows-d17b5d3ae2294834.yaml | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-path-issue-windows-d17b5d3ae2294834.yaml diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 185c30e1a22..35b2e18cd8e 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -262,7 +262,7 @@ def _extract_module_file_path(item) -> str: module_file_path = os.path.relpath(inspect.getfile(item.__class__)) return module_file_path except ValueError: - log.debug("Tried to collect module file path but it is on different paths on Windows") + log.debug("Tried to collect module file path but it is using different paths on Windows") return "" @@ -545,7 +545,7 @@ def _mark_test_as_unskippable(obj): try: test_module_path = os.path.relpath(obj.__code__.co_filename) except ValueError: - log.debug("Tried to collect unskippable decorator but it is on different paths on Windows") + log.debug("Tried to collect unskippable decorator but it is using different paths on Windows") if test_module_path: test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) diff --git a/releasenotes/notes/fix-path-issue-windows-d17b5d3ae2294834.yaml b/releasenotes/notes/fix-path-issue-windows-d17b5d3ae2294834.yaml new file mode 100644 index 00000000000..1b32fe970e1 --- /dev/null +++ b/releasenotes/notes/fix-path-issue-windows-d17b5d3ae2294834.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + CI Visibility: Fixes an issue where a ``ValueError`` was raised when using different path drives on Windows From dafd19351cd54a2b04bf312fb1a2dea2517ecbcd Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 14:35:01 +0100 Subject: [PATCH 03/13] fmt ruff --- ddtrace/contrib/pytest/plugin.py | 13 +++++---- ddtrace/contrib/unittest/patch.py | 29 +++++++++++++------ ddtrace/internal/ci_visibility/utils.py | 27 ++++++++++++----- ddtrace/internal/wrapping/__init__.py | 2 +- tests/appsec/iast/aspects/test_add_aspect.py | 2 +- .../taint_sinks/weak_randomness_random.py | 2 +- tests/debugging/exploration/debugger.py | 2 +- 7 files changed, 51 insertions(+), 26 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index a4ae46e6107..45be32bfb92 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -11,14 +11,14 @@ expected failures. """ +from doctest import DocTest import json import os import re -from doctest import DocTest from typing import Dict -import pytest from _pytest.nodes import get_fslocation_from_item +import pytest import ddtrace from ddtrace.constants import SPAN_KIND @@ -50,6 +50,7 @@ from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger + PATCH_ALL_HELP_MSG = "Call ddtrace.patch_all before running tests." log = get_logger(__name__) @@ -643,7 +644,7 @@ def pytest_runtest_protocol(item, nextitem): _CIVisibility.set_codeowners_of(item.location[0], span=span) if hasattr(item, "_obj"): test_method_object = item._obj - _add_start_end_source_file_path_data_to_span(span, test_method_object, test_name) + _add_start_end_source_file_path_data_to_span(span, test_method_object, test_name, item.config.rootdir) # We preemptively set FAIL as a status, because if pytest_runtest_makereport is not called # (where the actual test status is set), it means there was a pytest error @@ -810,10 +811,10 @@ def pytest_ddtrace_get_item_suite_name(item): try: suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) except TypeError: - log.warning( - "Tried to collect suite path but it is using different paths on Windows", + log.debug( + "Tried to collect suite path but it is using different drive paths on Windows, using absolute path instead", ) - return "" + return os.path.abspath(pytest_module_item.nodeid) return suite_path return pytest_module_item.nodeid diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 35b2e18cd8e..5eb8ae8c68b 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -259,10 +259,20 @@ def _extract_test_file_name(item) -> str: def _extract_module_file_path(item) -> str: if _is_test(item): try: - module_file_path = os.path.relpath(inspect.getfile(item.__class__)) + test_module_object = inspect.getfile(item.__class__) + except TypeError: + log.debug( + "Tried to collect module file path but it is a built-in Python function", + ) + return "" + try: + module_file_path = os.path.relpath(test_module_object, start=os.getcwd()) return module_file_path except ValueError: - log.debug("Tried to collect module file path but it is using different paths on Windows") + log.debug( + "Tried to collect module file path but it is using different drive paths on Windows, using absolute path instead" + ) + return os.path.abspath(test_module_object) return "" @@ -541,14 +551,15 @@ def add_xpass_test_wrapper(func, instance, args: tuple, kwargs: dict): def _mark_test_as_unskippable(obj): test_name = obj.__name__ test_suite_name = str(obj).split(".")[0].split()[1] - test_module_path = None try: - test_module_path = os.path.relpath(obj.__code__.co_filename) + test_module_path = os.path.relpath(obj.__code__.co_filename, start=os.getcwd()) except ValueError: - log.debug("Tried to collect unskippable decorator but it is using different paths on Windows") - if test_module_path: - test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) - _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) + log.debug( + "Tried to collect unskippable decorator but it is using different drive paths on Windows, using absolute path instead" + ) + test_module_path = os.path.abspath(obj.__code__.co_filename) + test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) + _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) return obj @@ -802,7 +813,7 @@ def _start_test_span(instance, test_suite_span: ddtrace.Span) -> ddtrace.Span: _CIVisibility.set_codeowners_of(_extract_test_file_name(instance), span=span) - _add_start_end_source_file_path_data_to_span(span, test_method_object, test_name) + _add_start_end_source_file_path_data_to_span(span, test_method_object, test_name, os.getcwd()) _store_test_span(instance, span) return span diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index d681cb8a069..cb1843b1d81 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -6,14 +6,22 @@ from ddtrace.ext import test from ddtrace.internal.logger import get_logger + log = get_logger(__name__) -def get_source_file_path_for_test_method(test_method_object) -> typing.Union[str, None]: +def get_source_file_path_for_test_method(test_method_object, repo_directory: str) -> typing.Union[str, None]: + try: + file_object = inspect.getfile(test_method_object) + except TypeError: + return "" try: - source_file_path = os.path.relpath(inspect.getfile(test_method_object)) - except (TypeError, ValueError): - return None + source_file_path = os.path.relpath(file_object, start=repo_directory) + except ValueError: + log.debug( + "Tried to collect source file path but it is using different drive paths on Windows, using absolute path instead", + ) + return os.path.abspath(file_object) return source_file_path @@ -29,17 +37,22 @@ def get_source_lines_for_test_method( return start_line, end_line -def _add_start_end_source_file_path_data_to_span(span: ddtrace.Span, test_method_object, test_name: str): +def _add_start_end_source_file_path_data_to_span( + span: ddtrace.Span, test_method_object, test_name: str, repo_directory: str +): if not test_method_object: log.debug( "Tried to collect source start/end lines for test method %s but test method object could not be found", test_name, ) return - source_file_path = get_source_file_path_for_test_method(test_method_object) + import pdb + + pdb.set_trace() + source_file_path = get_source_file_path_for_test_method(test_method_object, repo_directory) if not source_file_path: log.debug( - "Tried to collect file path for test %s but it is a built-in Python function or using different paths on Windows", + "Tried to collect file path for test %s but it is a built-in Python function", test_name, ) return diff --git a/ddtrace/internal/wrapping/__init__.py b/ddtrace/internal/wrapping/__init__.py index 4c501f03db3..48c4aa5743d 100644 --- a/ddtrace/internal/wrapping/__init__.py +++ b/ddtrace/internal/wrapping/__init__.py @@ -14,8 +14,8 @@ except ImportError: from typing_extensions import Protocol # type: ignore[assignment] -import bytecode as bc from bytecode import Instr +import bytecode as bc from ddtrace.internal.assembly import Assembly from ddtrace.internal.compat import PYTHON_VERSION_INFO as PY diff --git a/tests/appsec/iast/aspects/test_add_aspect.py b/tests/appsec/iast/aspects/test_add_aspect.py index 2c3b173bbd8..479874344c6 100644 --- a/tests/appsec/iast/aspects/test_add_aspect.py +++ b/tests/appsec/iast/aspects/test_add_aspect.py @@ -13,8 +13,8 @@ from ddtrace.appsec._iast._taint_tracking import taint_pyobject from ddtrace.appsec._iast._taint_tracking import taint_ranges_as_evidence_info from ddtrace.appsec._iast._taint_tracking._native.taint_tracking import TaintRange_ - import ddtrace.appsec._iast._taint_tracking.aspects as ddtrace_aspects from ddtrace.appsec._iast._taint_tracking.aspects import add_aspect + import ddtrace.appsec._iast._taint_tracking.aspects as ddtrace_aspects from ddtrace.appsec._iast._utils import _is_python_version_supported as python_supported_by_iast except (ImportError, AttributeError): pytest.skip("IAST not supported for this Python version", allow_module_level=True) diff --git a/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py b/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py index b8c180870da..548adf62a91 100644 --- a/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py +++ b/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py @@ -2,7 +2,6 @@ CAVEAT: the line number is important to some IAST tests, be careful to modify this file and update the tests if you make some changes """ -import random as random_module from random import betavariate from random import choice from random import choices @@ -22,6 +21,7 @@ from random import uniform from random import vonmisesvariate from random import weibullvariate +import random as random_module def random_random(): diff --git a/tests/debugging/exploration/debugger.py b/tests/debugging/exploration/debugger.py index 229ac4e64d1..c4a0edaccdd 100644 --- a/tests/debugging/exploration/debugger.py +++ b/tests/debugging/exploration/debugger.py @@ -8,9 +8,9 @@ from output import log from ddtrace.debugging._config import di_config -import ddtrace.debugging._debugger as _debugger from ddtrace.debugging._debugger import Debugger from ddtrace.debugging._debugger import DebuggerModuleWatchdog +import ddtrace.debugging._debugger as _debugger from ddtrace.debugging._encoding import LogSignalJsonEncoder from ddtrace.debugging._function.discovery import FunctionDiscovery from ddtrace.debugging._probe.model import Probe From 27f8cca924d7dba2edd05d73262b6a199b3769a5 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 14:40:13 +0100 Subject: [PATCH 04/13] fmt ruff --- ddtrace/internal/wrapping/__init__.py | 2 +- tests/appsec/iast/aspects/test_add_aspect.py | 2 +- .../appsec/iast/fixtures/taint_sinks/weak_randomness_random.py | 2 +- tests/debugging/exploration/debugger.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ddtrace/internal/wrapping/__init__.py b/ddtrace/internal/wrapping/__init__.py index 48c4aa5743d..4c501f03db3 100644 --- a/ddtrace/internal/wrapping/__init__.py +++ b/ddtrace/internal/wrapping/__init__.py @@ -14,8 +14,8 @@ except ImportError: from typing_extensions import Protocol # type: ignore[assignment] -from bytecode import Instr import bytecode as bc +from bytecode import Instr from ddtrace.internal.assembly import Assembly from ddtrace.internal.compat import PYTHON_VERSION_INFO as PY diff --git a/tests/appsec/iast/aspects/test_add_aspect.py b/tests/appsec/iast/aspects/test_add_aspect.py index 479874344c6..2c3b173bbd8 100644 --- a/tests/appsec/iast/aspects/test_add_aspect.py +++ b/tests/appsec/iast/aspects/test_add_aspect.py @@ -13,8 +13,8 @@ from ddtrace.appsec._iast._taint_tracking import taint_pyobject from ddtrace.appsec._iast._taint_tracking import taint_ranges_as_evidence_info from ddtrace.appsec._iast._taint_tracking._native.taint_tracking import TaintRange_ - from ddtrace.appsec._iast._taint_tracking.aspects import add_aspect import ddtrace.appsec._iast._taint_tracking.aspects as ddtrace_aspects + from ddtrace.appsec._iast._taint_tracking.aspects import add_aspect from ddtrace.appsec._iast._utils import _is_python_version_supported as python_supported_by_iast except (ImportError, AttributeError): pytest.skip("IAST not supported for this Python version", allow_module_level=True) diff --git a/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py b/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py index 548adf62a91..b8c180870da 100644 --- a/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py +++ b/tests/appsec/iast/fixtures/taint_sinks/weak_randomness_random.py @@ -2,6 +2,7 @@ CAVEAT: the line number is important to some IAST tests, be careful to modify this file and update the tests if you make some changes """ +import random as random_module from random import betavariate from random import choice from random import choices @@ -21,7 +22,6 @@ from random import uniform from random import vonmisesvariate from random import weibullvariate -import random as random_module def random_random(): diff --git a/tests/debugging/exploration/debugger.py b/tests/debugging/exploration/debugger.py index c4a0edaccdd..229ac4e64d1 100644 --- a/tests/debugging/exploration/debugger.py +++ b/tests/debugging/exploration/debugger.py @@ -8,9 +8,9 @@ from output import log from ddtrace.debugging._config import di_config +import ddtrace.debugging._debugger as _debugger from ddtrace.debugging._debugger import Debugger from ddtrace.debugging._debugger import DebuggerModuleWatchdog -import ddtrace.debugging._debugger as _debugger from ddtrace.debugging._encoding import LogSignalJsonEncoder from ddtrace.debugging._function.discovery import FunctionDiscovery from ddtrace.debugging._probe.model import Probe From 7cdbb379a84304f1263d16114fa948ed17aa5f2a Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 14:41:45 +0100 Subject: [PATCH 05/13] fmt ruff --- ddtrace/contrib/pytest/plugin.py | 3 ++- ddtrace/contrib/unittest/patch.py | 6 ++++-- ddtrace/internal/ci_visibility/utils.py | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index 45be32bfb92..bdc98767e96 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -812,7 +812,8 @@ def pytest_ddtrace_get_item_suite_name(item): suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) except TypeError: log.debug( - "Tried to collect suite path but it is using different drive paths on Windows, using absolute path instead", + "Tried to collect suite path but it is using different drive paths on Windows, " + "using absolute path instead", ) return os.path.abspath(pytest_module_item.nodeid) return suite_path diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 5eb8ae8c68b..46bc0218853 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -270,7 +270,8 @@ def _extract_module_file_path(item) -> str: return module_file_path except ValueError: log.debug( - "Tried to collect module file path but it is using different drive paths on Windows, using absolute path instead" + "Tried to collect module file path but it is using different drive paths on Windows, " + "using absolute path instead" ) return os.path.abspath(test_module_object) @@ -555,7 +556,8 @@ def _mark_test_as_unskippable(obj): test_module_path = os.path.relpath(obj.__code__.co_filename, start=os.getcwd()) except ValueError: log.debug( - "Tried to collect unskippable decorator but it is using different drive paths on Windows, using absolute path instead" + "Tried to collect unskippable decorator but it is using different drive paths on Windows, " + "using absolute path instead" ) test_module_path = os.path.abspath(obj.__code__.co_filename) test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index cb1843b1d81..25e40638fdf 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -19,7 +19,8 @@ def get_source_file_path_for_test_method(test_method_object, repo_directory: str source_file_path = os.path.relpath(file_object, start=repo_directory) except ValueError: log.debug( - "Tried to collect source file path but it is using different drive paths on Windows, using absolute path instead", + "Tried to collect source file path but it is using different drive paths on Windows, " + "using absolute path instead", ) return os.path.abspath(file_object) return source_file_path From 9f06535270648c222d7b336f8b3e9c23c123e896 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 14:46:31 +0100 Subject: [PATCH 06/13] typo plugin.py --- ddtrace/contrib/pytest/plugin.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index bdc98767e96..be429496391 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -331,9 +331,7 @@ def _start_test_suite_span(item, test_module_span, should_enable_coverage=False) test_suite_span.set_tag_str(test.MODULE, test_module_span.get_tag(test.MODULE)) test_module_path = test_module_span.get_tag(test.MODULE_PATH) test_suite_span.set_tag_str(test.MODULE_PATH, test_module_path) - test_suite_name = item.config.hook.pytest_ddtrace_get_item_suite_name(item=item) - if test_suite_name: - test_suite_span.set_tag_str(test.SUITE, test_suite_name) + test_suite_span.set_tag_str(test.SUITE, item.config.hook.pytest_ddtrace_get_item_suite_name(item=item)) _store_span(pytest_module_item, test_suite_span) if should_enable_coverage: From ee9d9ae1a8343b805e7cc64000da5b226b115666 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Thu, 16 Nov 2023 15:12:50 +0100 Subject: [PATCH 07/13] fix typo utils.py --- ddtrace/internal/ci_visibility/utils.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index 25e40638fdf..37defb4d2e9 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -47,9 +47,6 @@ def _add_start_end_source_file_path_data_to_span( test_name, ) return - import pdb - - pdb.set_trace() source_file_path = get_source_file_path_for_test_method(test_method_object, repo_directory) if not source_file_path: log.debug( From c6f5b6384143b2dedc93f676ab429510e9148f91 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Fri, 17 Nov 2023 12:38:34 +0100 Subject: [PATCH 08/13] add force test cov env var --- ddtrace/internal/ci_visibility/recorder.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 65b25bd311e..d46cdf39d39 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -169,6 +169,8 @@ def __init__(self, tracer=None, config=None, service=None): @staticmethod def _should_collect_coverage(coverage_enabled_by_api): + if asbool(os.getenv("_DD_CIVISIBILITY_ITR_FORCE_ENABLE_COVERAGE", default=False)): + return True if not coverage_enabled_by_api: return False if compat.PY2: From 54419ef6a3523776d803d9f4cba2334dcceb9fbb Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Fri, 17 Nov 2023 12:39:44 +0100 Subject: [PATCH 09/13] Revert "add force test cov env var" This reverts commit c6f5b6384143b2dedc93f676ab429510e9148f91. --- ddtrace/internal/ci_visibility/recorder.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index d46cdf39d39..65b25bd311e 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -169,8 +169,6 @@ def __init__(self, tracer=None, config=None, service=None): @staticmethod def _should_collect_coverage(coverage_enabled_by_api): - if asbool(os.getenv("_DD_CIVISIBILITY_ITR_FORCE_ENABLE_COVERAGE", default=False)): - return True if not coverage_enabled_by_api: return False if compat.PY2: From d1cbd269eba36b109b8795f49116eb2b16efa29a Mon Sep 17 00:00:00 2001 From: Eric Navarro Date: Fri, 17 Nov 2023 19:05:29 +0100 Subject: [PATCH 10/13] Update ddtrace/contrib/pytest/plugin.py Co-authored-by: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com> --- ddtrace/contrib/pytest/plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index be429496391..15d92e2e14f 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -808,7 +808,7 @@ def pytest_ddtrace_get_item_suite_name(item): log.warning("Suite path is not under module path: '%s' '%s'", pytest_module_item.nodeid, test_module_path) try: suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) - except TypeError: + except ValueError: log.debug( "Tried to collect suite path but it is using different drive paths on Windows, " "using absolute path instead", From 02b651b17ff2c52bd2c83c4cb8588d3ee00e2a48 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Mon, 20 Nov 2023 11:03:02 +0100 Subject: [PATCH 11/13] refactor --- ddtrace/contrib/pytest/plugin.py | 15 +++++---------- ddtrace/contrib/unittest/patch.py | 24 ++++++------------------ ddtrace/internal/ci_visibility/utils.py | 23 ++++++++++++++--------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index 15d92e2e14f..5d15b4b3d4f 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -46,7 +46,10 @@ from ddtrace.internal.ci_visibility.constants import TEST from ddtrace.internal.ci_visibility.coverage import _initialize_coverage from ddtrace.internal.ci_visibility.coverage import build_payload as build_coverage_payload -from ddtrace.internal.ci_visibility.utils import _add_start_end_source_file_path_data_to_span +from ddtrace.internal.ci_visibility.utils import ( + _add_start_end_source_file_path_data_to_span, + get_relative_or_absolute_path_for_path, +) from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger @@ -806,15 +809,7 @@ def pytest_ddtrace_get_item_suite_name(item): if test_module_path: if not pytest_module_item.nodeid.startswith(test_module_path): log.warning("Suite path is not under module path: '%s' '%s'", pytest_module_item.nodeid, test_module_path) - try: - suite_path = os.path.relpath(pytest_module_item.nodeid, start=test_module_path) - except ValueError: - log.debug( - "Tried to collect suite path but it is using different drive paths on Windows, " - "using absolute path instead", - ) - return os.path.abspath(pytest_module_item.nodeid) - return suite_path + return get_relative_or_absolute_path_for_path(pytest_module_item.nodeid, test_module_path) return pytest_module_item.nodeid diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index 46bc0218853..b05c1157e9d 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -30,7 +30,10 @@ from ddtrace.internal.ci_visibility.constants import TEST from ddtrace.internal.ci_visibility.coverage import _initialize_coverage from ddtrace.internal.ci_visibility.coverage import build_payload as build_coverage_payload -from ddtrace.internal.ci_visibility.utils import _add_start_end_source_file_path_data_to_span +from ddtrace.internal.ci_visibility.utils import ( + _add_start_end_source_file_path_data_to_span, + get_relative_or_absolute_path_for_path, +) from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.utils.formats import asbool @@ -265,15 +268,7 @@ def _extract_module_file_path(item) -> str: "Tried to collect module file path but it is a built-in Python function", ) return "" - try: - module_file_path = os.path.relpath(test_module_object, start=os.getcwd()) - return module_file_path - except ValueError: - log.debug( - "Tried to collect module file path but it is using different drive paths on Windows, " - "using absolute path instead" - ) - return os.path.abspath(test_module_object) + return get_relative_or_absolute_path_for_path(test_module_object, os.getcwd()) return "" @@ -552,14 +547,7 @@ def add_xpass_test_wrapper(func, instance, args: tuple, kwargs: dict): def _mark_test_as_unskippable(obj): test_name = obj.__name__ test_suite_name = str(obj).split(".")[0].split()[1] - try: - test_module_path = os.path.relpath(obj.__code__.co_filename, start=os.getcwd()) - except ValueError: - log.debug( - "Tried to collect unskippable decorator but it is using different drive paths on Windows, " - "using absolute path instead" - ) - test_module_path = os.path.abspath(obj.__code__.co_filename) + test_module_path = get_relative_or_absolute_path_for_path(obj.__code__.co_filename, os.getcwd()) test_module_suite_name = _generate_module_suite_test_path(test_module_path, test_suite_name, test_name) _CIVisibility._unittest_data["unskippable_tests"].add(test_module_suite_name) return obj diff --git a/ddtrace/internal/ci_visibility/utils.py b/ddtrace/internal/ci_visibility/utils.py index 37defb4d2e9..5ff18bdb999 100644 --- a/ddtrace/internal/ci_visibility/utils.py +++ b/ddtrace/internal/ci_visibility/utils.py @@ -10,20 +10,25 @@ log = get_logger(__name__) -def get_source_file_path_for_test_method(test_method_object, repo_directory: str) -> typing.Union[str, None]: +def get_relative_or_absolute_path_for_path(path: str, start_directory: str): try: - file_object = inspect.getfile(test_method_object) - except TypeError: - return "" - try: - source_file_path = os.path.relpath(file_object, start=repo_directory) + relative_path = os.path.relpath(path, start=start_directory) except ValueError: log.debug( - "Tried to collect source file path but it is using different drive paths on Windows, " + "Tried to collect relative path but it is using different drive paths on Windows, " "using absolute path instead", ) - return os.path.abspath(file_object) - return source_file_path + return os.path.abspath(path) + return relative_path + + +def get_source_file_path_for_test_method(test_method_object, repo_directory: str) -> typing.Union[str, None]: + try: + file_object = inspect.getfile(test_method_object) + except TypeError: + return "" + + return get_relative_or_absolute_path_for_path(file_object, repo_directory) def get_source_lines_for_test_method( From 0d3339b0bb211668b37608bf6d52d8e1f64825d8 Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Mon, 20 Nov 2023 11:07:04 +0100 Subject: [PATCH 12/13] fmt ruff --- ddtrace/contrib/pytest/plugin.py | 6 ++---- ddtrace/contrib/unittest/patch.py | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/ddtrace/contrib/pytest/plugin.py b/ddtrace/contrib/pytest/plugin.py index 5d15b4b3d4f..921339d2954 100644 --- a/ddtrace/contrib/pytest/plugin.py +++ b/ddtrace/contrib/pytest/plugin.py @@ -46,10 +46,8 @@ from ddtrace.internal.ci_visibility.constants import TEST from ddtrace.internal.ci_visibility.coverage import _initialize_coverage from ddtrace.internal.ci_visibility.coverage import build_payload as build_coverage_payload -from ddtrace.internal.ci_visibility.utils import ( - _add_start_end_source_file_path_data_to_span, - get_relative_or_absolute_path_for_path, -) +from ddtrace.internal.ci_visibility.utils import _add_start_end_source_file_path_data_to_span +from ddtrace.internal.ci_visibility.utils import get_relative_or_absolute_path_for_path from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger diff --git a/ddtrace/contrib/unittest/patch.py b/ddtrace/contrib/unittest/patch.py index b05c1157e9d..23f58ff3c16 100644 --- a/ddtrace/contrib/unittest/patch.py +++ b/ddtrace/contrib/unittest/patch.py @@ -30,10 +30,8 @@ from ddtrace.internal.ci_visibility.constants import TEST from ddtrace.internal.ci_visibility.coverage import _initialize_coverage from ddtrace.internal.ci_visibility.coverage import build_payload as build_coverage_payload -from ddtrace.internal.ci_visibility.utils import ( - _add_start_end_source_file_path_data_to_span, - get_relative_or_absolute_path_for_path, -) +from ddtrace.internal.ci_visibility.utils import _add_start_end_source_file_path_data_to_span +from ddtrace.internal.ci_visibility.utils import get_relative_or_absolute_path_for_path from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.utils.formats import asbool From ce0f4dd4b9e5cded67544988c00e673a50b85e6c Mon Sep 17 00:00:00 2001 From: "eric.navarro" Date: Mon, 20 Nov 2023 12:48:06 +0100 Subject: [PATCH 13/13] add test --- tests/ci_visibility/test_utils.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 tests/ci_visibility/test_utils.py diff --git a/tests/ci_visibility/test_utils.py b/tests/ci_visibility/test_utils.py new file mode 100644 index 00000000000..b28656d3632 --- /dev/null +++ b/tests/ci_visibility/test_utils.py @@ -0,0 +1,17 @@ +import os.path +from unittest import mock + +from ddtrace.internal.ci_visibility.utils import get_relative_or_absolute_path_for_path + + +def test_gets_relative_path(): + actual_output = get_relative_or_absolute_path_for_path("ddtrace/contrib", start_directory=os.getcwd()) + + assert not os.path.isabs(actual_output) + + +def test_gets_absolute_path_with_exception(): + with mock.patch("os.path.relpath", side_effect=ValueError()): + actual_output = get_relative_or_absolute_path_for_path("ddtrace/contrib", start_directory=os.getcwd()) + + assert os.path.isabs(actual_output)