xaviershay/rspec-fire — pull request #5 by myronmarston (2012-03-13 23:47:46+00:00)
Title: Transfer nested constants

This is a followup to #4.

* This switches `fire_class_double` to use a module rather than a subclass of `RSpec::Mocks::Mock`. The reason for the change is to allow class doubles to have nested constants like real classes and modules. I was surprised at how seamless this change was. I think we do lose a couple things in this change, though:
* There's not the `:__declared_as` entry in the mock options hash that rspec uses as part of its error message. This isn't a big deal to me, though.
* RSpec mocks support additional options when declared that aren't supported here. Besides some deprecated things (like `:null_object => true`), I think rspec treats an options hash as a list of methods-to-stub and corresponding values. It wouldn't be hard to add that here.
* I decided to not transfer constants by default. I like the flexibility of being able to either have it transfer all nested constants (`:transfer_nested_constants => true`) or specify a list of constants to transfer (`:transfer_nested_constants => [:A, :B])`. The APIs were more natural and readable this way then trying to do the inverse, I think.
* There are a few edge cases I had to think carefully about and decide what to do for. I decided to raise errors for several of these, but I can be convinced otherwise. Specifically, here are the cases that cause it to raise an error:
* When `stub_const` is called with a `:transfer_nested_constants` option and the named constant is not a class or module. The original object doesn't have any constants to transfer then.
* When `stub_const` is called with a `:transfer_nested_constants` option and the given stub object is not a class or module. The constants cannot be transferred to it then.
* When a list of constants is used (`:transfer_nested_constants => [:A]`) but one or more of those are not defined constants. I can see an argument either way on this one (especially given rspec-fire's use case of being used with as little loaded as possible), but I felt like putting a constant in the whitelist implies it is an important constant that you always want to be transferred, and thus an error to notify you that the constant is undefined is helpful. In most cases, I would expect this to happen due to fat-fingering a constant name.
* Currently, constant transferring isn't supported when using `fire_replaced_class_double`. Instead you have to use `fire_class_double("ClassName").as_replaced_constant(:transfer_nested_constants => true)`. I felt like it would be confusing to support the option directly on `fire_replaced_class_double` since it accepts a hash of options that are intended to be test double options. The `:transfer_nested_constants` option is specific to the constant stubbing and felt like it belonged there to keep things simpler. This suggests to me that maybe `fire_class_double("...").as_replaced_constant` is the right interface for this.
* I noticed that the rspec-fire was broken against the just-released rspec 2.9.0.rc2, so I fixed that as well.

Feedback welcome, of course.

/cc @freelancing-god