-
-
Notifications
You must be signed in to change notification settings - Fork 753
Add silence_filter_announcements config option #2007
Conversation
|
||
context "with a filter but with silence_filter_announcements" do | ||
it "does not announce" do | ||
allow(configuration).to receive(:silence_filter_announcements?) { true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to see this set configuration.silence_filter_announcements = true
. Stubbing it doesn't have any advantages over setting it as far as I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thought. I just made the change. I did it as a new commit to show the progression but feel free to squash it into the first one later on.
Personally I don't think this adds any value, if you don't like the default filters remove them... |
I can understand that perspective for sure, but the value I see is having a consistent filter that you know exists (e.g. the Adding this config option with a default falsey value will preserve the default behavior so existing users won't affected. |
This is simple enough (and looks like it'll have such a low maintenance cost) that I'll be happy to merge this. You'll need to get the build green first, though. Let us know if you need help. Thanks for doing this! |
c.silence_filter_announcements = true | ||
end | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theres an excess line here that I think is tripping a build rule, it's either that or you have spaces on the ends of your lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just removed the extra line.
I did some fixes, rebased and squashed commits. (If you want to see the commit versions that go along with the progression of comments, I kept them recorded in another branch). |
By default, RSpec will print a message before your specs run indicating what | ||
filters are configured, for instance, it might print | ||
"Run options: include {:focus=>true}" if you set `config.filter_run_including | ||
:focus => true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Relish will insert line breaks at the same places you have it here which won't look good. We're moving in the direction of having each paragraph be one lone unwrapped line so that HTML flow can wrap it. That looks much better. Can you change this to one long line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I just modified my commit to combine the lines here and for the second sentence below as well.
I just realized that the |
Add silence_filter_announcements config option
Thanks @draffensperger |
Add silence_filter_announcements config option
[skip ci]
We run specs with a couple of filters, but when I run them at the command like, I'd like to avoid cluttering the output with filter announcements like "Run options: include {:focus=>true}".
This add a new config option
silence_filter_announcements
(defaultfalse
) which allows you to prevent the filter announcements from being printed when you don't want them to be.This fixes this issue: #1896 and would resolve this Stack Overflow question: http://stackoverflow.com/questions/27698184/how-to-suppress-rspec-output-run-options-include-focus-true (as of now, 8 upvotes and 142 views)