Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature proposal: constant stubbing #144

Closed
myronmarston opened this issue May 31, 2012 · 5 comments

Comments

Projects
None yet
5 participants
@myronmarston
Copy link
Member

commented May 31, 2012

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 comment has been minimized.

Copy link
Member

commented Jun 1, 2012

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

This comment has been minimized.

Copy link
Member Author

commented Jun 1, 2012

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

This comment has been minimized.

Copy link

commented Jun 1, 2012

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

@justinko

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2012

👍

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

@xaviershay

This comment has been minimized.

Copy link
Member

commented Jun 2, 2012

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...)

myronmarston added a commit that referenced this issue Jun 2, 2012

First pass at implementing constant stubbing.
This is almost copied verbatim from rspec-fire.

For #144.

myronmarston added a commit that referenced this issue Jun 2, 2012

Always restore original constants.
The original logic from rspec-fire did not restore original constants
if the user changed them in the example after stubbing them, but after
talking it over with @dchelimsky and @garyberhnardt we've decided to
be consistent and always restore them.

For #144.

myronmarston added a commit that referenced this issue Jun 2, 2012

Remove the bang from our #stub! methods.
I'm not really sure why I used them when I wrote this in rspec-fire;
given there were not corresponding bang-less methods, it didn't
really make sense.

For #144.

myronmarston added a commit that referenced this issue Jun 2, 2012

Fix a constant stubbing edge case.
stub_const("A::B::C", whatever) cannot work if A::B is defined
but A::B is not a module.

For #144.

myronmarston added a commit that referenced this issue Jun 2, 2012

myronmarston added a commit that referenced this issue Jun 2, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.