diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index e0145a3b793..c72600a8e7b 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -335,7 +335,7 @@ def _fetch_tests_to_skip(self, skipping_mode): self._test_suites_to_skip = [] if response.status >= 400: - log.warning("Test skips request responded with status %d", response.status) + log.warning("Skippable tests request responded with status %d", response.status) return try: if isinstance(response.body, bytes): @@ -343,18 +343,27 @@ def _fetch_tests_to_skip(self, skipping_mode): else: parsed = json.loads(response.body) except json.JSONDecodeError: - log.warning("Test skips request responded with invalid JSON '%s'", response.body) + log.warning("Skippable tests request responded with invalid JSON '%s'", response.body) return - for item in parsed["data"]: - if item["type"] == skipping_mode and "suite" in item["attributes"]: - module = item["attributes"].get("configurations", {}).get("test.bundle", "").replace(".", "/") - path = "/".join((module, item["attributes"]["suite"])) if module else item["attributes"]["suite"] + if "data" not in parsed: + log.warning("Skippable tests request missing data, no tests will be skipped") + return - if skipping_mode == SUITE: - self._test_suites_to_skip.append(path) - else: - self._tests_to_skip[path].append(item["attributes"]["name"]) + try: + for item in parsed["data"]: + if item["type"] == skipping_mode and "suite" in item["attributes"]: + module = item["attributes"].get("configurations", {}).get("test.bundle", "").replace(".", "/") + path = "/".join((module, item["attributes"]["suite"])) if module else item["attributes"]["suite"] + + if skipping_mode == SUITE: + self._test_suites_to_skip.append(path) + else: + self._tests_to_skip[path].append(item["attributes"]["name"]) + except Exception: + log.warning("Error processing skippable test data, no tests will be skipped", exc_info=True) + self._test_suites_to_skip = [] + self._tests_to_skip = defaultdict(list) def _should_skip_path(self, path, name, test_skipping_mode=None): if test_skipping_mode is None: diff --git a/tests/ci_visibility/test_ci_visibility.py b/tests/ci_visibility/test_ci_visibility.py index 980cc2f4ab5..ec21a3605c1 100644 --- a/tests/ci_visibility/test_ci_visibility.py +++ b/tests/ci_visibility/test_ci_visibility.py @@ -1,5 +1,7 @@ +from collections import defaultdict import json import os +import textwrap import time import mock @@ -10,6 +12,8 @@ from ddtrace.ext import ci from ddtrace.internal.ci_visibility import CIVisibility from ddtrace.internal.ci_visibility.constants import REQUESTS_MODE +from ddtrace.internal.ci_visibility.constants import SUITE +from ddtrace.internal.ci_visibility.constants import TEST from ddtrace.internal.ci_visibility.encoder import CIVisibilityEncoderV01 from ddtrace.internal.ci_visibility.filters import TraceCiVisibilityFilter from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient @@ -1030,6 +1034,256 @@ def test_encoder_py2_payload_force_unicode_strings(): ) == {u"string_key": [1, {u"unicode_key": u"string_value"}, u"unicode_value", {u"string_key": u"unicode_value"}]} +class TestFetchTestsToSkip: + @pytest.fixture(scope="function") + def mock_civisibility(self): + with mock.patch.object(CIVisibility, "__init__", return_value=None): + _civisibility = CIVisibility() + _civisibility._api_key = "notanapikey" + _civisibility._dd_site = "notdatadog.notcom" + _civisibility._service = "test-service" + _civisibility._git_client = None + _civisibility._requests_mode = REQUESTS_MODE.AGENTLESS_EVENTS + _civisibility._tags = { + ci.git.REPOSITORY_URL: "test_repo_url", + ci.git.COMMIT_SHA: "testcommitsssshhhaaaa1234", + } + + yield _civisibility + CIVisibility._test_suites_to_skip = None + CIVisibility._tests_to_skip = defaultdict(list) + + @pytest.fixture(scope="class", autouse=True) + def _test_context_manager(self): + with mock.patch("ddtrace.ext.ci._get_runtime_and_os_metadata"), mock.patch("json.dumps", return_value=""): + yield + + def test_fetch_tests_to_skip_test_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body=textwrap.dedent( + """{ + "data": [ + { + "id": "123456789", + "type": "test", + "attributes": { + "configurations": { + "test.bundle": "testbundle" + }, + "name": "test_name_1", + "suite": "test_suite_1.py" + } + }, + { + "id": "987654321", + "type": "test", + "attributes": { + "configurations": { + "test.bundle": "testpackage/testbundle" + }, + "name": "test_name_2", + "suite": "test_suite_2.py" + } + } + ] + }""" + ), + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(TEST) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == { + "testbundle/test_suite_1.py": ["test_name_1"], + "testpackage/testbundle/test_suite_2.py": ["test_name_2"], + } + + def test_fetch_tests_to_skip_suite_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body=textwrap.dedent( + """{ + "data": [ + { + "id": "34640cc7ce80c01e", + "type": "suite", + "attributes": { + "configurations": { + "test.bundle": "testbundle" + }, + "suite": "test_module_1.py" + } + }, + { + "id": "239fa7de754db779", + "type": "suite", + "attributes": { + "configurations": { + "test.bundle": "testpackage/testbundle" + }, + "suite": "test_suite_2.py" + } + } + ] + }""" + ), + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [ + "testbundle/test_module_1.py", + "testpackage/testbundle/test_suite_2.py", + ] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_no_data_test_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body="{}", + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(TEST) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_no_data_suite_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body="{}", + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_data_is_none_test_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body='{"data": null}', + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(TEST) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_data_is_none_suite_level(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body='{"data": null}', + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_bad_json(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body="{ this is not valid JSON { ", + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_bad_response(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=500, + body="Internal server error", + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_test_to_skip_invalid_data_missing_key(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body='{"data": [{"somekey": "someval"}]}', + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_test_to_skip_invalid_data_type_error(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body=textwrap.dedent( + """{ + "data": [ { + "id": "12345", + "type": "suite", + "attributes": { + "configurations": { + "test.bundle": "2" + }, + "suite": 1 + } + }]}""", + ), + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_test_to_skip_invalid_data_attribute_error(self, mock_civisibility): + with mock.patch( + "ddtrace.internal.ci_visibility.recorder._do_request", + return_value=Response( + status=200, + body=textwrap.dedent( + """{ + "data": [ { + "id": "12345", + "type": "suite", + "attributes": { + "configurations": { + "test.bundle": 2 + }, + "suite": "1" + } + }]}""", + ), + ), + ): + ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config() + mock_civisibility._fetch_tests_to_skip(SUITE) + assert mock_civisibility._test_suites_to_skip == [] + assert mock_civisibility._tests_to_skip == {} + + def test_fetch_tests_to_skip_custom_configurations(): with override_env( dict(