fix(ci_visibility): add flag to show class in test name [backport 1.20] (#7278)

Backport 70dda63 from #7225 to 1.20.

Provides a workaround to #4550 by adding a
`--ddtrace-include-class-name` option that will, for class-based tests,
add the class name to the test as `ClassName.test_name` .

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: Romain Komorn <[email protected]>
Co-authored-by: Romain Komorn <[email protected]>

Author: github-actions[bot]

ddtrace/contrib/pytest/constants.py

@@ -1,7 +1,9 @@
 FRAMEWORK = "pytest"
 KIND = "test"
 
-HELP_MSG = "Enable tracing of pytest functions."
+DDTRACE_HELP_MSG = "Enable tracing of pytest functions."
+NO_DDTRACE_HELP_MSG = "Disable tracing of pytest functions."
+DDTRACE_INCLUDE_CLASS_HELP_MSG = "Prepend 'ClassName.' to names of class-based tests."
 
 # XFail Reason
 XFAIL_REASON = "pytest.xfail.reason"

ddtrace/contrib/pytest/plugin.py

@@ -22,9 +22,11 @@
 
 import ddtrace
 from ddtrace.constants import SPAN_KIND
+from ddtrace.contrib.pytest.constants import DDTRACE_HELP_MSG
+from ddtrace.contrib.pytest.constants import DDTRACE_INCLUDE_CLASS_HELP_MSG
 from ddtrace.contrib.pytest.constants import FRAMEWORK
-from ddtrace.contrib.pytest.constants import HELP_MSG
 from ddtrace.contrib.pytest.constants import KIND
+from ddtrace.contrib.pytest.constants import NO_DDTRACE_HELP_MSG
 from ddtrace.contrib.pytest.constants import XFAIL_REASON
 from ddtrace.contrib.unittest import unpatch as unpatch_unittest
 from ddtrace.ext import SpanTypes
@@ -229,6 +231,14 @@ def _get_suite_name(item, test_module_path=None):
     return item.nodeid
 
 
+def _get_item_name(item):
+    """Extract name from item, prepending class if desired"""
+    if hasattr(item, "cls") and item.cls:
+        if item.config.getoption("ddtrace-include-class-name") or item.config.getini("ddtrace-include-class-name"):
+            return "%s.%s" % (item.cls.__name__, item.name)
+    return item.name
+
+
 def _is_test_unskippable(item):
     return any(
         [
@@ -348,15 +358,15 @@ def pytest_addoption(parser):
         action="store_true",
         dest="ddtrace",
         default=False,
-        help=HELP_MSG,
+        help=DDTRACE_HELP_MSG,
     )
 
     group._addoption(
         "--no-ddtrace",
         action="store_true",
         dest="no-ddtrace",
         default=False,
-        help=HELP_MSG,
+        help=NO_DDTRACE_HELP_MSG,
     )
 
     group._addoption(
@@ -367,9 +377,18 @@ def pytest_addoption(parser):
         help=PATCH_ALL_HELP_MSG,
     )
 
-    parser.addini("ddtrace", HELP_MSG, type="bool")
-    parser.addini("no-ddtrace", HELP_MSG, type="bool")
+    group._addoption(
+        "--ddtrace-include-class-name",
+        action="store_true",
+        dest="ddtrace-include-class-name",
+        default=False,
+        help=DDTRACE_INCLUDE_CLASS_HELP_MSG,
+    )
+
+    parser.addini("ddtrace", DDTRACE_HELP_MSG, type="bool")
+    parser.addini("no-ddtrace", DDTRACE_HELP_MSG, type="bool")
     parser.addini("ddtrace-patch-all", PATCH_ALL_HELP_MSG, type="bool")
+    parser.addini("ddtrace-include-class-name", DDTRACE_INCLUDE_CLASS_HELP_MSG, type="bool")
 
 
 def pytest_configure(config):
@@ -494,8 +513,10 @@ def pytest_collection_modifyitems(session, config, items):
         for item in items:
             test_is_unskippable = _is_test_unskippable(item)
 
+            item_name = _get_item_name(item)
+
             if test_is_unskippable:
-                log.debug("Test %s in module %s is marked as unskippable", (item.name, item.module))
+                log.debug("Test %s in module %s is marked as unskippable", (item_name, item.module))
                 item._dd_itr_test_unskippable = True
 
             # Due to suite skipping mode, defer adding ITR skip marker until unskippable status of the suite has been
@@ -512,7 +533,7 @@ def pytest_collection_modifyitems(session, config, items):
                         item_to_skip._dd_itr_forced = True
                     items_to_skip_by_module[item.module] = []
 
-            if _CIVisibility._instance._should_skip_path(str(get_fslocation_from_item(item)[0]), item.name):
+            if _CIVisibility._instance._should_skip_path(str(get_fslocation_from_item(item)[0]), item_name):
                 if test_is_unskippable or (
                     _CIVisibility._instance._suite_skipping_mode and current_suite_has_unskippable_test
                 ):
@@ -547,6 +568,8 @@ def pytest_runtest_protocol(item, nextitem):
         )
     )
 
+    item_name = _get_item_name(item)
+
     test_session_span = _extract_span(item.session)
 
     pytest_module_item = _find_pytest_item(item, pytest.Module)
@@ -605,7 +628,7 @@ def pytest_runtest_protocol(item, nextitem):
         span.set_tag_str(SPAN_KIND, KIND)
         span.set_tag_str(test.FRAMEWORK, FRAMEWORK)
         span.set_tag_str(_EVENT_TYPE, SpanTypes.TEST)
