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

Action: Add test for unstable Ruby VMs #1550

Merged
merged 6 commits into from
Jun 10, 2021
Merged

Action: Add test for unstable Ruby VMs #1550

merged 6 commits into from
Jun 10, 2021

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Jun 8, 2021

This PR runs the latest, still unstable, version of CRuby, JRuby, and TruffleRuby against our existing test suite.

I used GitHub Actions instead of CircleCI because there's not easy way to get bleeding edge Ruby images in CircleCI, while the GitHub Action ruby/setup-ruby has great support for it out of the box.

Luckily, they all the tests I could run pretty much pass, except for JRuby ruby/setup-ruby action being non-functional at this moment (not related to ddtrace). I've opened an issue description with the problem.

There was only a single fix required for Ruby 3.1 support as of today.

I did not attempt to run tests that depended on external services (MySQL, Redis, MongoDB, etc.), and stuck to the ones that only depend on Ruby code.

I'll leave this PR in draft until the JRuby issue gets traction.

@marcotc marcotc self-assigned this Jun 8, 2021
@ivoanjo
Copy link
Member

ivoanjo commented Jun 9, 2021

Since head may fail due to unrelated issues, what do you think of adding something like continue-on-error? (see [1, 2])

That would even enable us to merge this as-is now: JRuby is failing, no impact :)

@marcotc
Copy link
Member Author

marcotc commented Jun 9, 2021

I come back to actions/runner#2347 every so often and it just makes me sad that there's no support for it as described 😢.

I guess we have not other way today, except completely hiding the output in case of error, but that's just GitHub actions I guess.

@codecov-commenter
Copy link

Codecov Report

Merging #1550 (53548dd) into master (b20dc3b) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1550      +/-   ##
==========================================
- Coverage   98.24%   98.23%   -0.01%     
==========================================
  Files         878      878              
  Lines       42372    42372              
==========================================
- Hits        41627    41626       -1     
- Misses        745      746       +1     
Impacted Files Coverage Δ
spec/ddtrace/span_spec.rb 100.00% <100.00%> (ø)
spec/ddtrace/profiling/ext/forking_spec.rb 99.38% <0.00%> (-0.62%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b20dc3b...53548dd. Read the comment docs.

@marcotc marcotc marked this pull request as ready for review June 9, 2021 22:13
@marcotc marcotc requested a review from a team June 9, 2021 22:13
@marcotc marcotc added the dev/ci Involves CircleCI, GitHub Actions, or GitLab label Jun 9, 2021
@marcotc
Copy link
Member Author

marcotc commented Jun 9, 2021

As documented in actions/runner#2347, the only way to get a green "total" CI outcome today, while allowing failures, is to add continue-on-error on a per-step basis. This also mean that the failure outcome of CI is hidden.

@ivoanjo
Copy link
Member

ivoanjo commented Jun 10, 2021

As documented in actions/runner#2347, the only way to get a green "total" CI outcome today, while allowing failures, is to add continue-on-error on a per-step basis. This also mean that the failure outcome of CI is hidden.

Meh indeed. I wonder how difficult it would be to post those outcomes as a GitHub comment, or as a slack message, something like "Hey, this is low priority/non-blocking but your PR failed to build on whatever-head".

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.

LGTM 👍

@marcotc marcotc merged commit 638a836 into master Jun 10, 2021
@marcotc marcotc deleted the action/ruby-head branch June 10, 2021 18:53
@github-actions github-actions bot added this to the 0.51.0 milestone Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev/ci Involves CircleCI, GitHub Actions, or GitLab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants