Skip to content

ci: switch repo to use pytest plugin v2 #11379

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

Merged

Conversation

romainkomorn-exdatadog
Copy link
Contributor

@romainkomorn-exdatadog romainkomorn-exdatadog commented Nov 13, 2024

This makes tweaks to run the new version of the pytest plugin on the dd-trace-py repostitory.

  • the coverage collector is tweaked to only cover use code, and always ignore the coverage module itself
  • ITR is disabled for the appsec suite
  • _CI_DD_ENV is introduced as a way to override the DD_ENV from the environment (as setting it in our current CI breaks dozens of unrelated tests)
  • the -s flag is added to pytest invocations for output to appear
  • a test is refactored to account for the fact that ModuleWatchdog now has children classes that may be active

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

Copy link
Contributor

github-actions bot commented Nov 13, 2024

CODEOWNERS have been resolved as:

.gitlab/tests.yml                                                       @DataDog/python-guild @DataDog/apm-core-python
ddtrace/contrib/pytest/_plugin_v2.py                                    @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/recorder.py                              @DataDog/ci-app-libraries
ddtrace/internal/ci_visibility/writer.py                                @DataDog/ci-app-libraries
ddtrace/internal/coverage/code.py                                       @DataDog/apm-core-python @datadog/ci-app-libraries
riotfile.py                                                             @DataDog/apm-python
tests/internal/test_module.py                                           @DataDog/debugger-python @DataDog/apm-core-python

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 13.91%. Comparing base (7ceea02) to head (d47ca80).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
ddtrace/internal/coverage/code.py 0.00% 4 Missing ⚠️
tests/internal/test_module.py 0.00% 3 Missing ⚠️
...dtrace/internal/coverage/instrumentation_py3_10.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11379      +/-   ##
==========================================
- Coverage   13.93%   13.91%   -0.02%     
==========================================
  Files        1520     1530      +10     
  Lines      131435   132660    +1225     
==========================================
+ Hits        18313    18463     +150     
- Misses     113122   114197    +1075     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@romainkomorn-exdatadog
Copy link
Contributor Author

@christophe-papazian , mind taking a peek at the riotfile.py change for appsec ?

@pr-commenter
Copy link

pr-commenter bot commented Nov 13, 2024

Benchmarks

Benchmark execution time: 2024-11-14 17:21:28

Comparing candidate commit 6d139ad in PR branch romain.komorn/chore/enable_pytest_v2_plugin_for_repo with baseline commit c7246a4 in branch main.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 387 metrics, 2 unstable metrics.

scenario:iast_aspects-ospathnormcase_aspect

  • 🟩 execution_time [-221.150ns; -199.170ns] or [-8.272%; -7.450%]

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

datadog-dd-trace-py-rkomorn bot commented Nov 14, 2024

Datadog Report

Branch report: romain.komorn/chore/enable_pytest_v2_plugin_for_repo
Commit report: 6d139ad
Test service: dd-trace-py

✅ 0 Failed, 14 Passed, 1454 Skipped, 1m 3.15s Total duration (35m 44.09s time saved)

@romainkomorn-exdatadog romainkomorn-exdatadog enabled auto-merge (squash) November 14, 2024 16:41
@romainkomorn-exdatadog romainkomorn-exdatadog merged commit 6c628db into main Nov 15, 2024
533 checks passed
@romainkomorn-exdatadog romainkomorn-exdatadog deleted the romain.komorn/chore/enable_pytest_v2_plugin_for_repo branch November 15, 2024 08:52
brettlangdon pushed a commit that referenced this pull request Jul 11, 2025
Test output capture was disabled (with the pytest `-s` option) in PR
#11379, when the pytest v2 plugin was introduced (I suppose because we
wanted to check if the new plugin was behaving properly at the time).
This is not needed anymore. This PR removes the `-s` flag.

## Checklist
- [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
alyshawang pushed a commit that referenced this pull request Jul 25, 2025
Test output capture was disabled (with the pytest `-s` option) in PR
#11379, when the pytest v2 plugin was introduced (I suppose because we
wanted to check if the new plugin was behaving properly at the time).
This is not needed anymore. This PR removes the `-s` flag.

## Checklist
- [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
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.

4 participants