-
Notifications
You must be signed in to change notification settings - Fork 564
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 coverage for eval #1037
Conversation
Since Ruby 3.2, coverage.so can measure coverage of code that evaluated by `Kernel#eval`. Typically, this would be useful for ERB. https://bugs.ruby-lang.org/issues/19008 This change adds a new API for SimpleCov to enable the feature. ```ruby SimpleCov.start do enable_coverage_for_eval end ```
... instead of raising an exception
How about this PR? Ruby 3.2.0 will be released in a little over two weeks. Sorry to be so selfish, but I hope simplecov will be released in time. |
@colszowka Sorry for the name calling. What do you think of this? SimpleCov is the de facto coverage measurement tool for Ruby. Unless SimpleCov supports coverage for eval, there is no point in providing the feature. I would appreciate your feedback. |
@mame Hey, thanks for the PR, this looks very nice! I saw it last week already but didn't yet have the time to take a closer look unfortunately, but I will take a look in the next days and will cut a new release in time before ruby day 👍 |
@colszowka Thank you! If there's anything I can do to help, feel free to let me know. |
Hey @mame, v0.22.0 featuring this and #1035 are now out on rubygems, thanks again for your contributions! I have two questions regarding this new feature:
|
Thank you very much!
I am not familiar with the implementation of Rails. It works on Rails out of the box by the following procedure:
And you will be able to see the coverage of "app/views/layouts/application.html.erb" in coverge/index.html.
Hm, indeed it is not documented yet. The simplest answer is that Coverage handles the third argument of
ERB provides |
@mame Thanks for the quick reply! Regarding rails, my understanding was that rails was using erubis instead of irb, and I saw that it's still a dependency of Actionpack, so I assumed that this would mean that it wouldn't work out of the box there, but if it works already that's very cool, thanks for the clarification 🎉! I wanted to try it out myself yesterday but had some issue installing ruby head locally and ran into some CI issues that were blocking me from publishing the new version, so I focused on that.
I think having some basic example code on how to integrate with this would be really nice, I think it would suffice to show how to pass the neccessary arguments to eval for an arbitrary eval code block, as a reference for other libraries i.e. for templates on how to integrate this 📒 We could then also point to this from the readme docs on here for anyone interested in integrating with that |
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
This is now possible with ruby 3.2.1 as per: simplecov-ruby/simplecov#1037 Add specs for missing coverage.
After enabling this via
I've disabled it for now but let me know if there's anything I can do to help diagnose. |
It probably means something is wrong with the template (erb has some known issues). |
@ioquatix Thanks for the quick reply - didn't expect anything back. :) When you say "something is wrong", is it something that we've done with the template or do you mean just that |
Last time I checked, there were some issues with |
Since you have not provided your templates, I cannot know for sure what is going on. However, my guess is an ERB template with 5 lines of input code, generates 6 lines of "compiled template". I've made some code to play around with this: ![]() I added a quick hack to print out the number of lines of code and the number of coverage counts captured. You can see a difference 8 vs 9. See the files here: https://github.com/ioquatix/covered/tree/main/examples/coverage/erb for the code. In terms of a probable fix for |
Hey @ioquatix, thanks for the detailed response here! I re-ran the tests and picked out a simple one to look at as well: Here's the <%# TODO: ... %>
<% person ||= current_user # Assume current user if nobody passed in. %>
<% style ||= "" # Assume no specific styles. %>
<% if person.avatar.nil? %>
<%= image_tag "avatar_icon.png", class: "avatar circle", style: style %>
<% else %>
<%= image_tag person.avatar.url( :medium ), class: "avatar circle", style: style %>
<% end -%>
And here is the warning message:
|
According to your template:
ERB compiles to:
Note that I've set the ERB template to base-0 line numbers. That's because of this: https://github.com/ruby/erb/blob/3d3df5ce5c9cb9564bdee8b02b0bb4fd81981e82/lib/erb.rb#L353 which is given to https://github.com/ruby/erb/blob/3d3df5ce5c9cb9564bdee8b02b0bb4fd81981e82/lib/erb.rb#L429 There are two fixes:
|
@ioquatix Great analysis! This is above my head to fix but good reference for anybody that wants to take a stab at this. For now I think I'll either suppress the warning or disable the |
I think you can also just ignore the warnings, no harm is caused AFAIK. |
It's just nasty, though. :) In our case, over 100+ lines of these warnings. Makes our test output noisy. |
Does it mean that the following part of the readme is somewhat obsolete for Ruby 3.2+: |
Since Ruby 3.2, coverage.so can measure coverage of code that evaluated by
Kernel#eval
. Typically, this would be useful for ERB.https://bugs.ruby-lang.org/issues/19008
This change adds a new API for SimpleCov to enable the feature.
Here is a sample output:
I hope this feature will be released until the release date of Ruby 3.2 (25th in this month).