rspec / rspec-core

RSpec runner and formatters
http://rspec.info
MIT License
1.22k stars 763 forks source link

Use a directory instead of a suffix to distinguish specs #644

Closed agraves closed 11 years ago

agraves commented 12 years ago

I'd like to briefly preface this with a few disclaimers:

  1. This is a feature request, and the change would have to be made responsibly and gradually.
  2. I use Rspec only within Rails, and I recognize that the needs of others differ from mine.
  3. No one (that I know of) has been killed by the status quo. I'm pursuing perfection, not adequacy.
  4. I don't care what any of these folders are named; it's just the folder vs suffix discussion that interests me.

The context:

At my company we typically TDD by repeatedly executing single tests via Rspec. We test deploys by running rake. We have, on three separate occasions, shipped with broken tests because we saved the files with typos in the filenames. I've been told by many to "make fewer typos" and "adjust your default --pattern to remove _spec." I've finished the latter and am working on the former, but I find myself wondering: what's the point of this default, anyways?

That said:

I propose Rspec by default distinguish specs from "other" files via a directory hierarchy rather than a "magic suffix." In other words, splitting the contents of spec/ into two (or more) subfolders. One folder would be automatically loaded as actual specs to be executed by Rspec (exactly as files suffixed with "_spec" are treated today). The other would not (exactly as files not suffixed with "_spec" are treated today).

Here are my main justifications:

  1. Organization. As it stands, fixture data, support files and actual tests are freely munged together inside spec. Actual specs are "special." They are named differently, loaded differently and treated differently. All of the arguments that Rails makes for separating helpers from controllers and so forth apply here.
  2. Simplicity. Rspec's role in the filesystem is then reduced to: load spec/foo. Users would be free to choose any naming scheme they like and organize fixtures / helpers how they see fit.
  3. One less opportunity to make mistakes. I see the _spec extension much like the human appendix*: harmless the vast majority of the time, serving little apparent purpose and really, really dangerous in a few edge cases. By switching to a folder, we can type less, have fewer opportunities for mistakes, simplify loading tests...at the expense of...forcing Rspec users to better organize their tests? That seems like a win-win.

I've already faced some opposition on this topic, and that's fine. Rspec should serve the best interests of its users on the whole, of course...but this default burned me, and I feel obligated to at least raise the question, regardless of the result.

Thank you for reading.


* I know some research indicates that the appendix may serve a purpose in maintaining your intestinal flora; humor me.

myronmarston commented 12 years ago

You argue this well :). My two cents:

dchelimsky commented 12 years ago

re: why this is the default - it was actually just a variation on the defaults in rake (@pattern = 'test/test*.rb') and in the Rails rake tasks (FileList['test/**/*_test.rb']). In other words you'll find exactly the same problem in any Rails project that does not use rspec, and many non-Rails projects as well.

There are some pretty strong arguments against changing this default. @patmaddox pointed out one with which I agree very strongly: there is a world of tooling built around this convention that would either be forced to change or simply break. The TextMate bundle which we manage, has a command to switch back and forth between a spec and its target code based on naming conventions. We could change this ourselves, in theory, but there are several other tools that we do not manage that support the same idea based on the same convention.

So I think changing the default is unlikely, but I am intrigued by the idea of a directory convention that could eventually be expressed as a configuration option.

Another possibility is to add a "strict-pattern" mode which would raise an error when you try to run an individual spec file that doesn't match whatever the pattern is set to. That would solve your initial problem without the downside of changing the default pattern.

kaiwren commented 12 years ago

Related issue: When running rake spec, it globs everything under the spec/ dir. This includes what's under spec/support/.

I wrote a shared spec that had a filename ending with _spec.rb as recommended in the Cuke docs at https://www.relishapp.com/rspec/rspec-core/docs/example-groups/shared-examples and the rake spec task loads it and tries to run it as a normal spec, causing an exception.

Removing the _spec in the filename of the shared spec file fixes the problem. Is the documentation correct and am I missing something, or does that documentation need to be updated?

dchelimsky commented 12 years ago

Is the documentation correct and am I missing something, or does that documentation need to be updated?

I think the doc needs to be updated. The examples on https://www.relishapp.com/rspec/rspec-core/docs/example-groups/shared-examples all have the shared examples in the same file as the groups that use them. I think it's easy to misconstrue that as a recommendation to name support files (that get included by others) with the same convention, but that is not the intent.

myronmarston commented 12 years ago

and the rake spec task loads it and tries to run it as a normal spec, causing an exception.

I'm surprised at this behavior and would consider it a bug in RSpec. While naming a support file containing shared examples "spec/support/foo_spec.rb" may cause it to be loaded when you don't want it to be, I don't think it should cause an error.

What error are you seeing?

myronmarston commented 11 years ago

Circling back around on this: I'm open to changing the changing the default spec pattern if there's broad community support for it, but I haven't seen any evidence of such support since this was opened.

I'm going to close this, but we can certainly re-open if/when there is more community desire to see this changed.