chore(internal): fix CI Visibility usage of the core API

ddtrace/ext/ci_visibility/api.py

@@ -235,12 +235,6 @@ def finish(
             "ci_visibility.session.finish", (CISession.FinishArgs(item_id, force_finish_children, override_status),)
         )
 
-    @staticmethod
-    @_catch_and_log_exceptions
-    def get_settings(item_id: Optional[CISessionId] = None):
-        log.debug("Getting settings for session %s", item_id)
-        core.dispatch_with_results("ci_visibility.session.get_settings", (item_id,))
-
     @staticmethod
     @_catch_and_log_exceptions
     def get_known_tests(item_id: Optional[CISessionId] = None):
@@ -309,11 +303,13 @@ def mark_itr_forced_run(item_id: Union[CISuiteId, CITestId]):
 
     @staticmethod
     @_catch_and_log_exceptions
-    def is_item_itr_skippable(item_id: Union[CISuiteId, CITestId]):
+    def is_item_itr_skippable(item_id: Union[CISuiteId, CITestId]) -> bool:
         """Skippable items are not currently tied to a test session, so no session ID is passed"""
         log.debug("Getting skippable items")
-        skippable_items = core.dispatch_with_results("ci_visibility.itr.is_item_skippable", (item_id,))
-        return skippable_items["skippable_items"]
+        is_item_skippable: Optional[bool] = core.dispatch_with_results(
+            "ci_visibility.itr.is_item_skippable", (item_id,)
+        ).is_item_skippable.value
+        return bool(is_item_skippable)
 
     class AddCoverageArgs(NamedTuple):
         item_id: Union[_CIVisibilityChildItemIdBase, _CIVisibilityRootItemIdBase]

ddtrace/internal/ci_visibility/recorder.py

@@ -921,18 +921,11 @@ def _on_finish_session(finish_args: CISession.FinishArgs):
     session.finish(finish_args.force_finish_children, finish_args.override_status)
 
 
-@_requires_civisibility_enabled
-def _on_get_session_settings(session_id: CISessionId):
-    log.debug("Handling get session settings for session id %s", session_id)
-    return CIVisibility.get_session_settings(session_id)
-
-
 def _register_session_handlers():
     log.debug("Registering session handlers")
     core.on("ci_visibility.session.discover", _on_discover_session)
     core.on("ci_visibility.session.start", _on_start_session)
     core.on("ci_visibility.session.finish", _on_finish_session)
-    core.on("ci_visibility.session.get_settings", _on_get_session_settings)
 
 
 @_requires_civisibility_enabled
@@ -1140,7 +1133,7 @@ def _on_itr_mark_forced_run(item_id: Union[CISuiteId, CITestId]) -> None:
 @_requires_civisibility_enabled
 def _on_itr_is_item_skippable(item_id: Union[CISuiteId, CITestId]) -> bool:
     """Skippable items are fetched as part CIVisibility.enable(), so they are assumed to be available."""
-    log.debug("Handling get skippable items")
+    log.debug("Handling is item skippable for item id %s", item_id)
 
     if not isinstance(item_id, (CISuiteId, CITestId)):
         log.warning("Only suites or tests can be skippable, not %s", type(item_id))
@@ -1156,7 +1149,7 @@ def _on_itr_is_item_skippable(item_id: Union[CISuiteId, CITestId]) -> bool:
 def _register_itr_handlers():
     log.debug("Registering ITR-related handlers")
     core.on("ci_visibility.itr.finish_skipped_by_itr", _on_itr_finish_item_skipped)
-    core.on("ci_visibility.itr.is_item_skippable", _on_itr_is_item_skippable)
+    core.on("ci_visibility.itr.is_item_skippable", _on_itr_is_item_skippable, "is_item_skippable")
     core.on("ci_visibility.itr.mark_unskippable", _on_itr_mark_unskippable)
     core.on("ci_visibility.itr.mark_forced_run", _on_itr_mark_forced_run)
 

tests/ci_visibility/api/test_ext_ci_visibility_api.py

@@ -3,7 +3,10 @@
 
 import pytest
 
+from ddtrace.ext.ci_visibility import api
 from ddtrace.ext.ci_visibility.api import CISourceFileInfo
+from ddtrace.internal.ci_visibility import CIVisibility
+from tests.ci_visibility.util import set_up_mock_civisibility
 
 
 class TestCISourceFileInfo:
@@ -53,3 +56,126 @@ def test_source_file_info_enforces_start_line_less_than_end_line(self):
         with pytest.raises(ValueError):
             # start_line cannot be None if end_line is provided
             _ = CISourceFileInfo(Path("/absolute/path/my_file_name"), end_line=1)
+
+
+class TestCIITRMixin:
+    """Tests whether or not skippable tests and suites are correctly identified
+
+    Note: these tests do not bother discovering a session as the ITR functionality currently does not rely on sessions.
+    """
+
+    def test_api_is_item_itr_skippable_test_level(self):
+        with set_up_mock_civisibility(
+            itr_enabled=True,
+            skipping_enabled=True,
+            suite_skipping_mode=False,
+            skippable_items={
+                "skippable_module/suite.py": ["skippable_test"],
+            },
+        ):
+            CIVisibility.enable()
+
+            assert CIVisibility.enabled is True
+            assert CIVisibility._instance._suite_skipping_mode is False
+
+            session_id = api.CISessionId("session_id")
+
+            skippable_module_id = api.CIModuleId(session_id, "skippable_module")
+
+            skippable_suite_id = api.CISuiteId(skippable_module_id, "suite.py")
+            skippable_test_id = api.CITestId(skippable_suite_id, "skippable_test")
+            non_skippable_test_id = api.CITestId(skippable_suite_id, "non_skippable_test")
+
+            non_skippable_suite_id = api.CISuiteId(skippable_module_id, "non_skippable_suite.py")
+            non_skippable_suite_skippable_test_id = api.CITestId(non_skippable_suite_id, "skippable_test")
+
+            assert api.CITest.is_item_itr_skippable(skippable_test_id) is True
+            assert api.CITest.is_item_itr_skippable(non_skippable_test_id) is False
+            assert api.CITest.is_item_itr_skippable(non_skippable_suite_skippable_test_id) is False
+
+            CIVisibility.disable()
+
+    def test_api_is_item_itr_skippable_false_when_skipping_disabled_test_level(self):
+        with set_up_mock_civisibility(
+            itr_enabled=True,
+            skipping_enabled=False,
+            suite_skipping_mode=False,
+            skippable_items={
+                "skippable_module/suite.py": ["skippable_test"],
+            },
+        ):
+            CIVisibility.enable()
+
+            assert CIVisibility.enabled is True
+            assert CIVisibility._instance._suite_skipping_mode is False
+
+            session_id = api.CISessionId("session_id")
+
+            skippable_module_id = api.CIModuleId(session_id, "skippable_module")
+
+            skippable_suite_id = api.CISuiteId(skippable_module_id, "suite.py")
+            skippable_test_id = api.CITestId(skippable_suite_id, "skippable_test")
+            non_skippable_test_id = api.CITestId(skippable_suite_id, "non_skippable_test")
+
+            non_skippable_suite_id = api.CISuiteId(skippable_module_id, "non_skippable_suite.py")
+            non_skippable_suite_skippable_test_id = api.CITestId(non_skippable_suite_id, "skippable_test")
+
+            assert api.CITest.is_item_itr_skippable(skippable_test_id) is False
+            assert api.CITest.is_item_itr_skippable(non_skippable_test_id) is False
+            assert api.CITest.is_item_itr_skippable(non_skippable_suite_skippable_test_id) is False
+
+            CIVisibility.disable()
+
+    def test_api_is_item_itr_skippable_suite_level(self):
+        with set_up_mock_civisibility(
+            itr_enabled=True,
+            skipping_enabled=True,
+            suite_skipping_mode=True,
+            skippable_items=["skippable_module/skippable_suite.py"],
+        ):
+            CIVisibility.enable()
+
+            assert CIVisibility.enabled is True
+            assert CIVisibility._instance._suite_skipping_mode is True
+
+            session_id = api.CISessionId("session_id")
+
+            skippable_module_id = api.CIModuleId(session_id, "skippable_module")
+            skippable_suite_id = api.CISuiteId(skippable_module_id, "skippable_suite.py")
+            non_skippable_suite_id = api.CISuiteId(skippable_module_id, "non_skippable_suite.py")
+
+            non_skippable_module_id = api.CIModuleId(session_id, "non_skippable_module")
+            non_skippable_module_skippable_suite_id = api.CISuiteId(non_skippable_module_id, "skippable_suite.py")
+
+            assert api.CISuite.is_item_itr_skippable(skippable_suite_id) is True
+            assert api.CISuite.is_item_itr_skippable(non_skippable_suite_id) is False
+            assert api.CISuite.is_item_itr_skippable(non_skippable_module_skippable_suite_id) is False
+
+            CIVisibility.disable()
+
+    def test_api_is_item_itr_skippable_false_when_skipping_disabled_suite_level(self):
+        with set_up_mock_civisibility(
+            itr_enabled=True,
+            skipping_enabled=False,
+            suite_skipping_mode=True,
+            skippable_items=["skippable_module/skippable_suite.py"],
+        ):
+            CIVisibility.enable()
+
+            assert CIVisibility.enabled is True
+            assert CIVisibility._instance._suite_skipping_mode is True
+
+            session_id = api.CISessionId("session_id")
+
+            skippable_module_id = api.CIModuleId(session_id, "skippable_module")
+            skippable_suite_id = api.CISuiteId(skippable_module_id, "skippable_suite.py")
+            non_skippable_suite_id = api.CISuiteId(skippable_module_id, "non_skippable_suite.py")
+
+            non_skippable_module_id = api.CIModuleId(session_id, "non_skippable_module")
+            non_skippable_module_skippable_suite_id = api.CISuiteId(non_skippable_module_id, "skippable_suite.py")
+
+            assert api.CISuite.is_item_itr_skippable(skippable_suite_id) is False
+            assert api.CISuite.is_item_itr_skippable(non_skippable_suite_id) is False
+            assert api.CISuite.is_item_itr_skippable(non_skippable_module_skippable_suite_id) is False
+
+            CIVisibility.disable()

