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

Use current released version of simplecov for MRI #3977

Closed
wants to merge 4 commits into from

Conversation

p-datadog
Copy link
Contributor

@p-datadog p-datadog commented Oct 7, 2024

What does this PR do?

This PR changes to the most recent simplecov version for MRI. JRuby retains the patched simplecov tree.

Motivation:

It is annoying having to bundle install all the time because "simplecov version is not checked out".

Additional Notes:

The original patch was made for Ruby 2.3 (and 2.4?) and JRuby. It was submitted upstream in simplecov-ruby/simplecov#972 but the focus there was on MRI 2.3 and 2.4, and the patch has not been accepted so far (3 years) due to targeting old Ruby versions. However, it is also needed for JRuby.

I restricted the patch to JRubies only and made MRI use a regular simplecov release.

How to test the change?
I don't know if there are any requirements besides CI being green. Without the patch in place JRuby 9.3 and 9.4 configurations fail CI.

Unsure? Have a question? Request a review!

@pr-commenter
Copy link

pr-commenter bot commented Oct 7, 2024

Benchmarks

Benchmark execution time: 2024-10-07 14:33:05

Comparing candidate commit b123d69 in PR branch simplecov-version with baseline commit 56038a3 in branch master.

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

@ivoanjo
Copy link
Member

ivoanjo commented Oct 7, 2024

Oops, I saw there was a profiler flaky test on this PR, sorry for that! I'll look into it and disable it if needed until I can figure it out

@ivoanjo
Copy link
Member

ivoanjo commented Oct 7, 2024

(I've triggered a re-run of the failed specs to skip that one)

@p-datadog
Copy link
Contributor Author

JRuby 9.3 and 9.4 are failing with flat_map call on nil.

@ivoanjo
Copy link
Member

ivoanjo commented Oct 7, 2024

(Btw I've opened #3980 do add a bit more logging to the flaky spec, and hopefully I'll be able to debug the issue with that extra info)

@p-datadog p-datadog marked this pull request as ready for review October 8, 2024 16:58
@p-datadog p-datadog requested a review from a team as a code owner October 8, 2024 16:58
@p-datadog p-datadog changed the title Use current released version of simplecov Use current released version of simplecov for MRI Oct 8, 2024
@p-datadog
Copy link
Contributor Author

The coverage task is still failing, I think what is happening is since JRuby doesn't generate branch coverage, attempting to merge JRuby and MRI results from MRI will fail without the patch in simplecov-ruby/simplecov#972. Which means we always need that patch even on MRI.

@p-datadog p-datadog closed this Oct 8, 2024
@p-datadog p-datadog deleted the simplecov-version branch October 8, 2024 17:33
@ivoanjo
Copy link
Member

ivoanjo commented Oct 9, 2024

To be honest, our patch is so small, perhaps we could carry our patch in-tree and load it as a monkey patch instead?

E.g. right now we're using an older version of simplecov from git just to be able to have that change -- with the monkey patch we could probably use the latest version instead.

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