Permalink
Browse files

Add `Constant.original` API to query rspec-mocks about stubbed consta…

…nts.

This needs to be documented, but I want to get feedback from others before spending effort on that.
  • Loading branch information...
1 parent f94e6b2 commit 211743db22a44a5a42943ff2d0dff7ae8733260c @myronmarston myronmarston committed Jun 7, 2012
Showing with 124 additions and 0 deletions.
  1. +60 −0 lib/rspec/mocks/stub_const.rb
  2. +64 −0 spec/rspec/mocks/stub_const_spec.rb
@@ -17,6 +17,45 @@ def recursive_const_defined?(name)
end
end
+ class Constant
@dchelimsky

dchelimsky Jun 7, 2012

Owner

I'm not sure I'm comfortable with the name Constant. Seems too generic to me - likely to conflict. WDYT?

@myronmarston

myronmarston Jun 7, 2012

Owner

I started with the name StubbedConstant, but it seemed funny to have a method StubbedConstant#stubbed? and it made me realize you can get one of these for a constant that has not been stubbed, so I shortened it to Constant.

"likely to conflict"--you mean within rspec-mocks or in end-user test suites? It's properly namespaced within RSpec::Mocks so I'm not too concerned about name conflicts. Also, I don't think end users will ever manually construct one of these.

Still, if you've got a better idea for a name, I'm all ears :).

+ extend RecursiveConstMethods
+
+ def initialize(name)
+ @name = name
+ end
+
+ attr_reader :name
+ attr_accessor :original_value
+ attr_writer :previously_defined, :stubbed
+
+ def previously_defined?
+ @previously_defined
+ end
+
+ def stubbed?
+ @stubbed
+ end
+
+ def to_s
+ "#<#{self.class.name} #{name}>"
+ end
+ alias inspect to_s
+
+ def self.unstubbed(name)
@dchelimsky

dchelimsky Jun 7, 2012

Owner

Make unstubbed private?

+ const = new(name)
+ const.previously_defined = recursive_const_defined?(name)
+ const.stubbed = false
+ const.original_value = recursive_const_get(name) if const.previously_defined?
+
+ const
+ end
+
+ def self.original(name)
+ stubber = ConstantStubber.find(name)
+ stubber ? stubber.to_constant : unstubbed(name)
+ end
+ end
+
# Provides a means to stub constants.
class ConstantStubber
extend RecursiveConstMethods
@@ -62,6 +101,15 @@ def initialize(full_constant_name, stubbed_value, transfer_nested_constants)
@context_parts = @full_constant_name.split('::')
@const_name = @context_parts.pop
end
+
+ def to_constant
+ const = Constant.new(full_constant_name)
+ const.stubbed = true
+ const.previously_defined = previously_defined?
+ const.original_value = original_value
+
+ const
+ end
end
# Replaces a defined constant for the duration of an example.
@@ -80,6 +128,10 @@ def stub
transfer_nested_constants(constants_to_transfer)
end
+ def previously_defined?
+ true
+ end
+
def rspec_reset
@context.send(:remove_const, @const_name)
@context.const_set(@const_name, @original_value)
@@ -142,6 +194,10 @@ def stub
context.const_set(@const_name, @stubbed_value)
end
+ def previously_defined?
+ false
+ end
+
def rspec_reset
@deepest_defined_const.send(:remove_const, @const_to_remove)
end
@@ -181,6 +237,10 @@ def self.stubbers
@stubbers ||= []
end
+ def self.find(name)
+ stubbers.find { |s| s.full_constant_name == name }
+ end
+
# Used internally by the constant stubbing to raise a helpful
# error when a constant like "A::B::C" is stubbed and A::B is
# not a module (and thus, it's impossible to define "A::B::C"
@@ -240,6 +240,70 @@ def change_const_value_to(value)
end
end
end
+
+ describe Constant do
+ describe ".original" do
+ context 'for a previously defined unstubbed constant' do
+ subject { Constant.original("TestClass::M") }
+
+ its(:name) { should eq("TestClass::M") }
+ its(:previously_defined?) { should be_true }
+ its(:stubbed?) { should be_false }
+ its(:original_value) { should eq(:m) }
@dchelimsky

dchelimsky Jun 7, 2012

Owner

This is a beautiful example of how to properly use its! However, given that I've been talking about removing its from rspec-3, I'd like to not add more itss to rspec's own suite. How about:

describe Constant do
  describe ".original" do
    context 'for a previously defined unstubbed constant' do
      let(:orig) { Constant.original("TestClass::M") }

      it("exposes name")                         { orig.name.should eq "TestClass::M" }
      it("exposes original_value")               { orig.original_value.should eq :m }
      it("returns true for previously_defined?") { orig.previously_defined?.should be_true }
      it("returns false for stubbed?")           { orig.stubbed?.should be_false }
    end

    # ...
  end
end

Slightly noisier but quite readable. In fact, seeing orig.original_value this way makes me realize the redundancy in the name original_value, which I think could just be value: orig.value. And perhaps orig. defined? instead of orig.previously_defined???

Also, the output of --format documentation is cleaner this way.

@myronmarston

myronmarston Jun 7, 2012

Owner

This is a beautiful example of how to properly use its! However, given that I've been talking about removing its from rspec-3, I'd like to not add more itss to rspec's own suite. How about:

Haha, yeah, I almost didn't use its for that reason, but it felt nice to have a valid use case for it :). I like your refactoring, and you're right; the documentation output will be better.

In fact, seeing orig.original_value this way makes me realize the redundancy in the name original_value, which I think could just be value: orig.value. And perhaps orig. defined? instead of orig.previously_defined???

I think it looks redundant when you've chosen orig as your local variable name, or if you're chaining it directly off of Constant.original...but when a user does something like:

const = Constant.original("SomeConst")
# a bunch of code goes in between here
const.original_value
const.previously_defined?

It's nice to have the more descriptive names. In this context, const.value would sound (to me, at least) like it would return the current value; and const.defined? sounds like it returns whether or not the constant is currently defined. I think the longer names bring more clarity (it's really hard to misinterpret them!) even though it can be a bit redundant with a variable name like orig.

+ end
+
+ context 'for a previously defined stubbed constant' do
+ before { stub_const("TestClass::M", :other) }
+ subject { Constant.original("TestClass::M") }
+
+ its(:name) { should eq("TestClass::M") }
+ its(:previously_defined?) { should be_true }
+ its(:stubbed?) { should be_true }
+ its(:original_value) { should eq(:m) }
+ end
+
+ context 'for a previously undefined stubbed constant' do
+ before { stub_const("TestClass::Undefined", :other) }
+ subject { Constant.original("TestClass::Undefined") }
+
+ its(:name) { should eq("TestClass::Undefined") }
+ its(:previously_defined?) { should be_false }
+ its(:stubbed?) { should be_true }
+ its(:original_value) { should be_nil }
+ end
+
+ context 'for a previously undefined unstubbed constant' do
+ subject { Constant.original("TestClass::Undefined") }
+
+ its(:name) { should eq("TestClass::Undefined") }
+ its(:previously_defined?) { should be_false }
+ its(:stubbed?) { should be_false }
+ its(:original_value) { should be_nil }
+ end
+
+ context 'for a previously defined constant that has been stubbed twice' do
+ before { stub_const("TestClass::M", 1) }
+ before { stub_const("TestClass::M", 2) }
+ subject { Constant.original("TestClass::M") }
+
+ its(:name) { should eq("TestClass::M") }
+ its(:previously_defined?) { should be_true }
+ its(:stubbed?) { should be_true }
+ its(:original_value) { should eq(:m) }
+ end
+
+ context 'for a previously undefined constant that has been stubbed twice' do
+ before { stub_const("TestClass::Undefined", 1) }
+ before { stub_const("TestClass::Undefined", 2) }
+ subject { Constant.original("TestClass::Undefined") }
+
+ its(:name) { should eq("TestClass::Undefined") }
+ its(:previously_defined?) { should be_false }
+ its(:stubbed?) { should be_true }
+ its(:original_value) { should be_nil }
+ end
+ end
+ end
end
end

0 comments on commit 211743d

Please sign in to comment.