<eregon>
donV: I think once the require is placed on top we're good to merge :)
<donV>
done
<eregon>
please squash all commits in one
<donV>
ok
<donV>
eregon: done
<eregon>
donV: So why only line 1 is reported (3 times) when Coverage.result is called later?
<eregon>
is there some sort of implicit context to Coverage.start?
<donV>
No lines are reported when we load/require the file that starts coverage.
<donV>
@config_file => [] <== empty array
<donV>
JRuby actually did report something here, but it has been corrected.
<eregon>
right, only further load/require start counting?
<donV>
yes.
<eregon>
But then what hapenned when start is called twice as in the first example?
<donV>
It was unexpected for me, but we follow MRI now.
<donV>
Second start is a noop.
<eregon>
then in coverage result there should be 1,1,1,3,3,3, no?
<donV>
for start_coverage.rb there are only 3 lines, so the array should never be longer than 3 entries.
<eregon>
ah, right, the array is the lines with index-1 and execution counts
<donV>
eregon: not sure what you are asking now :)
<donV>
yes, exactly!
<donV>
empty array has some special meaning that the file was seen but not covered
<eregon>
I think we should separate the case of "second start is noop", so there is a simple and easy basic case to follow :)
<donV>
“0” in the array mean loaded but not run.
<donV>
eregon: agreed
<donV>
“nil” in the array means “not runnable code” like “end” and comments.
<eregon>
all these semantics would be awesome if they can be expressed as spec documentation
<donV>
eregon: I’ll add a basic example, and change the first example to NOOP case
<eregon>
but maybe let's merge this one and improve on it? as you prefer
<donV>
I’d prefer to merge and then improve :)
<eregon>
ok :)
<eregon>
merged!
enebo has joined #jruby
<donV>
enebo: Hi!
<enebo>
donV: hello
<donV>
enebo: Got the first coverage spec into ruby/spec!
<enebo>
donV: and you saw that we were not failing with your tests?
<donV>
enebo: Yes!
<enebo>
donV: or at least not the three specs you made yesterday?
<enebo>
ok
<donV>
enebo: Working on expanding it now.
<donV>
enebo: After Coverage.result is called, result still returns entries for all seen files with empty arrays?
<enebo>
donV: yeah if they have been covered at all then they become []
<enebo>
donV: if not then they stick around with full [-1, 0…]
<donV>
This is in no way mentioned in the doc. What is the point of this?
<donV>
It makes all spec depend on all earlier specs
<enebo>
donV: I don’t have any idea. I guess they are assuming a new session is not interested in old results
<donV>
Yes, but I would expect files from previous start..result sessions to be wiped from the next result.
<enebo>
yeah
<donV>
…I think I am missing something, but I don’t know what :)
<eregon>
so result always accumulate?
<enebo>
I was pretty surprised by this behavior as well
<enebo>
eregon: the hash never loses keys even if reset
<donV>
Line counts do not accumulate, but the keys (the list of files) do.
<donV>
exactly
<enebo>
eregon: but values arrays will change based on whether a reset changes
<eregon>
reset = start?
<donV>
reset = result
<enebo>
ah yeah I means result
<enebo>
meant
<eregon>
this is definitely strange indeed
<enebo>
I guess they realized some people may not want this so peek_result was added
<donV>
eregon: does the fixture helper method load the file?
<eregon>
maybe worth a bug report if we can show the behavior to just strange
<enebo>
I don’t get it but it seems intentional but it is weird
<eregon>
donV: no it just expands to the file path
<donV>
ok
<enebo>
and not many people really consume this library directly. Only a handful of coverage libraries
<enebo>
well I know know of simplecov but I expect it is not the lone consumer of it :)
<eregon>
hopefully :)
<eregon>
yeah it's pretty much like a CLI flag with some minimal API on top
<donV>
Argh! I cannot see a way to write independent specs! Keys are accumulating in the result hash :(
<donV>
enebo: Can you help me write a but for it? Never done it for Ruby (MRI) before.
<eregon>
donV: one way to deal with global state is for instance to just test coverage for a single file (key of the hash)
<eregon>
or using subprocesses if it's not enough, but that's heavy
<donV>
eregon: I can also filter the hash for entries with empty arrays.
yfeldblum has joined #jruby
<enebo>
donV: I think if your first spec run might look at actual hash and all other specs are just examining a single value from the hash then this buildup of keys is not that important
<eregon>
right but then you expect other entries to always be empty arrays
<enebo>
donV: certainly one spec is seeing if an entry becomes an empty array too
<eregon>
exactly
<donV>
enebo: Exactly!
<donV>
This weakens the test a lot.
<enebo>
Largely though I don’t see much of an issue past perhaps one spec and I have no idea on guaranteed execution order so that one spec could also be a full subspawn (which is costly for us)
<eregon>
it's better to test feature by feature than trying the ultimate integration test which will always fail :)
<donV>
I’d like to file this as a bug just to see what the retionale behind this is.
<eregon>
donV: do you have a gist to reproduce that weird behavior?
<enebo>
donV: it may even resolve to some documentation explaining it too
<enebo>
My guess is that someone did not fully think through the lifecycle of the library and then added peek_result to satisfy it’s lack of proper lifecycle
<eregon>
coverer - rage :)
<enebo>
but they did not want to mess with compatibility of original design
<eregon>
enebo: there is a feature bug for adding peek_result
tenderlove has joined #jruby
<enebo>
eregon: yeah it feels that way without even knowing
<enebo>
yeah I am surprised it was even questioned as being useful
<enebo>
original design seems fundamentally broken
<enebo>
I would never ever use result I don’t thikn
<enebo>
At least original result() method seems very specific that once you call it any files with coverage are just dead
<enebo>
what coverage tool would want that though?
<enebo>
peek_result is just as good but without that limitation
<donV>
tenderlove: Hi! You committed the patch for Coverage.peek_result, right?
<enebo>
yeah tenderlove … why do you hate coverage
<enebo>
heh ok…wrong day for me to be serious about anything
<enebo>
donV: thanks. Your work is appreciated
<enebo>
donV: we will hopefully have this library spec’d good enough for Truffle by the time they impl this one. So you will get a two-fer by making specs for it
<eregon>
enebo: truffle has some basic coverage support with the tooling stuff, but I think it's not fully functional yet
<chrisseaton>
eregon enebo: coverage works pretty well and is always enabled and low overhead, however at the moment it can't distinguish between lines with no code on them (nil) and lines with code on them that is never executed (0), we're fixing this though
<enebo>
chrisseaton: eregon: ah sorry I thought there was an open bug for impling this
<enebo>
nonetheless the specs will help you be compliant
<chrisseaton>
oh and we have some issues with file lines if you specify an offset in an eval or something like that
<enebo>
chrisseaton: all newline marked nodes (well one per distinct line) which is not from an eval will be a 0 and all others will be -1)
<enebo>
chrisseaton: but evals are not covered in coverage at all
<chrisseaton>
oh right
<chrisseaton>
this is what we need the specs for
<enebo>
chrisseaton: I also added a flag on rootnode for this I think ‘needsCoverage'
<enebo>
chrisseaton: although that is a rather statically made thing