-        span.set_tag_str(test.NAME, item.name)
+        span.set_tag_str(test.NAME, item_name)
         span.set_tag_str(test.COMMAND, _get_pytest_command(item.config))
         if test_session_span:
             span.set_tag_str(_SESSION_ID, str(test_session_span.span_id))

docs/spelling_wordlist.txt

@@ -164,6 +164,7 @@ plugin
 posix
 postgres
 pre
+prepend
 prepended
 profiler
 protobuf

releasenotes/notes/ci_visibility-fix-pytest-class-names-option-8675b04e442425a4.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    CI Visibility: fixes an issue where class-based test methods with the same
+    name across classes would be considered duplicates, and cause one (or more)
+    tests to be dropped from results, by adding ``--ddtrace-include-class-name``
+    as an optional flag (defaulting to false) to prepend the class name to the
+    test name.

tests/contrib/pytest/test_pytest.py

@@ -2992,3 +2992,109 @@ def test_inner_wasnot_going_to_skip_skipif():
         assert inner_module_span.get_tag("test.itr.tests_skipping.tests_skipped") == "false"
         assert inner_module_span.get_tag("_dd.ci.itr.tests_skipped") == "false"
         assert inner_module_span.get_tag("test.itr.forced_run") == "true"
+
+    def test_pytest_ddtrace_test_names(self):
+        package_outer_dir = self.testdir.mkpydir("test_package")
+        os.chdir(str(package_outer_dir))
+        with open("test_names.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    def test_ok():
+                        assert True
+
+                    class TestClassOne():
+                        def test_ok(self):
+                            assert True
+
+                    class TestClassTwo():
+                        def test_ok(self):
+                            assert True
+                    """
+                    )
+                )
+            )
+
+        self.testdir.chdir()
+        self.inline_run("--ddtrace")
+
+        spans = self.pop_spans()
+        assert len(spans) == 6
+
+        session_span = [span for span in spans if span.get_tag("type") == "test_session_end"][0]
+        assert session_span.get_tag("test.status") == "pass"
+
+        module_span = [span for span in spans if span.get_tag("type") == "test_module_end"][0]
+        assert module_span.get_tag("test.module") == "test_package"
+
+        suite_span = [span for span in spans if span.get_tag("type") == "test_suite_end"][0]
+        assert suite_span.get_tag("test.module") == "test_package"
+        assert suite_span.get_tag("test.suite") == "test_names.py"
+
+        test_spans = [span for span in spans if span.get_tag("type") == "test"]
+        assert len(test_spans) == 3
+        assert test_spans[0].get_tag("test.module") == "test_package"
+        assert test_spans[0].get_tag("test.suite") == "test_names.py"
+        assert test_spans[0].get_tag("test.name") == "test_ok"
+
+        assert test_spans[1].get_tag("test.module") == "test_package"
+        assert test_spans[1].get_tag("test.suite") == "test_names.py"
+        assert test_spans[1].get_tag("test.name") == "test_ok"
+
+        assert test_spans[2].get_tag("test.module") == "test_package"
+        assert test_spans[2].get_tag("test.suite") == "test_names.py"
+        assert test_spans[2].get_tag("test.name") == "test_ok"
+
+    def test_pytest_ddtrace_test_names_include_class_opt(self):
+        package_outer_dir = self.testdir.mkpydir("test_package")
+        os.chdir(str(package_outer_dir))
+        with open("test_names.py", "w+") as fd:
+            fd.write(
+                textwrap.dedent(
+                    (
+                        """
+                    def test_ok():
+                        assert True
+
+                    class TestClassOne():
+                        def test_ok(self):
+                            assert True
+
+                    class TestClassTwo():
+                        def test_ok(self):
+                            assert True
+                    """
+                    )
+                )
+            )
+
+        self.testdir.chdir()
+        self.inline_run("--ddtrace", "--ddtrace-include-class-name")
+
+        spans = self.pop_spans()
+        assert len(spans) == 6
+
+        session_span = [span for span in spans if span.get_tag("type") == "test_session_end"][0]
+        assert session_span.get_tag("test.status") == "pass"
+
+        module_span = [span for span in spans if span.get_tag("type") == "test_module_end"][0]
+        assert module_span.get_tag("test.module") == "test_package"
+
+        suite_span = [span for span in spans if span.get_tag("type") == "test_suite_end"][0]
+        assert suite_span.get_tag("test.module") == "test_package"
+        assert suite_span.get_tag("test.suite") == "test_names.py"
+
+        test_spans = [span for span in spans if span.get_tag("type") == "test"]
+        assert len(test_spans) == 3
+        assert test_spans[0].get_tag("test.module") == "test_package"
+        assert test_spans[0].get_tag("test.suite") == "test_names.py"
+        assert test_spans[0].get_tag("test.name") == "test_ok"
+
+        assert test_spans[1].get_tag("test.module") == "test_package"
+        assert test_spans[1].get_tag("test.suite") == "test_names.py"
+        assert test_spans[1].get_tag("test.name") == "TestClassOne.test_ok"
+
+        assert test_spans[2].get_tag("test.module") == "test_package"
+        assert test_spans[2].get_tag("test.suite") == "test_names.py"
+        assert test_spans[2].get_tag("test.name") == "TestClassTwo.test_ok"