-
Notifications
You must be signed in to change notification settings - Fork 553
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
Coverage on ERB #38
Comments
No, unfortunately it's not since the underlying Ruby 1.9 Stdlib Coverage library only tracks files that are Templates on the other hand are read from a file and evaluated later on, and thus are out of the visibility of the coverage lib. Sorry :/ |
Very old ticket, but do you know if this is possible with any other 1.9 coverage tools? |
This is a "spike" which demonstrates how to obtain test coverage data for the `additional-commodity-code` flow. This data was not previously available [1], because the flow files were being read and eval'ed rather than simply required. The latter is necessary [2] for `SimpleCov` to record coverage data for the file. By extracting the existing DSL code into a `define` method on a subclass of `SmartAnswer::Flow`, we can safely require the file separately from instantiating the flow. This feels like a better state of affairs in general, but it specifically addresses the test coverage problem above. We'd really like to have this coverage data available so we can have confidence that we're not breaking anything as we continue to refactor the app. Note that I've intentionally not fixed the indentation in the `additional-commodity-code` flow file so that it's easier to see my changes. [1]: https://ci-new.alphagov.co.uk/job/govuk_smart_answers/2396/rcov/ [2]: simplecov-ruby/simplecov#38
This makes it possible to define smart answer flows as a subclass of `SmartAnswer::Flow` such that we can obtain test coverage data for the flow logic. This data was not previously available [1], because the flow files were being read and eval'ed (at load time) rather than simply required. The latter is necessary [2] for `SimpleCov` to record coverage data for the file. By extracting the existing DSL code into a `define` method on a subclass of `SmartAnswer::Flow`, we can safely require the file separately from instantiating the flow. This feels like a better state of affairs in general, but it specifically addresses the test coverage problem above. We'd want to have this coverage data available so we can have confidence that we're not breaking anything as we continue to refactor the app. No flows have been converted in this commit, it just makes it possible to convert them. The plan is to convert each flow one at a time in subsequent commits. I've chosen to suffix the subclass name with the word `Flow`, because I hit autoloading problems when running **all** the tests, because there are a bunch of classes in the `SmartAnswer::Calculators` namespace with the same name as flows. [1]: https://ci-new.alphagov.co.uk/job/govuk_smart_answers/2396/rcov/ [2]: simplecov-ruby/simplecov#38
I know it is an old issue, but it might be possible to add a hook on ERB when it read a template file or it compiles it and add those calls to the result set. Does it makes sense? @colszowka ? |
What about HAML / Slim and friends? |
I were checking, and it seems that using trasepoint instead of coverage we
could track coverage of template files, I will do some testing on that,
what do you think about that approach?
El sáb., nov. 25, 2017 16:35, Dmitry Polushkin <notifications@github.com>
escribió:
… What about HAML / Slim and friends?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#38 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKm5XD1tEnfDtXIE88Hc2yvoc0F469mks5s6GvzgaJpZM4AShSg>
.
|
Any update on using tracepoints? |
I were trying just a bit, but found not way differentiate code that is not
being executed from lines that are being ignored.
For example, on the following code:
```
def foo bar
if bar > 2
bar
else
2
end
end
foo 3
```
I shuld have `[1,1,1,nil,0,nil,nil,nil,1]`. I have no way to distinguish
`nil` from `0` using tracepoint.
|
Yeah I tried using trace points and came to the same conclusion. :( |
I think you'd have to patch ERB in some way to cause the template text to
be considered a file after it was parsed but before it was eval'd.
I just tried making a template.rb
<% if true %>
"true"
<%= 'true' %>
<% else %>
"false"
<% end %>
And then from the terminal
ruby -rerb -rcoverage -e 'Coverage.start; puts
ERB.new(File.read("./template.rb")).result; p Coverage.result'
"true"
true
{}
Which didn't pickup up evaluating any Ruby files, which you'd expect since
the input to `ERB` is text that happened to be read in from a file, but
the file wasn't evaluated
…On Fri, Jan 26, 2018 at 2:11 PM, Samuel Williams ***@***.***> wrote:
Yeah I tried using trace points and came to the same conclusion. :(
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#38 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIuQlOG2h1ixJYYDuRRyokbRLg5a9BCks5tOjF9gaJpZM4AShSg>
.
|
I had a look in the MRI source code and for some reason it doesn't compute coverage for https://github.com/ruby/ruby/blob/582951e2c8995d6bab5ddaf98cd3816310f8d506/parse.y#L4785 It might just be a matter if being slightly more allowing at that line, i.e. if calling eval with the optional file name argument set to something. |
It works in covered but I don’t use standard coverage library because this bug still applies. |
I have created a PR that will solve this issue: #1037 |
Great work on this gem. Is it possible to check code coverage in ERB templates? In a Rails app there might be an "if" condition in a view which is not covered. It would be great if it were somehow possible to check this.
The text was updated successfully, but these errors were encountered: