[ruby-core:114590] [Ruby master Bug#19857] Eval coverage is reset after each `eval`.

Issue #19857 has been reported by ioquatix (Samuel Williams). ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different computation is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). Example output: ```
./test.rb foo bar {"foo.rb"=>{:lines=>[1, 1, 0, nil, 1, nil, nil]}}
(swap lines)
./test.rb bar foo {"foo.rb"=>{:lines=>[1, 1, 1, nil, 0, nil, nil]}}
----------------------------------------
Bug #19857: Eval coverage is reset after each `eval`.
https://bugs.ruby-lang.org/issues/19857#change-104405
* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
* Assignee: ioquatix (Samuel Williams)
* Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
----------------------------------------
It seems like `eval` based coverage is reset every time eval is invoked.
```ruby
#!/usr/bin/env ruby
require 'coverage'
def measure(flag)
c = Class.new
c.class_eval(<<~RUBY, "foo.rb", 1)
def foo(flag)
if flag
puts "foo"
else
puts "bar"
end
end
RUBY
return c.new.foo(flag)
end
Coverage.start(lines: true, eval: true)
# Depending on the order of these two operations, different computation is calculated, because the evaluation of the code is considered different, even if the content/path is the same.
measure(false)
measure(true)
p Coverage.result
Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh). Currently only the coverage of the last call to `Kernel#eval` is kept. This seems somewhat reasonable to me because: * I don't think calling eval on the same filename has such an important use case. * If diffrent code is `eval`'d with the same filename, you get broken coverage results. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104406 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different computation is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). We found it was discussed here: https://bugs.ruby-lang.org/issues/19008#note-5 I'm a little embarrassed I didn't consider or fix it at the time. Anyway... Here is the proposed fix: https://github.com/ruby/ruby/pull/8330 Specifically, when an iseq is compiled code, it resets all the coverage to zero for lines that are executable. Instead of resetting to zero, we should leave it as is if it already contains coverage information. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104410 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different computation is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN to 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED
Currently only the coverage of the last call to Kernel#eval is kept.
This is not true. Coverage is stored in a global map of path -> coverage. Every attempt is made to use the same coverage for a given path. If you load or eval the same path multiple times, the same coverage will be reused. The bug is because `iseq_set_sequence` sets the coverage to zero for executable lines, for the lines it covers. So, it will: 1. Load the current shared coverage for a given path. 2. Based on the eval line number, update that coverage. 3. For each executable instruction, assign to the `coverage[instruction_line + eval_line_number]`. So, it won't keep the last result, it just resets some or all of the coverage, depending on how the `eval` call is invoked. In theory, the same problem can apply to `require` and `load`, but would be, I expect, a little more involved to reproduce. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104411 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different computation is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). Status changed from Open to Closed Merged in https://github.com/ruby/ruby/commit/7e0f5df2f99693267d61636d23da47f79924e9d5 ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104448 * Author: ioquatix (Samuel Williams) * Status: Closed * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh). Status changed from Closed to Open Hey, I am the maintainer of the coverage library. Please don't merge it until I say it's okay. I'm really feeling bad smell about merging coverage results automatically. Please give me time to understand what the current situation is and to consider whether it is really OK to merge the results. So, please let me revert the commit once. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104451 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). It was reverted in https://github.com/ruby/ruby/commit/4f4c1170bc988104f3bd3321558099af7ea19c18 Okay @mame, I look forward to your analysis. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104456 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh). @ioquatix When creating a ticket, could you write up a use case? Even if you believe it is a bug, it is not necessarily so to others. You only showed an example of evaling a method definition many times. As far as that case is concerned, I don't think that it is so important and frequent that we need to care in the coverage library. Please explain how the current behavior is troubling in a real-world example. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104460 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). Bake uses `module_eval` to load source files: https://github.com/ioquatix/bake/blob/c6b0d313a216077ceff460e2e90324206f3c24... If you load the same file several times during testing, coverage of previous tests is lost/cleared. Any code which uses a variant of this - e.g. `rack` https://github.com/rack/rack/blob/64610db2d2e7e910c4d3e17874dc316ed3eb227a/l... can suffer the same problem. It's common to load `config.ru` fresh each time you run a test, for example. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104476 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). Here is an example of the ERB test issue: https://github.com/ioquatix/covered/blob/main/examples/erb/test.rb I could imagine this could show up in any code which reloads ERB templates between tests. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104477 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh). Thanks. In your case of `ERB.new`, the `test` function does not actually use its arguments. Consider a more realistic example: ``` def test(value = nil) template = ERB.new(File.read(path).gsub("VALUE") { value || "default" }) template.result(binding) end ``` In this case, if we ever pass a multi-line string to `test`, automatic merging of the coverage results would bring trouble. I think it is safer to take only the coverage of the first or last version of code when trying to compile with the same pathname, instead of automatically merging them. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104478 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams).
Thanks. In your case of ERB.new, the test function does not actually use its arguments.
I'm not sure what you mean. Depending on the argument, different coverage is generated. Can you explain why you think "the test function does not actually use its arguments"?
In this case, if we ever pass a multi-line string to test, automatic merging of the coverage results would bring trouble.
``` ERB.new(File.read(path).gsub("VALUE") { value || "default" }) ``` I understand your concern, and I have never seen such a code in the real world. Can you share the real world risk? ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104479 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). By the way, did you realise, eval coverage is already merged, if it's a subset of the eval'ed code? So the behaviour is only broken if the eval'ed segments overlap, in which case the behaviour is, frankly, unexpected and pretty weird. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104480 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by jeremyevans0 (Jeremy Evans). Merging coverage results for eval would be inconsistent with how coverage for load is handled. example code: `t.rb`: ```ruby if $a p 1 else p 2 end ``` Print coverage result: ``` # Loaded Once $ ruby -r coverage -e 'Coverage.start; $a = true; load "t.rb"; p Coverage.result' 1 {"t.rb"=>[1, 1, nil, 0, nil]} # Loaded Twice, not merged $ ruby -r coverage -e 'Coverage.start; $a = true; load "t.rb"; $a = false; load "t.rb"; p Coverage.result' 1 2 {"t.rb"=>[1, 0, nil, 1, nil]} ``` I agree with @mame that if there is no way to ensure that the same code is evaluated in each evaluation, it is wrong to merge the results. I can understand why real world code would evaluate multiple times for the same file and line: ```ruby [Array, Hash].each do |klass| klass.class_eval(RUBY, __FILE__, __LINE__+1) if self == Hash def foo; :bar end else def foo; :baz end end RUBY end ``` This is a trivial example, but at least shows why you would want to do it. In this example, it would be easy to separate the example code per class, but that is probably non-trivial and a bad idea for more complex examples. I'm not necessarily opposed to merging results, but as it is not possible to ensure result merging makes sense, it should be an option and not the default behavior (assuming that @mame is open to supporting it as an option). ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104481 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams). @jeremyevans0 I'm not proposing any change to `load`, only to `eval` and related methods. However, I actually believe your example is equally confusing because the coverage is not merged. If someone loads the same file several times, it's reasonable to expect coverage to be merged (and probably unexpected that it's not).
I agree with @mame (Yusuke Endoh) that if there is no way to ensure that the same code is evaluated in each evaluation, it is wrong to merge the results.
We have no way to disambiguate coverage, since by its nature, coverage refers to files and line offsets. In practice, After the program executes, coverage is displayed by loading those files and displaying information to the user. If those file paths and line numbers are not consistent, coverage will become meaningless. So if the user does not desire coverage, they should simply avoid setting the path and line number. This kind of consistency is also important for the Ruby debugger, so this is not something specific to coverage.
I'm not necessarily opposed to merging results, but as it is not possible to ensure result merging makes sense, it should be an option
This is already the case: it already has an option, `Coverage.start(eval: true)`. When you select this mode, coverage is already merged. If you have several evals for the same file, they all get merged into one coverage report. What this bug is trying to address, is the fact that when those eval ranges overlap, they get reset. In every case I can think of, this is undesirable and basically not reporting coverage correctly, as lines which are previously executed, with coverage, get reset to 0. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104486 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh).
By the way, did you realise, eval coverage is already merged, if it's a subset of the eval'ed code?
Yes, I think that that is bad. I think I missed it because I didn't review https://github.com/ruby/ruby/pull/6396 enough. I want to limit it even from now. I think "coverage for eval" feature was a good idea for measuring the coverage of Rails view code, but I would like to design it to the minimum necessary for that purpose. Currently, I think it is too flexible than I expected. The design I am currently considering is as follows. * If there are loaded and eval'ed codes with the same path, only the loaded code is measured * If there are multiple codes eval'd with the same path, the first (or the last) eval'ed code is measured I need to take time to understand and organize the current situation and carefully consider how it should be designed. However, I am not likely to be able to spend sufficient time to the coverage library for a while, so please wait. BTW, I'm not keen on having `Coverage.start` have a new keyword argument for this purpose. Coverage library already has many different modes. I don't want to complicate things further. (This is mostly my fault, of course.) ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104488 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams).
I want to limit it even from now... If there are loaded and eval'ed codes with the same path, only the loaded code is measured
Is there some reason why you think we shouldn't measure the complete coverage of a file, including evaluated code? The point of code coverage is to provide feedback on whether every line of code is exercised by tests. Your definition of coverage does not seem to include "all lines of code executed". Why? ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-104496 * Author: ioquatix (Samuel Williams) * Status: Open * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by mame (Yusuke Endoh). Status changed from Open to Rejected Sorry, but I am not keen on this proposal. Let me reject this once. When I started developing the covearge library, I decided that its target is "source files that actually exist". The measured coverage is visually observed by a human, which requires source code, and Ruby interpreter did not keep the eval'ed source code (at that time). I think that the design decision to target only source files was natural. Because the Rails view code (.erb) exists as a file, I thought that it is the target of covearge library. So I accepted #19008. Beyond that, measuring coverage on dynamically generated code conflicts with the coverage library's initial design decision. To be honest, I don't want to further complicate the coverage library. Frankly, its API is already very complicated. I no longer want to expand it beyond the initial design decision. --- One constructive note. When I first started implementing the coverage library, I had no choice but to implement it as a built-in, but now we have TracePoint, which is well-designed and general-purpose than the coverage library. Also, Prism API is being developed, and it may be possible (although I think TracePoint needs to be extended) to radically improve the coverage measurement on a column-by-column basis rather than line-by-line basis. I don't have time right now, but I would like to rebuild the library as a pure external gem in future, i.e. `coverage2` or something, with dynamically generated code, etc. in mind from the beginning. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-106238 * Author: ioquatix (Samuel Williams) * Status: Rejected * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/

Issue #19857 has been updated by ioquatix (Samuel Williams).
When I started developing the covearge library, I decided that its target is "source files that actually exist".
I completely agree with this and I don't think that statement has any bearing on this proposal.
The measured coverage is visually observed by a human, which requires source code, and Ruby interpreter did not keep the eval'ed source code (at that time). I think that the design decision to target only source files was natural.
Makes sense.
Because the Rails view code (.erb) exists as a file, I thought that it is the target of covearge library. So I accepted #19008.
That's reasonable.
Beyond that, measuring coverage on dynamically generated code conflicts with the coverage library's initial design decision.
That's not what this PR is doing?
To be honest, I don't want to further complicate the coverage library. Frankly, its API is already very complicated. I no longer want to expand it beyond the initial design decision.
This PR addresses a real world problem I ran into. I accept the example given isn't clear. However, I'd like to discuss this at developer meeting. ---------------------------------------- Bug #19857: Eval coverage is reset after each `eval`. https://bugs.ruby-lang.org/issues/19857#change-106243 * Author: ioquatix (Samuel Williams) * Status: Rejected * Priority: Normal * Assignee: ioquatix (Samuel Williams) * Backport: 3.0: DONTNEED, 3.1: DONTNEED, 3.2: REQUIRED ---------------------------------------- It seems like `eval` based coverage is reset every time eval is invoked. ```ruby #!/usr/bin/env ruby require 'coverage' def measure(flag) c = Class.new c.class_eval(<<~RUBY, "foo.rb", 1) def foo(flag) if flag puts "foo" else puts "bar" end end RUBY return c.new.foo(flag) end Coverage.start(lines: true, eval: true) # Depending on the order of these two operations, different coverage is calculated, because the evaluation of the code is considered different, even if the content/path is the same. measure(false) measure(true) p Coverage.result ``` Further investigation is required. -- https://bugs.ruby-lang.org/
participants (3)
-
ioquatix (Samuel Williams)
-
jeremyevans0 (Jeremy Evans)
-
mame (Yusuke Endoh)