Issue #13901 has been updated by marcandre (Marc-Andre Lafortune).


mame (Yusuke Endoh) wrote:
> `obj.foo.foo` has two NODE_CALLs.  It is difficult to distinguish them by only the beginning, unless we have other information about the node in question.  Honestly, I'm unsure if we can distinguish all relevant NODEs by only a pair of the beginning and the end.

It depends what you consider the beginning position, but if you return the beginning of the method sending (4 vs 8 in your example, or 5 vs 9), then it's easy to distinguish between the two NODE_CALLs. These correspond to `dot`, or `selector` range in `parser` (try `ruby-parser -L -e "obj.foo.foo"` :-)

If you only provide only the beginning of the expression (1 vs 1), then yes, the end position is needed in that case.

> > For example, deep-cpver will color `0&.nonzero?&.foo(1, 2, 3).nil?` all in green, but "foo(1, 2, 3)" in red (well, the (), are gray, actually)
> 
> This is what I'm now aiming (Ko1 called it "callsite coverage").  Great.

Cool :-)

> > A minor issue is that branch coverage will not solve cases of exceptions being raised. Check the last line (char coverage) of:
> > 
> > $ deep-cover -e "puts(:a, notdefined, :c) rescue nil"
> > 
> > The first argument (:a) is in green, the last one (:c) in red, because `notdefined` raised a NoMethodError. Is there a plan to implement such detailed coverage?
> 
> I had no thought of counting trivial evaluation such as a literal, but if your pure-Ruby approach does not cause performance issue, now I'd like to consider supporting it.  It would be difficult (for me) to implement all in 2.5, though.

Just to be clear: literals (or anything that can not change control flow) do not cause *any performance loss at all* for us.

We always deduce their execution from normal control flow. E.g. in `var = [1, 2, 3]`, to know if `3` was executed, we check if it's previous sibbling (`2`) was executed. To know that, we check if `1` was executed. `1` has no previous sibbling, so we check the parent `[]`. It itself checks the parent `a = `, which finally asks bring us to our parent root. Only this parent introduces a small performance hit with a counter `$global[][] += 1`. So we get the execution count of the literals, the array creation and the variable assignment for free.

To summarize our strategy: a Node is not responsible to know how many times it was entered by control flow, that is the responsibility of the parent node. A Node's responsibility is to know how many times control flow exited it normally. For many nodes like literals, exit = entry so there's nothing special to be done. A "send" node must add a $tracker+=1 after the call to know that. I need to credit Maxime Lapointe for pointing us in that direction early on.


----------------------------------------
Feature #13901: Add branch coverage 
https://bugs.ruby-lang.org/issues/13901#change-67684

* Author: mame (Yusuke Endoh)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
I plan to add "branch coverage" (and "method coverage") as new target types of coverage.so, the coverage measurement library.  I'd like to introduce this feature for Ruby 2.5.0.  Let me to hear your opinions.

## Basic Usage of the Coverage API

The sequence is the same as the current: (1) require "coverage.so", (2) start coverage measurement by `Coverage.start`, (3) load a program being measured (typically, a test runner program), and (4) get the result by `Coverage.result`.

When you pass to `Coverage.start` with keyword argument "`branches: true`", branch coverage measurement is enabled.

test.rb

~~~
require "coverage"
Coverage.start(lines: true, branches: true)
load "target.rb"
p Coverage.result
~~~

target.rb

~~~
1: if 1 == 0
2:   p :match
3: else
4:   p :not_match
5: end
~~~

By measuring coverage of target.rb, the result will be output (manually formatted):

