Add specs for Coverage.supported?
#1039
Conversation
AI-Mozi
force-pushed
the
add_specs_for_coverage_supported
branch
2 times, most recently
from
58b7876
to
ec86da2
Compare
| Coverage.supported?(:foo).should == false | ||
| Coverage.supported?(:bar).should == false | ||
| end | ||
| end |
There was a problem hiding this comment.
minor: It's kind of feature discovering mechanism so it doesn't make sense to require in specs some specific result (true or false).
IMHO the only thing that could be checked is that this method:
- returns
trueorfalse(for existing modes and arbitrary Symbols) - converts its argument to Symbol
There was a problem hiding this comment.
Does it converts argument to Symbol? Isn't it raising an error?
There was a problem hiding this comment.
TBH I haven't checked. I meant handling of not Symbol arguments in general.
AI-Mozi
force-pushed
the
add_specs_for_coverage_supported
branch
from
ec86da2
to
a8b0cef
Compare
| Coverage.supported?(:methods).should == true | ||
| Coverage.supported?(:eval).should == true | ||
| Coverage.supported?(:all).should == false | ||
| end |
There was a problem hiding this comment.
I meant something like this:
[true, false].should.include?(Coverage.supported?(:lines))or
(Coverage.supported?(:lines) == true || Coverage.supported?(:lines) == false).should == trueThere was a problem hiding this comment.
For arbitrary Symbols that are not lines, branches, methods nor eval it should always return false?
AI-Mozi
force-pushed
the
add_specs_for_coverage_supported
branch
2 times, most recently
from
335b794
to
fade356
Compare
library/coverage/supported_spec.rb
Outdated
| [true, false].should.include?(Coverage.supported?(:branches)) | ||
| [true, false].should.include?(Coverage.supported?(:methods)) | ||
| [true, false].should.include?(Coverage.supported?(:eval)) | ||
| Coverage.supported?(:all).should == false |
There was a problem hiding this comment.
suggestion: I would move the last case into a separate it, e.g. it returns false for not existing modes, and use obviously not existing modes like :foo, :bar etc. :all is too meaningful and looks like correct mode name.
AI-Mozi
force-pushed
the
add_specs_for_coverage_supported
branch
from
fade356
to
e6a4441
Compare
|
Thank you! |
#1016
[Feature #19026]