Multiple Guard::RSpec instances don't run all files #74

Closed
ashmoran opened this Issue Nov 27, 2011 · 7 comments

Comments

Projects
None yet
3 participants
Contributor

ashmoran commented Nov 27, 2011

I was trying to configure multiple groups as described in the Running a subset of all specs section of the readme, but I found it was only loading files from the last group defined. Here are the guard "rspec" blocks I was using:

guard "rspec", spec_paths: %w[ spec/decorators spec/presenters ], cli: "--color --format Fuubar" do
  watch('spec/spec_helper.rb')                              { %w[ spec/decorators spec/presenters ] }
  watch('spec/environments/decorators.rb')                  { "spec/decorators" }
  watch('spec/environments/presenters.rb')                  { "spec/presenters" }
  watch(%r{^spec/support/(.+)\.rb$})                        { %w[ spec/decorators spec/presenters ] }

  watch(%r{^spec/(?:decorators|presenters)/.+_spec\.rb$})
  watch(%r{^app/(?:decorators|presenters)/(.+)\.rb$})       { |m| "spec/#{m[1]}_spec.rb" }
end

guard "rspec", spec_paths: %w[ spec/models ], cli: "--color --format Fuubar" do
  watch('spec/spec_helper.rb')          { "spec/models" }
  watch('spec/environments/mongoid.rb') { "spec/models" }
  watch(%r{^spec/support/(.+)\.rb$})    { "spec/models" }

  watch(%r{^spec/models/.+_spec\.rb$})
  watch(%r{^app/models/(.+)\.rb$})      { |m| "spec/#{m[1]}_spec.rb" }
  watch(%r{^app/models/concerns.rb$})   { |m| "spec/models" }
end

I didn't determine what exact circumstances this did and didn't behave correctly. I looked in the code and figured out that the root cause was the Inspector and Runner objects being singletons (everything being defined on a Module).

I've refactored the code to use instantiable classes, and this appears to solve the problem without introducing any side-effects. I'll send a pull request in a second for you to look over, I had to do a few odd things to make the specs pass.

Owner

rymai commented Nov 27, 2011

Hi, cool idea!

However it appears that you didn't read the README correctly, you actually have to use the #group DSL method if you want to define multiple RSpec guards, like this:

group 'acceptance-tests' do
  guard 'rspec', :spec_paths => ['spec/acceptance'] do
    # ...
  end
end

group 'unit-tests' do
  guard 'rspec', :spec_paths => ['spec/models', 'spec/controllers', 'spec/routing'] do
    # ...
  end
end
Contributor

ashmoran commented Nov 27, 2011

Hi rymai

Sorry for posting an ambiguous issue description - I actually tried what you describe first. As it didn't solve the problem, I went off and made the change in guard-rspec. When I came back to my app, I found the group blocks weren't needed, so I removed them, and that's what I ended up posting.

Maybe it behaves differently with group? If so, I was still triggering an edge case that meant it was only running the last set of files for me. Hopefully this fixes the problem in all cases.

Cheers
Ash

Owner

rymai commented Nov 27, 2011

No problem @ashmoran, I personnally don't use several RSpec guards, so I don't really know if it works or not, but I trust you when you say that I doesn't work! ;)

The pull-request looks very cool by the way!

Contributor

ashmoran commented Nov 27, 2011

This is also my bad - I thought about writing a spec to demonstrate the problem I was having, but I couldn't see an easy way to drop one in… then I started browsing the code and forgot about it :) I was 99% sure it was being caused by instance variables getting overwritten so I went in and changed it.

Hopefully on the basis it doesn't break anything else it's still good to merge in…

Owner

thibaudgg commented Dec 25, 2011

I think we can't close this issue right?

Contributor

ashmoran commented Dec 26, 2011

By all means, I'm using a released gem version with no problems now. (I assume you meant "can" rather than "can't"!)

Owner

thibaudgg commented Dec 26, 2011

Yep, you right I meant "can"!

thibaudgg closed this Dec 26, 2011

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