~~~
$ ruby test.rb
:not_match
{".../target.rb" => {
  :lines => [1, 0, nil, 1, nil],
  :branches => {
    [:if, 0, 1] => { [:then, 1, 2] => 0, [:else, 2, 4] => 1 }
  }
}
~~~

`[:if, 0, 1]` reads "if branch at Line 1", and `[:then, 1, 2]` reads "then clause at Line 2".  So, `[:if,0,1] => { [:then,1,2]=>0, [:else,2,4]=>0 }` reads "the branch from Line 1 to Line 2 has never executed, and the branch from Line 1 to Line 4 has executed once."

The second number (`0` of `[:if, 0, 1]`) is a unique ID to avoid conflict, just in case where multiple branches are written in one line.  This format of a key is discussed in "Key format" section.

## Why needed

Traditional coverage (line coverage) misses a branch in one line.  Branch coverage is useful to find such untested code.  See the following example.

target.rb

~~~
p(:foo) unless 1 == 0
p(1 == 0 ? :foo : :bar)
~~~

The result is:

~~~
{".../target.rb" => {
  :lines => [1, 1],
  :branches => {
    [:unless, 0, 1] => { [:else, 1, 1] => 0, [:then, 2, 1] => 1 },
    [:if, 3, 2] => { [:then, 4, 2] => 0, [:else, 5, 2] => 1 }
  }
}}
~~~

Line coverage tells coverage 100%, but branch coverage shows that the `unless` statement of the first line has never taken true and that the ternary operator has never taken true.

## Current status

I've already committed the feature in trunk as an experimental feature.  To enable the feature, you need to set the environment variable `COVERAGE_EXPERIMENTAL_MODE` = `true`.  I plan to activate this feature by default by Ruby 2.5 release, if there is no big problem.

## Key format

The current implementation uses `[<label>, <unique ID>, <lineno>]`, like `[:if, 0, 1]`, to represent the site of branch.  `<unique ID>` is required for the case where multiple branches are in one line.

I think this format is arguable.  I thought of some other candidates:

* `[<label>, <lineno>, <column-no>]`: A big problem, how should we handle TAB character?
* `[<label>, <offset from file head>]`: Looks good for machine readability.
* Are `<label>` and `<lineno>` needed? They are useful for human, but normally, this result will be processed by a visualization script (such as SimpleCov).

What do you think?  I'm now thinking that `[<label>, <lineno>, <offset from file head>]` is reasonable, but it is hard for me to implement.  I'll try later but parse.y is so difficult... (A patch is welcome!)

## Compatibility

This API is 100% compatible.  If no keyword argument is given, the result will be old format, i.e., a hash from filename to an array that represents line coverage.

~~~
# compatiblie mode
Coverage.start
load "target.rb"
p Coverage.result
#=> {".../target.rb" => [1, 1, 1, ...] }

# If "lines: true" is given, the result format differs slightly
Coverage.start(lines: true)
load "target.rb"
p Coverage.result
#=> {".../target.rb" => { :lines => [1, 1, 1, ...] } }
~~~

## Method coverage

Method coverage is also supported.  You can measure it by using `Coverage.start(methods: true)`.

target.rb

~~~
 1: def foo
 2: end
 3: def bar
 4: end
 5: def baz
 6: end
 7:
 8: foo
 9: foo
10: bar
~~~

result (manually formatted)

~~~
{".../target.rb"=> {
  :methods => {
    [:foo, 0, 1] => 2,
    [:bar, 1, 3] => 1,
    [:baz, 2, 5] => 0,
  }
}}
~~~

## Notes

* `if` statements whose condtion is literal, such as `if true` and `if false`, are not considered as a branch.

* `while`, `until`, and `case` are also supported.  See Examples 2 and 3.

* This proposal is based on #9508.  The proposal has [some spec-level issues](https://github.com/ruby/ruby/pull/511#issuecomment-328753499), but the work was really inspiring me.

## Future work

* Someone may want to know how many times an one-line block is executed, such as `n.times {  }`.

* Someone may want to know how many times each method call is executed, such as `obj.foo.bar.baz` (For example, if method `foo` always raises an exception, calls to `bar` and `baz` are not executed.)

## Some examples

### Example 1

target.rb

~~~
1: if 1 == 0
2:   p :match1
3: elsif 1 == 0
4:   p :match2
5: else
6:   p :not_match
7: end
~~~

result (manually formatted)

~~~
{"target.rb" => {
  :lines => [1, 0, 1, 0, nil, 1, nil],
  :branches => {
    [:if, 1] => { [:then, 2] => 0, [:else, 3] => 1 },
    [:if, 3] => { [:then, 4] => 0, [:else, 5] => 1 },
  }
}
~~~

### Example 2

target.rb

~~~
 1:case :BOO
 2:when :foo then p :foo
 3:when :bar then p :bar
 4:when :baz then p :baz
 5:else p :other
 6:end
 7:
 8:x = 3
 9:case
10:when x == 0 then p :foo
11:when x == 1 then p :bar
12:when x == 2 then p :baz
13:else p :other
14:end
~~~

result (manually formatted)

~~~
{"target.rb" => {
  :lines => [1, 0, 0, 0, 1, nil, nil, 1, 1, 1, 1, 1, 1, nil],
  :branches => {
    [:case, 1] => {
      [:when, 2] => 0,
      [:when, 3] => 0,
      [:when, 4] => 0,
      [:else, 5] => 1
    },
    [:case, 8] => {
      [:when, 9] => 0,
      [:when, 10] => 0,
      [:when, 11] => 0,
      [:else, 12] => 1
    }
  }
}
~~~

### Example 3

target.rb

~~~
 1:n = 0
 2:while n < 100
 3:  n += 1
 4:end
~~~

result (manually formatted)

~~~
{"target.rb" => {
  :lines => [1, 101, 100, nil],
  :branches => {
    [:while, 2] => {
      [:body, 3] => 100,
      [:end, 5] => 1
    }
  }
}
~~~



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>