grosser / rspec-instafail

Show failing specs instantly
MIT License
283 stars 34 forks source link

use LoadErrors to detect RSpec version #2

Closed tooky closed 13 years ago

tooky commented 13 years ago

I had a problem when running cucumber that the Spec constant appeared to be defined even though I was using RSpec 2, and therefore getting a LoadError when trying to load spec/runner/formatter/progress_bar_formatter.

This pull request uses LoadErrors to determine which rspec is loaded. It defaults to 2, and falls back to 1 if there is a LoadError.

Cheers, /tooky

grosser commented 13 years ago

I think the idea is good, but solution could be made cleaner. Rspec2 also defined Spec, to catch errors I suppose, so it seems to me that the check only needs to be reversed to if not defined? RSpec

grosser commented 13 years ago

hmm just gave it a try and defined? Spec is nil, but calling Spec returns RSpec, strange... Can you try Spec::VERSION::STRING and see if it returns something ?

grosser commented 13 years ago

please give the not defined? a try, if it works then this would be a smaller change/fix

tooky commented 13 years ago

hmmm, its weird.

The original breaks running my cucumber features, but I can't seem to find Spec defined anywhere.

The pattern I used here is the same as used in cucumber-rails.

tooky commented 13 years ago

Won't RSpec always be defined though as we are inside an RSpec module at this point?

Doesn't rescuing the LoadError ensure that we are using rspec2 if available, and then trying rspec1 if we don't, and we only throw a LoadError if that isn't available either?

grosser commented 13 years ago

yep good point, we can have a version = defined?(Rspec) ? 2 : 1 line above the module definition, but another straigt-forward test would be nice (e.g. Spec::VERSION::STRING / RSpec::Version::STRING)

Im not a fan of wrapping a bunch of code in a big rescue block, but having the rescue at the top would also work require 'xxx'; version = 1; rescue LoadError; require 'yyy'; version = 2

tooky commented 13 years ago

Yep agreed having a bunch of code in a rescue isn't ideal.

Just wondering if we can have the class definitions in separate files - just require the version 1 or 2 file as needed?

grosser commented 13 years ago

looks pretty clean, ill merge that :)

tooky commented 13 years ago

Great, thanks.