Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Adding a file with _spec on the end to spec/support ends up loading the spec/support files twice #623

Closed
DArrigoni opened this Issue Oct 17, 2012 · 9 comments

Comments

Projects
None yet
5 participants

Steps to Reproduce:

rails new rspec-test -T
cd rspec-test
echo "gem 'rspec-rails'" >> Gemfile
bundle install
rails g rspec:install
rails g model thing thing:string
mkdir -p spec/support
echo "puts 'foobar'" > spec/support/redundant_spec.rb
rspec

Expected Result:

foobar to be printed out once

Actual Result:

foorbar printed out twice

Work Arround:

Rename file to not include _spec

Notes:
This is a trivial example but the original issue I had was a shared_example being declared in the _spec file, which got loaded twice and failed out my specs.

This might be rspec-core, but I'm not sure.

Owner

myronmarston commented Oct 17, 2012

RSpec uses pattern to find files to load, which defaults to "spec/**/*_spec.rb". Files ending in _spec.rb in your spec/support folder match this.

The generated spec_helper.rb file loads all the files in spec/support. Thus, RSpec itself is loading the file when it loads all the specs files, and your spec_helper.rb is also loading it.

RSpec doesn't treat the support directory as special in any way -- it's just a convention that's arisen in the community, and at some point we started including the snippet in the generated spec_helper.rb to load all the files in there.

Given that we don't have any special casing logic for spec/support (and probably never will), and you named your file in a way that matched the pattern for spec files, this seems like the correct behavior, even though it caused you trouble.

Maybe we need to document it better, though. Got a suggestion for that?

Your explanation makes sense, thanks.

As for documentation, I'm not sure. Maybe a warning message could get raised if any of the files in the files loaded from the generated spec/support helper end in _spec?

"rspec has found _spec.rb files in the spec/support folder and may end up loading these twice" or something like that.

Dir[Rails.root.join("spec/support/*/.rb")].each do |f|
require f
warn '_spec file detected in support, which may cause double loading of resources' if f.name =~ /_spec/
end

Something like that maybe? Might be overkill

Owner

myronmarston commented Oct 17, 2012

That's a pretty good idea. Given that users can change their spec file pattern, it should probably check if the name matches that...maybe something like:

RSpec.configuration.files_to_load.include?(f)

I also like that this solution would go in the generated spec_helper file, and not in RSpec itself (since RSpec itself doesn't know anything about the spec/support convention).

I think both rspec-core and rspec-rails can generate a spec_helper file, so maybe this should go in both?

@dchelimsky / @alindeman / @patmaddox -- any thoughts here?

Contributor

alindeman commented Oct 20, 2012

I think it sounds reasonable, though I'm a bit concerned about performance on larger spec suites. I think files_to_run is currently an Array, so searching through it is O(n). If we can make it a Set and maybe run a few benchmarks that show negligible cost, I'm probably in favor.

Member

samphippen commented May 25, 2013

@myronmarston @alindeman if we make the files to run a Set anyway, there won't be any duplicates. That way regardless of the file glob we'll only run each file once. I could make that change if you like?

@DArrigoni what do you think of that?

Owner

dchelimsky commented May 25, 2013

The issue is loading support files twice, and they are not part of files_to_run, so I don't think turning that into a set makes a difference and adds the complication that a set is unordered, so we'd have to do any ordering after the set is established.

In terms of checking each support file against the pattern, I don't think the list of files ever gets terribly long.

Owner

dchelimsky commented May 25, 2013

Previous comment not withstanding, I think this should be treated as a documentation issue. I think it's useful for users to understand that rspec is treating files that match a pattern as "spec files" and that you shouldn't use that pattern for support files.

Member

samphippen commented May 25, 2013

Do you think a comment in the generated spec_helper and something in the read me is good enough?

On 25 May 2013, at 17:26, David Chelimsky notifications@github.com wrote:

Previous comment not withstanding, I think this should be treated as a documentation issue. I think it's useful for users to understand that rspec is treating files that match a pattern as "spec files" and that you shouldn't use that pattern for support files.


Reply to this email directly or view it on GitHub.

Owner

dchelimsky commented May 28, 2013

For anybody following along: yes, I think comments in the README and generated files are sufficient. I made some comments on those comments in #754.

@samphippen samphippen closed this in #754 Jun 3, 2013

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