Consider making `rspec path/to/file_not_matching_pattern.rb` not run the named file

[myronmarston]

As discussed in #642, users occasionally run into an issue where they've got files that were written to be spec files but due to not matching the pattern, they are silently ignored by rspec or rspec spec, which can be quite frustrating. A couple users have requested we change the default pattern, but as discussed in that issue, that comes with its own set of issues and would be a hard sell to the community.

Thinking about that issue again, I think I've come up with an alternate solution that, IMO, would work much, much better, with none of the downsides of changing the default pattern.

It seems to me that the problem isn't so much that rspec spec may fail to find specific files but that rspec spec/path/to/file_not_matching_pattern.rb runs the named file as a spec file even though rspec spec would never run it. rspec path/to/file doesn't rely on the pattern and thus happily loads and runs the named file regardless of whether or not it matches the conventional pattern. When a user writes a single file and runs it in isolation in that fashion, it gives the impression that RSpec's going to run it when the entire suite runs, but currently RSpec provides no such guarantee. If, on the other hand, RSpec refused to run any files that didn't match the pattern, the user would immediately be made aware that their file doesn't match the pattern and would rename it to match. In fact, if we made this change, I think the only way a user could get into a situation where rspec spec silently ignores an intended spec file is if they never ran it at all...and if they're not even bothering to see a spec in their file fail and then pass...there's not much we can do to help.

Changing this in 3.x would potentially be a breaking change, so it should perhaps wait until RSpec 4. Also, there my be a legitimate use case for wanting to be able to have singleton files that are only runnable on demand when specifically named...but I can't think of it.

On second thought: if we did this, maybe we should just make it emit a loud warning at the end but still run the spec file. That would not be a breaking change (and thus could happen in 3.x) and would also support unorthodox usage patterns where users do want (for some odd reason) to be able to have files that are only runnable on demand.

Thoughts?

@agraves / @ruiyang -- would this solve the problems you reported in #642?

[agraves]

This is more consistent than the current behavior and would have prevented my issues. I support it.

I would advocate a garish message for the minor version and a refusal to run (perhaps without some kind of --force) in the major.

[ryang-bgl]

Yes, that would prevent this issue from happening. I really like your out of box thinking of this issue @myronmarston I will vote for not running the test. Also give meaningful exception message, let it break.

[cupakromer]

if we did this, maybe we should just make it emit a loud warning at the end but still run the spec file. That would not be a breaking change

:+1: This route will also give the community time to provide feedback on the potential behavior change.

[JoshCheek]

@cupakromer When did your hair stop being blue?! ^_^

Also, I like this idea a lot. Just saw a student hit this yesterday.

Independent, but somewhat related: it would be nice if I had the ability to manipulate multiple patterns (where "manipulate" means both "add" and "remove") It took a certain amount of right-brain activity to conceive of this pattern to load both RSpec and Minitest files (it expands to "**{,/*/**}/{*_spec,*_test,test_*}.rb").

[myronmarston]

I'd be on board with changing RSpec to support an array of patterns instead of just one pattern, provided it can be done in a backwards-compatible way.

[Startouf]

I ran into this pitfall more than once, and it still happens to me occasionally that I forget the _spec suffix, and have some surprises when I realize a few months later that some specs had never been ran on the CI and were actually failing, and for a good reason...

It may be legit to have spec files not matching the pattern, for example to run performance/speed tests that we might only want to run once in a while in a controlled environment (or any kind of very very expensive tests that we might want to run only run once in a while, for instance with production-like data)

I have no preference regarding the warning VS raising an error, but I believe the warning will be easier to be accepted by the community for 3.x

[JoshCheek]

It may be legit to have spec files not matching the pattern, for example to run performance/speed tests that we might only want to run once in a while in a controlled environment (or any kind of very very expensive tests that we might want to run only run once in a while, for instance with production-like data)

I think the expected way to do that is to use a tag to control whether they run or not.

---

Another possibility is that you can configure it with a manifest of files that you know aren't tests, eg spec/spec_helper.rb and spec/fixtures/*, and it will fail if there are tests in spec that aren't caught by the pattern and aren't in the manifest of exceptions. Just a right-brained thought that I haven't thought through very much, could be a good idea or a bad one, might fit nicely or poorly with what exists, but it's a different enough approach that I felt like it was worth saying, at least. (a followup equally unthought idea: RSpec could maintain that manifest for you, the first time it sees new files in the spec dir, it prompts you, and then adds them to zspec/manifest.json at a key of "is_test" or "is_not_test", or something) Not a suggestion, just a thought I spend a few min exploring!