Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(test_visibility): rename and privatize CI Visibility API to Test Visibility #10425

Conversation

romainkomorndatadog
Copy link
Collaborator

@romainkomorndatadog romainkomorndatadog commented Aug 28, 2024

This PR renames the unreleased, to-be-externally-facing Test Visibility-related methods/classes/etc to use some variations of "test visibility"-themed names to reflect the fact that the CI Visibility product is now known as Test Visibility.

It also takes several pieces that were previously public and creates a new, internal, test visibility API that extends the external one.

Notably:

  • New core event handlers for enabling/disabling/getting status of test visibility added to the tracer's global handlers (test visibility-related handlers continue to only be added if the CIVisibility service is imported)
  • Externally facing changes
    • event hub names are renamed from ci_visibility.* to test_visibility.*
    • CI* API classes are renamed to Test*
    • Move of *ItemId classes to a separate file to reduce circular dependencies between external and internal API
    • Moved to internal interface:
      • ITR mixin classes
      • Methods to access item spans directly
  • Internal changes
    • CIVisibility* API classes renamed to TestVisibility
  • No test definition changes except for snapshot data
  • Unrelated bundled changes
    • Whether to use test-level or suite-level skipping in ITR is now configured in the introduced test_visibility integration config (renamed from ci_visibility)
    • the v2 pytest plugin now uses this configuration item to enable suite-level skipping

One important note is that DD_CIVISIBILITY_* environment variables remain untouched. This does lead to the incongruity of configuring Test Visibility with CI Visibility-named vars.

There are no release notes because the public API components are unreleased and marked as subject to change in comments.

Checklist

  • 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
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • 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 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

@romainkomorndatadog romainkomorndatadog added the changelog/no-changelog A changelog entry is not required for this PR. label Aug 28, 2024
@romainkomorndatadog romainkomorndatadog self-assigned this Aug 28, 2024
Copy link
Contributor

github-actions bot commented Aug 28, 2024

CODEOWNERS have been resolved as:

ddtrace/ext/test_visibility/__init__.py                                 @DataDog/ci-app-libraries
ddtrace/ext/test_visibility/_item_ids.py                                @DataDog/ci-app-libraries
ddtrace/ext/test_visibility/_utils.py                                   @DataDog/ci-app-libraries
ddtrace/ext/test_visibility/api.py                                      @DataDog/ci-app-libraries
ddtrace/internal/test_visibility/_utils.py                              @DataDog/ci-app-libraries
ddtrace/internal/test_visibility/api.py                                 @DataDog/ci-app-libraries
tests/ci_visibility/api/test_ext_test_visibility_api.py                 @DataDog/ci-app-libraries
tests/ci_visibility/api/test_internal_test_visibility_api.py            @DataDog/ci-app-libraries
.github/CODEOWNERS                                                      @DataDog/python-guild @DataDog/apm-core-python
ddtrace/_trace/trace_handlers.py                                        @DataDog/apm-sdk-api-python
ddtrace/contrib/pytest/_plugin_v2.py                                    @DataDog/ci-app-libraries
ddtrace/contrib/pytest/_utils.py                                        @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/__init__.py                              @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/constants.py                             @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/recorder.py                              @DataDog/ci-app-libraries
ddtrace/internal/coverage/code.py                                       @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/instrumentation_py3_10.py                     @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/instrumentation_py3_11.py                     @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/instrumentation_py3_12.py                     @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/instrumentation_py3_7.py                      @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/instrumentation_py3_8.py                      @DataDog/apm-core-python @datadog/ci-app-libraries
ddtrace/internal/coverage/multiprocessing_coverage.py                   @DataDog/apm-core-python @datadog/ci-app-libraries
tests/.suitespec.json                                                   @DataDog/python-guild @DataDog/apm-core-python
tests/ci_visibility/api/fake_runner_all_fail.py                         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_all_itr_skip_suite_level.py         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_all_itr_skip_test_level.py          @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_all_pass.py                         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_all_skip.py                         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_mix_fail.py                         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_mix_fail_itr_suite_level.py         @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_mix_fail_itr_test_level.py          @DataDog/ci-app-libraries
tests/ci_visibility/api/fake_runner_mix_pass.py                         @DataDog/ci-app-libraries
tests/ci_visibility/api/test_internal_ci_visibility_api.py              @DataDog/ci-app-libraries
tests/ci_visibility/test_ci_visibility.py                               @DataDog/ci-app-libraries
tests/ci_visibility/util.py                                             @DataDog/ci-app-libraries
tests/contrib/pytest/test_coverage_per_suite.py                         @DataDog/ci-app-libraries
tests/contrib/pytest/test_pytest.py                                     @DataDog/ci-app-libraries
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_all_fail.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_all_itr_skip_suite_level.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_all_itr_skip_test_level.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_all_pass.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_all_skip.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_mix_fail.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_mix_fail_itr_suite_level.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_mix_fail_itr_test_level.json  @DataDog/apm-python
tests/snapshots/test_api_fake_runners.test_manual_api_fake_runner_mix_pass.json  @DataDog/apm-python
ddtrace/ext/test_visibility/_test_visibility_base.py                    @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_base.py                             @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_coverage_data.py                    @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_module.py                           @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_session.py                          @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_suite.py                            @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/api/_test.py                             @DataDog/ci-app-libraries
ddtrace/internal/test_visibility/coverage_lines.py                      @DataDog/ci-app-libraries

@pr-commenter
Copy link

pr-commenter bot commented Aug 29, 2024

Benchmarks

Benchmark execution time: 2024-09-06 17:25:38

Comparing candidate commit 5b5db1d in PR branch romain.komorn/SDTEST-169/ci_visibilty_manual_api_cleanup with baseline commit a3e471b in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 353 metrics, 47 unstable metrics.

@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Aug 29, 2024

Datadog Report

Branch report: romain.komorn/SDTEST-169/ci_visibilty_manual_api_cleanup
Commit report: ac96f59
Test service: dd-trace-py

✅ 0 Failed, 138694 Passed, 1638 Skipped, 4h 14m 23.7s Total Time
❄️ 1 New Flaky

New Flaky Tests (1)

  • test_request_ipblock_nomatch_200 - test_django_appsec_snapshots.py - Last Failure

    Expand for error
     Unexpected test failure during snapshot test: HTTPConnectionPool(host='localhost', port=8000): Max retries exceeded with url: / (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7ff1ec2a0dc0>: Failed to establish a new connection: [Errno 111] Connection refused'))
    

@romainkomorndatadog romainkomorndatadog changed the title WIP chore(test_visibility): rename external CI Visibility API to Test Visibility chore(test_visibility): rename external CI Visibility API to Test Visibility Aug 29, 2024
@romainkomorndatadog romainkomorndatadog changed the title chore(test_visibility): rename external CI Visibility API to Test Visibility chore(test_visibility): rename and privatize CI Visibility API to Test Visibility Sep 4, 2024
@romainkomorndatadog
Copy link
Collaborator Author

@mabdinur , @ZStriker19 , could I get a review for the trace_handlerts.py file?

@tabgok , can I get a review on the... non-IDM IDM things?

@romainkomorndatadog romainkomorndatadog merged commit dafd6db into main Sep 6, 2024
408 of 409 checks passed
@romainkomorndatadog romainkomorndatadog deleted the romain.komorn/SDTEST-169/ci_visibilty_manual_api_cleanup branch September 6, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants