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

Add Minitest CI integration #2932

Merged
merged 4 commits into from
Jul 5, 2023
Merged

Conversation

bravehager
Copy link
Contributor

What does this PR do?

Add CI Visibility support for the Minitest framework.

Motivation

Datadog doesn't currently support using Minitest with the CI Visibility feature. It's easy enough to add your own custom integration, but I think it makes sense for dd-trace-rb to provide first-class support for Minitest given that it's the default testing framework for Ruby on Rails.

Additional Notes

  • RSpec uses a relative path with the ./ prefix for the test suite name. Right now, we're using Pathname.new(path).relative_path_from(Pathname.pwd).to_s which doesn't include the ./ prefix. Should we mirror the RSpec setup?
  • Currently, it only supports minitest/unit, I haven't dug into what it would take to support minitest/spec.
  • Minitest supports skip reasons, but provides a default message of "Skipped, no message given". Should we exclude the default message assuming that it doesn't provide much value?

I realize this is a pretty big patch that you might not want to accept from an outside contributor, but I figured I'd put together a PR since we've been using this internally.

How to test the change?

  • Added unit tests for instrumentation

@bravehager bravehager requested a review from a team June 27, 2023 15:21
@github-actions github-actions bot added the ci-app CI product for test suite instrumentation label Jun 27, 2023
@marcotc
Copy link
Member

marcotc commented Jun 27, 2023

👋 @bravehager, thank you so much for the contribution!

I'm pinging the CI team to get their 👀 here asap.

Thank you again!

Copy link
Contributor

@juan-fernandez juan-fernandez left a comment

Choose a reason for hiding this comment

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

this is great, thanks for this! ❤️

I've tested this end to end with the test suite in https://github.com/freeCodeCamp/devdocs and it works wonderfully.

@bravehager
Copy link
Contributor Author

bravehager commented Jun 30, 2023

Anything I can do to help get this merged? I see there's a CI check blocked but seems resolved by #2929, so might just need a merge or rebase.

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Thank you so much, @bravehager!

@marcotc
Copy link
Member

marcotc commented Jun 30, 2023

Could you add a section for Minitest documentation, similar to the RSpec one here: https://github.com/DataDog/dd-trace-rb/blob/master/docs/GettingStarted.md#rspec

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Merging #2932 (9311ef9) into master (18b6322) will decrease coverage by 0.03%.
The diff coverage is 74.73%.

@@            Coverage Diff             @@
##           master    #2932      +/-   ##
==========================================
- Coverage   97.96%   97.93%   -0.03%     
==========================================
  Files        1282     1287       +5     
  Lines       70959    71052      +93     
  Branches     3265     3272       +7     
==========================================
+ Hits        69512    69586      +74     
- Misses       1447     1466      +19     
Impacted Files Coverage Δ
lib/datadog/ci/contrib/minitest/test_helper.rb 35.71% <35.71%> (ø)
lib/datadog/ci/contrib/minitest/patcher.rb 84.61% <84.61%> (ø)
lib/datadog/ci/contrib/minitest/integration.rb 87.50% <87.50%> (ø)
...adog/ci/contrib/minitest/configuration/settings.rb 94.11% <94.11%> (ø)
lib/datadog/ci.rb 100.00% <100.00%> (ø)
lib/datadog/ci/contrib/minitest/ext.rb 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@marcotc
Copy link
Member

marcotc commented Jul 4, 2023

Thank you so much for the documentation!

I noticed one last thing: we have rbs type signatures in the repository and check them with steep. I noticed there's a type checking failure in the PR (Check / Check types): https://github.com/DataDog/dd-trace-rb/actions/runs/5427100107/jobs/9930832333?pr=2932

Could you follow the steps in the failure output and see if you can address it. If you find that the guide referenced there is insufficient please let us know so we can improve it.

@marcotc marcotc merged commit fc78216 into DataDog:master Jul 5, 2023
@github-actions github-actions bot added this to the 1.13.0 milestone Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-app CI product for test suite instrumentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants