Feature proposal: constant stubbing

[myronmarston]

A few months ago, I added constant stubbing functionality to @xaviershay's rspec-fire. From the start, I thought this would be a great addition to rspec-mocks, but wanted to vet it and work out the kinks in rspec-fire first. At this point, i'd like to get feedback on adding it rspec-mocks. I know that @garybernhardt is eager to see it added here.

Here's how the feature works in rspec-fire (and how I'd imagine it working in rspec-mocks):

• The basic API is stub_const("Foo", some_object). For the duration of the example, the constant Foo will be a reference to some_object.
• It supports deeply nested constants, even if the intermediary constants are undefined. Intermediary modules are dynamically created as necessary. stub_const("A::B::C::D", 5) will make A::B::C::D equal to 5 even if none of those modules previously existed.
• When the example completes and resets rspec-mocks space, it restores the constant to whatever state it was in before the constant stubbing. If the constant already existed, it is set back to refer to what it did previously. If the constant did not already exist, the dynamically created constants are undefined.
• There's a caveat to that last point: if, during the example, the value of the constant is changed by the user (e.g. because the example loads a file that redefines the constant, or whatever), then the constant is not restored to its original value; the thinking is that if the user is changing the constant on their own then they know what they're doing and we shouldn't change it out underneath them.
• stub_const also supports a :transfer_nested_constants option. This is sometimes needed when stubbing a module constant that is a namespace for other constants--without this option, a user would cut off their access to A::B when they stub A. With this option, you can pass true to have it transfer all nested constants, or pass a list of constants to transfer. Here's an example:

module A
  module B
  end

  module C
    module D
    end
  end

  module E
  end

  F = 5
end

stub_const("A", Module.new)
A # => the new module
# A::B, A::C, A::E, A::F => undefined const errors

stub_const("A", Module.new, :transfer_nested_constants => true)
A # => the new module
A::B # => the original A::B
A::C # => the original A::C
A::E # => the original A::E
A::F # => 5

stub_const("A", Module.new, :transfer_nested_constants => [:B, :F])
# => only A::B and A::F are transferred

# This will raise an error; since A::F isn't a module (and hence can't have constants nested under it)
# there are no constants to transfer.
stub_const("A::F", Module.new, :transfer_nested_constants => true)

# This will raise an error; constants can only be transferred to a module, but our stubbed value is not a module
stub_const("A", Object.new, :transfer_nested_constants => true)

I've found this to be very useful for a couple reasons:

• Previously, I have on occasion defined a class method with a constant value for something I would normally use a constant, just to make it stubbable. With this in place, you can still use constants where they make sense, and stub them in your tests.
• This is super useful (especially in the context of rspec-fire) as a way of doing dependency injection, particularly when the object-under-test has a dependency on a class method.

So...I'd love to get both feedback from the community and buy-in from other rspec contributors before I start working on this.

Thoughts?

/cc @dchelimsky @justinko @garybernhardt @xaviershay @freelancing-god @coreyhaines

[dchelimsky]

This all sounds good to me except:

There's a caveat to that last point: if, during the example, the value of the constant is changed by the user (e.g. because the example loads a file that redefines the constant, or whatever), then the constant is not restored to its original value; the thinking is that if the user is changing the constant on their own then they know what they're doing and we shouldn't change it out underneath them.

Personally, I'd rather be able to rely on trusting that no matter what happens in my examples, the state is restored at the end of each.

[myronmarston]

I don't feel very strongly either way on that point. At the time I was working on the code in rspec fire, it made sense to me to do it that way, but in retrospect, I'm not sure it was the right choice.

Also, I just tried this:

class Foo
  def self.bar
    "original bar"
  end
end

describe Foo do
  specify do
    Foo.stub(:bar) { "stubbed bar" }
    def Foo.bar; "overridden bar"; end
  end

  specify do
    Foo.bar.should eq("original bar")
  end
end

...and I'm seeing that stubbed methods get restored even if they get re-defined by the user after being stubbed. I think rspec-mocks should be consistent here, and always restore the original state of constants no matter what.

[garybernhardt]

I'd rather have the cleanup happen unconditionally as well.

[justinko]

:+1:

This is just great. Would allow me to get rid of my home grown stub_const that does not "rollback"!

[xaviershay]

This would be awesome because it means less code for me to maintain in rspec-fire :P

(which actually isn't a problem because @myronmarston does such a good job of it...)