tests/ci_visibility/test_ci_visibility.py

@@ -1673,10 +1673,10 @@ class TestIsITRSkippable:
     m3_s2_t7 = api.CITestId(m3_s2, "test_6[param3]")
 
     def _get_all_suite_ids(self):
-        return {getattr(self, suite_id) for suite_id in vars(self) if re.match(r"^m\d_s\d$", suite_id)}
+        return {getattr(self, suite_id) for suite_id in vars(self.__class__) if re.match(r"^m\d_s\d$", suite_id)}
 
     def _get_all_test_ids(self):
-        return {getattr(self, test_id) for test_id in vars(self) if re.match(r"^m\d_s\d_t\d$", test_id)}
+        return {getattr(self, test_id) for test_id in vars(self.__class__) if re.match(r"^m\d_s\d_t\d$", test_id)}
 
     def test_is_item_itr_skippable_test_level(self):
         with mock.patch.object(CIVisibility, "enabled", True), mock.patch.object(
@@ -1722,12 +1722,15 @@ def test_is_item_itr_skippable_suite_level(self):
             mock_instance._tests_to_skip = defaultdict(list)
             mock_instance._suite_skipping_mode = True
 
+            expected_skippable_suite_ids = {self.m1_s1, self.m2_s1, self.m2_s2, self.m3_s1}
+            expected_non_skippable_suite_ids = self._get_all_suite_ids() - set(expected_skippable_suite_ids)
+
             # Check skippable suites are correct
-            for suite_id in [self.m1_s1, self.m2_s1, self.m2_s2, self.m3_s1]:
+            for suite_id in expected_skippable_suite_ids:
                 assert CIVisibility.is_item_itr_skippable(suite_id) is True
 
             # Check non-skippable suites are correct
-            for suite_id in [self.m1_s2, self.m3_s2]:
+            for suite_id in expected_non_skippable_suite_ids:
                 assert CIVisibility.is_item_itr_skippable(suite_id) is False
 
             # Check all tests are not skippable

tests/ci_visibility/util.py

@@ -1,8 +1,15 @@
+from collections import defaultdict
 from contextlib import contextmanager
+from unittest import mock
 
 import ddtrace
 from ddtrace.internal.ci_visibility import DEFAULT_CI_VISIBILITY_SERVICE
+from ddtrace.internal.ci_visibility.git_client import METADATA_UPLOAD_STATUS
+from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient
+from ddtrace.internal.ci_visibility.recorder import CIVisibility
+from ddtrace.internal.ci_visibility.recorder import _CIVisibilitySettings
 from tests.utils import DummyCIVisibilityWriter
+from tests.utils import override_env
 
 
 @contextmanager
@@ -22,3 +29,78 @@ def _get_default_civisibility_ddconfig():
         },
     )
     return new_ddconfig
