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

[SDTEST-408] multi threaded code coverage support for datadog_cov #189

Merged
merged 9 commits into from
Jun 10, 2024

Conversation

anmarchenko
Copy link
Member

@anmarchenko anmarchenko commented Jun 5, 2024

What does this PR do?
Fixes an issue when per test code coverage for ITR ignored background threads. This led to the coverage being incomplete in the most cases as Rails uses threading for background jobs testing for example.

Now by default code coverage is collected for all threads, not only the current thread of the test. I added a parameter that will allow the library to switch back to track single thread in a less probable situation where threaded parallel test runner is used.

Motivation
Reports of incomplete code coverage when running Rails tests

Additional notes
This is built on top of PR that improves code coverage tool performance: #171

How to test the change?
Unit tests are provided

@anmarchenko anmarchenko changed the title multi threaded code coverage support for datadog_cov [SDTEST-408] multi threaded code coverage support for datadog_cov Jun 5, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.86%. Comparing base (b04133e) to head (19e5d8b).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #189      +/-   ##
==========================================
+ Coverage   98.85%   98.86%   +0.01%     
==========================================
  Files         228      229       +1     
  Lines       10082    10172      +90     
  Branches      465      468       +3     
==========================================
+ Hits         9967    10057      +90     
  Misses        115      115              

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

@anmarchenko anmarchenko marked this pull request as ready for review June 6, 2024 08:24
@anmarchenko anmarchenko requested review from a team as code owners June 6, 2024 08:24
@anmarchenko anmarchenko force-pushed the anmarchenko/code_coverage_improvements branch from b3db6ae to 0da2baa Compare June 7, 2024 07:09
@anmarchenko anmarchenko force-pushed the anmarchenko/code_coverage_multi_threading_support branch 3 times, most recently from 8c3bf11 to 305174a Compare June 10, 2024 11:14
Copy link
Member

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Seems reasonable!

Given that dd-trace-rb also has a bunch of background threads, would it make sense to exclude them by default?

E.g. by looking at their names for instance (all of our threads should be named with the Datadog:: prefix).

ext/datadog_cov/datadog_cov.c Outdated Show resolved Hide resolved
ext/datadog_cov/datadog_cov.c Outdated Show resolved Hide resolved
sig/datadog/ci/itr/coverage/ddcov.rbs Outdated Show resolved Hide resolved
@anmarchenko anmarchenko force-pushed the anmarchenko/code_coverage_multi_threading_support branch from 305174a to 18edce6 Compare June 10, 2024 11:27
Base automatically changed from anmarchenko/code_coverage_improvements to main June 10, 2024 11:37
@anmarchenko anmarchenko force-pushed the anmarchenko/code_coverage_multi_threading_support branch from 4ab5804 to 573eeda Compare June 10, 2024 12:01
@anmarchenko anmarchenko merged commit dcc86b9 into main Jun 10, 2024
28 checks passed
@anmarchenko anmarchenko deleted the anmarchenko/code_coverage_multi_threading_support branch June 10, 2024 13:34
@github-actions github-actions bot added this to the 1.1.0 milestone Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants