-
Notifications
You must be signed in to change notification settings - Fork 552
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
Support Ruby 3.0, 3.1 and 3.2 #1035
Conversation
Since Ruby 3.1, Coverage.start raises an exception if the coverage measurement is already started. ``` $ ruby -v -rcoverage -e 'Coverage.start; Coverage.start' ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [x86_64-linux] -e:1:in `start': coverage measurement is already setup (RuntimeError) from -e:1:in `<main>' ``` This should fix the issue simplecov-ruby#1020
The following gems need to be updated to pass the test suite on Ruby 3.0, 3.1 and 3.2 * rails: 6.1.0 -> 6.1.4 * capybara: 3.26 -> 3.36.0
... and jruby-9.2 which is compatible with Ruby 2.5. This is because capybara-3.36.0 requires ruby version >= 2.6.0. Ruby 2.5 series is already EOL at 2021-03-31. Instead, Ruby 3.1, ruby-head, and jruby-9.3 are added.
CI passed. Ruby 2.5 had to be removed from CI due to test code. The library code itself works. Can you review this PR? |
@mame Thank you! 👏 I think since Ruby 2.6 is also already EOL it would be fine to actually also drop that as well from CI and official support, and then adjust the corresponding section in the README to request Ruby 2.7 or JRuby 9.3. Would you be interested in updating the PR accordingly so we have it all in one place? Otherwise I can also merge and will make the corresponding changes as a followup |
@colszowka Hi, thank you for your quick response! I have added a commit to drop 2.6 and update README.md as you said. If you have any other concerns, please feel free to let me know. |
@mame Thank you, for taking the time to adjust those things! Merged ✔️ |
Excited to see this! We are upgrading our Rails host to Ruby 3 / Rails 7 and I was concerned we might need to discontinue use of |
@@ -342,7 +342,7 @@ def start_coverage_measurement | |||
if coverage_start_arguments_supported? | |||
start_coverage_with_criteria | |||
else | |||
Coverage.start | |||
Coverage.start unless Coverage.running? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to note, Coverage.running?
is not implemented on Truffleruby, so this fails. I have opened an issue there: oracle/truffleruby#2813
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these unless Coverage.running?
checks necessary?
Otherwise I would suggest to drop them to avoid breaking existing TruffleRuby releases and potentially many CI runs on TruffleRuby.
Of course we'll implement the method on TruffleRuby master, but unlikely before 2023 and won't affect existing releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the issue is partially described in 1cc7650
Coverage.start
raises now if already running.
It also feels like something would be wrong if you started coverage while it's already running. I have no doubt though that maybe the SimpleCov code, but most definitely various degrees of client code would break.
Since we are (still :P) 0.x and "well actually" Ruby 3 broke compatibility I'm kinda ok with that personally.
@mame am I missing something? 🤔
Also, hi all 👋 Been a long time, wanted to get back into this sooner but first stress, then covid hit, then bunny problems hit. Slowly but surely getting here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the context. I think then the best would be to add a private method like coverage_enabled?
and track it as an ivar on SimpleCov
and use Coverage.running?
if available and that ivar otherwise. I'll try to make a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@notEthan looonng story, but well we got Ghost back from the vet yesterday after almost 3 weeks and multiple very close calls with death.
she still looks a bit roughed up:
This PR has two commits to make the test suite pass on Ruby 3.0, 3.1 and 3.2