Fix regression in rails filters #618
Conversation
|
👋 Thanks! Could we add a "simple" test case that checks this maybe in the integration test cases so something like this doesn't happen again? :) |
|
Sure thing 👍 |
jenseng
force-pushed
the
fix-rails-profile
branch
from
1e11efa
to
c66fa18
Compare
|
This approach to testing is a little bit meh... ideally |
|
Oh, heh I think I glossed over your comment around feature tests, perhaps I'll just test it there |
In converting this to a regex in a0b14a1, an extra leading `/` was added, making this filter less useful (it would literally match the `/^/db/` path).
jenseng
force-pushed
the
fix-rails-profile
branch
from
c66fa18
to
82f5873
Compare
|
Upon further reflection, I think I found a decent way to test it at a lower level 🤞 |
| @@ -155,7 +155,9 @@ def matches?(_) | |||
| end | |||
| end | |||
|
|
|||
| context "with the default profile" do | |||
| context "with the default configuration" do | |||
| skip "requires the default configuration" if ENV["SIMPLECOV_NO_DEFAULTS"] | |||
There was a problem hiding this comment.
Slightly confused why this is here and not just above? Leftover? :)
There was a problem hiding this comment.
I'm not sure if there's a scenario where you'd run the specs w/ SIMPLECOV_NO_DEFAULTS, but this should ensure the suite still passes in that case, since it has to do with the default filters. Might be worth setting up in Travis?
|
|
||
| if SimpleCov.usable? | ||
| describe SimpleCov do | ||
| skip "requires the default configuration" if ENV["SIMPLECOV_NO_DEFAULTS"] |
There was a problem hiding this comment.
(with other point where it is I mean this here) :)
There was a problem hiding this comment.
Same story here, I just wanted to make this resilient in case people run specs with that set, since the profiles won't exist in that case.
| configure(&SimpleCov.profiles[name.to_sym]) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
👍 using let with anonymous classes! :)
| it "provides a sensible rails profile" do | ||
| config.load_profile(:rails) | ||
| expect(filtered?(config, "app/models/user.rb")).not_to be | ||
| expect(filtered?(config, "db/schema.rb")).to be |
There was a problem hiding this comment.
wait whaaaaatttttttttt? You can just use .to be/.not_to be. Touché. I've used rspec forever and consider myself pretty good at it bu you constantly teach me new stuff. Thanks 🙏
| config.load_profile(:rails) | ||
| expect(filtered?(config, "app/models/user.rb")).not_to be | ||
| expect(filtered?(config, "db/schema.rb")).to be | ||
| expect(filtered?(config, "config/environment.rb")).to be |
There was a problem hiding this comment.
👍 great way of testing it on a lower level, thank you very much! 👍
|
Thanks a bunch! 🚀 |
0.15.1 (2017-09-11) ======= ## Bugfixes * Filter directories outside SimpleCov.root that have it as a prefix. See [#617](simplecov-ruby/simplecov#617) (thanks @jenseng) * Fix standard rails profile rails filter (didn't work). See [#618](simplecov-ruby/simplecov#618) (thanks @jenseng again!)
In converting this to a regex in a0b14a1, an extra leading
/was added,making this filter less useful (it would literally match the
/^/db/path).