Object#stub can now be used without a block (along with Object#unstub) - Closes #152 #153

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Owner

zenspider commented Aug 15, 2012

Why?

I outlined my rationale in #152, but I'll give an example here (still somewhat contrived):

class Date
   def recent?
      (self - Date.today).to_i.abs <= 30
   end
end

describe Date
  before { Date.stub :today, Date.new(2012,9,1) }
  after  { Date.unstub :today }

  describe "#recent?"
    it "must return true for recent dates" do
      Date.new(2012,8,2).recent?.must_equal true
      Date.new(2012,9,15).recent?.must_equal true
      Date.new(2012,10,1).recent?.must_equal true
    end

    it "must return false for not-so-recent dates" do
      Date.new(2011,1,1).recent?.must_equal false
      Date.new(2012,8,1).recent?.must_equal false
      Date.new(2012,10,2).recent?.must_equal false
      Date.new(2014,12,31).recent?.must_equal false
    end
  end
end

This also works when say, testing IO stuff and APIs, where you can stub out things like File.open upfront in a setup/before block and restore them in the teardown/after block.

zenspider was assigned Aug 16, 2012

eloyesp commented Aug 24, 2012

Why not to unstub automatically on teardawn? This way, you cannot shoot yourself (because stubs are destroyed for free). in this way, a stub without a block can make a hook on after (or teardown) that remove the stub.

before { Date.stub :today, Date.new(2012,9,1) } # => after { Date.unstub :today }

This way stubing would be easier, and foolproof.

If it is possible to make an after hook (or teardawn) from inside a test it could be easier to add a stub inside a test with the same idea (and much more readable to add several stubs).

My guess is this is probably too much magic for @zenspider... Also, you might legitimately need to use either the stubbed or original method to do some cleanup work in your own after block, so there is this question of where this "automatic" cleanup block should be inserted into the chain.

eloyesp commented Aug 24, 2012

This block should run at the "very end" and should unstub every stubed method. Nothing stop to unstub manualy inside the test or in after block.

I'm thinking on a quite simple implemetation, but i'm not sure if its possible. I'd add an attr_accessor stubs (with a hash may be), so if you stub something without a block, then usefull info is added to this hash (like {:Date => :today} ), when you unstub it is deleted, and at the very_end (after_teardown) it unstub everything (if there is something to unstub).

As it is run in an ensure clause it will run for sure, so the environment keeps clean, and this will look nicer.

def test_stale_eh
  obj_under_test = Something.new

  refute obj_under_test.stale?

  Time.stub :now, Time.at(0) # stub goes away after teardown
  assert obj_under_test.stale?
end

eloyesp commented Aug 24, 2012

I looked at the code a while, and the tests are realy difficult to me to understand.

But is worse that I dont know how to refer to the TestCase instance from inside the test method.

@zenspider I came across another use case for this today:

  def with_doorkeeper_token(resource_owner_id = nil, scopes = [], &block)
    token = Object.new

    token.define_singleton_method(:accessible?) { true }
    token.define_singleton_method(:resource_owner_id) { resource_owner_id }
    token.define_singleton_method(:scopes) { scopes }

    @controller.stub(:doorkeeper_token, token) { yield }
  end

This could have been...

  def with_doorkeeper_token(resource_owner_id = nil, scopes = [], &block)
    token = Object.new

    token.stub :accessible?, true
    token.stub :resource_owner_id, resource_owner_id
    token.stub :scopes, scopes

    @controller.stub(:doorkeeper_token, token) { yield }
  end

Small difference, but the intent is clearer in the second version IMO. (I guess this might also be an argument against automatically unstubing in after/teardown.)

I picked up your suggestion (using a helper method) where it makes sense, but I still think there are use cases like these where it might make sense to do it differently, and I don't think minitest should dictate that I can't ever use it without a block (or that I must unstub a method every time for that matter).

In case you are wondering why I didn't use a mock, it's because these methods might get called multiple times (or not at all) and that's not the focus of my tests here to verify that.

eloyesp commented Aug 28, 2012

it might be done with the actual implementation... (but it's realy ugly)

def with_doorkeeper_token(resource_owner_id = nil, scopes = [], &block)
  token = Object.new

  token.stub :accessible?, true do
    token.stub :resource_owner_id, resource_owner_id do
      token.stub :scopes, scopes do
        @controller.stub(:doorkeeper_token, token) { yield }
      end
    end
  end
end

My point exactly (see also my comment in #152). Trust the programmer and let the programmer decide.

Owner

zenspider commented Nov 9, 2012

Closed for reasons spelled out in #152.

zenspider closed this Nov 9, 2012

zenspider locked and limited conversation to collaborators May 17, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.