+
+
+@contextmanager
+def set_up_mock_civisibility(
+    use_agentless: bool = True,
+    coverage_enabled: bool = False,
+    skipping_enabled: bool = False,
+    itr_enabled: bool = False,
+    require_git: bool = False,
+    suite_skipping_mode: bool = False,
+    skippable_items=None,
+):
+    """This is a one-stop-shop that patches all parts of CI Visibility for testing.
+
+    Its purpose is to allow testers to call CIVisibility.enable() without side effects and with predictable results
+    while still exercising most of the internal (eg: non-API, non-subprocess-executing) code.
+
+    It prevents:
+    * requests to settings and skippable API endpoints
+    * git client instantiation and use (skipping git metadata upload)
+
+    It additionally raises NotImplementedErrors to try and alert callers if they are trying to do something that should
+    be mocked, but isn't.
+    """
+
+    def _fake_fetch_tests_to_skip(*args, **kwargs):
+        if skippable_items is None:
+            if suite_skipping_mode:
+                CIVisibility._instance._test_suites_to_skip = []
+            else:
+                CIVisibility._instance._tests_to_skip = defaultdict(list)
+        else:
+            if suite_skipping_mode:
+                CIVisibility._instance._test_suites_to_skip = skippable_items
+            else:
+                CIVisibility._instance._tests_to_skip = skippable_items
+
+    def _mock_upload_git_metadata(obj, **kwargs):
+        obj._metadata_upload_status = METADATA_UPLOAD_STATUS.SUCCESS
+
+    env_overrides = {
+        "DD_CIVISIBILITY_AGENTLESS_ENABLED": "False",
+        "DD_SERVICE": "civis-test-service",
+        "DD_ENV": "civis-test-env",
+    }
+    if use_agentless:
+        env_overrides.update({"DD_API_KEY": "civisfakeapikey", "DD_CIVISIBILITY_AGENTLESS_ENABLED": "true"})
+    if suite_skipping_mode:
+        env_overrides.update({"_DD_CIVISIBILITY_ITR_SUITE_MODE": "true"})
+
+    with override_env(env_overrides), mock.patch(
+        "ddtrace.internal.ci_visibility.recorder.CIVisibility._check_enabled_features",
+        return_value=_CIVisibilitySettings(
+            coverage_enabled=coverage_enabled,
+            skipping_enabled=skipping_enabled,
+            require_git=require_git,
+            itr_enabled=itr_enabled,
+        ),
+    ), mock.patch(
+        "ddtrace.internal.ci_visibility.recorder.CIVisibility._fetch_tests_to_skip",
+        side_effect=_fake_fetch_tests_to_skip,
+    ), mock.patch.multiple(
+        CIVisibilityGitClient,
+        _get_repository_url=classmethod(lambda *args, **kwargs: "git@github.com:TestDog/dd-test-py.git"),
+        _is_shallow_repository=classmethod(lambda *args, **kwargs: False),
+        _get_latest_commits=classmethod(lambda *args, **kwwargs: ["latest1", "latest2"]),
+        _search_commits=classmethod(lambda *args: ["latest1", "searched1", "searched2"]),
+        _get_filtered_revisions=classmethod(lambda *args, **kwargs: "revision1\nrevision2"),
+        _upload_packfiles=classmethod(lambda *args, **kwargs: None),
+        upload_git_metadata=_mock_upload_git_metadata,
+        _do_request=NotImplementedError,
+    ), mock.patch(
+        "ddtrace.internal.ci_visibility.recorder._do_request", side_effect=NotImplementedError
+    ):
+        yield