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

Closed
agraves opened this Issue Jul 10, 2012 · 6 comments

Comments

Projects
None yet
4 participants

agraves commented Jul 10, 2012

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.

Owner

myronmarston commented Jul 10, 2012

You argue this well :). My two cents:

  • Elevating test support files to a top-level directory of the project root feels wrong to me. It elevates the support files to a central place that I don't believe they should have.
  • I don't think we'll ever be able to make the pattern spec/**/*, no matter how we organize the spec and support files. Rubinius compiles ruby files to .rbc bytecode files next to the original files. Obviously, we can't try to load these files. The best we could do is spec/**/*.rb, and while a 2-letter suffix is less likely to be fat-fingered than the current default, the same problem remains: if you typo the file name as something.br, you'll be able to run it as a single file, but the rake task won't pick it up.
  • Ultimately, I would only want to make a change like this with broad community support. I have yet to see broad support for this.
Owner

dchelimsky commented Jul 10, 2012

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.

Contributor

kaiwren commented Aug 22, 2012

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?

Owner

dchelimsky commented Aug 22, 2012

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.

Owner

myronmarston commented Aug 22, 2012

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?

Owner

myronmarston commented Dec 24, 2012

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment