lunarmodules / luassert

Assertion library for Lua
MIT License
202 stars 77 forks source link

Unable to assert stubs were called with references to self using `:` syntax. #123

Closed jimmyharris closed 9 years ago

jimmyharris commented 9 years ago

I think the easiest way to explain this is with an example:

Let us assume I have the following object.

local MyObject = { name = 'default' }
-- A method that will be stubbed.
function MyObject:mock_method( arg1 ) end

Now we add a method to MyObject that will first make a call to the stub, then change the value of the 'name' field.

function MyObject:do_something_to_change_state()

  -- First call the mocked method which will store a deep 
  -- copy of "self" when self.name == 'default'
  self:mock_method( "hello" )
  self.mock_method( self, 'hello')

  -- Then change self.name to a new value.
  self.name = "Hey! I'm Different"
end

Later in a test we stub out mock_method and attempt to assert that it was called on the table referenced by MyObject.

stub( MyObject, 'mock_method')

MyObject:do_something_to_change_state()

-- This assertion fails because the first argument's name field still equals 'default'
-- and MyObject.name == "Hey! I'm Different" instead of 'default'
assert.stub( MyObject.mock_method ).was.called_with( MyObject )

The assertion will fail because the call history made a deep copy of MyObject when MyObject was in a different state.

I don't really have any ideas how to work around this. I feel like in every other case I would absolutely want the state to be copied for deep comparison.

I could always just say "I don't care" about the first argument but that solution feels unsatisfactory. Do you have any ideas on how I could go about dealing with this case?

o-lim commented 9 years ago

You can use a matcher to match against your object. You'll probably have to create your own custom matcher for this though. Actually, I think the latest master branch already does not perform a deepcopy for spies/stubs. Have you tried with the latest master?

jimmyharris commented 9 years ago

I am running on the latest master. The issue isn't that it deep copies spies and stubs, but that it actually deep copies the table I am trying to compare against. I would effectively have to make an object instance ID that could be copied so that I could compare those item ID values to see if it is the same object. I wish there was some way to tell the stub/spy that I am spying on a member function so don't deep copy the first argument. something like:

spy.on.member( object, 'member' )
-- or
stub.member( object, 'methodName' )

Then we could call a utils.match_member_args( ... ) or something to take into account the 'self' operator.

o-lim commented 9 years ago

Sorry, I was thinking that MyObject was also a spy/stub. What about something like this?

spy.on(object, 'member').no_deepcopy_arg(1)
-- or
stub(object, 'member').no_deepcopy_arg(1)

This is a relatively easy change to make and it provides a general solution to prevent any arbitrary argument from being deep copied by the spy/stub. And you can always undo the operation with something like (in case you change your mind later):

s.deep_copy_arg(1) -- where s is a stub or spy
o-lim commented 9 years ago

Actually, maybe a better interface would be something like

local match = require 'luassert.match'
assert.stub(s).was.called_with(match.is_ref(object))

Then you could also do

stub(object, 'member').on_call_with(match.is_ref(object))

I think this would be more flexible and intuitive to use.

o-lim commented 9 years ago

I'll try to get a pull request sometime today or tomorrow.

o-lim commented 9 years ago

@jimmyharris I submitted PR #124, which should resolve this. Can you give it a try and let me know what you think? It uses match.is_ref to match against the object reference.

jimmyharris commented 9 years ago

That sounds good actually.

jimmyharris commented 9 years ago

That is actually a perfect solution.