Spy: cloning arguments so we can assert on the value at the time they were called #281

wants to merge 1 commit into

3 participants



I noticed that spyOn only keeps a reference to the arguments used in function calls.
This can be handy to compare exact reference-equality, but not so much in cases like this:

var mock = { fn: function(x) {} };
spyOn(mock, 'fn');

var params = { val: 1 };
params.val = 2;

expect(mock.fn.argsForCall[0]).toEqual([{val: 1}]);
// currently fails, saying it was called with "val = 2"

This pull request does a shallow copy of all arguments to avoid this - with corresponding unit tests.

All existing tests pass, however it does change the existing behaviour, in cases where users are testing for reference-equality of arguments. What do you think? A possibility could be to keep both the reference and a copy, and leave the option of which one to check to the user.


Oh, interesting.

I think most people want to test references with spies. I usually care more that I called a spy with a particular object than some specific values of that object. I expect these changes would cause the following assertion to fail (well, would fail if you didn't have the second mock.fn(params) call).


My guess is that more people would want that to pass than would want the ability to mutate the argument they've passed to a spy and then make expectations about it.

I'd suggest if you care about the arguments at the time the call is made, you use andCallFake and clone those arguments within the function you pass to the fake. Closing because I don't think we'll make this change, but happy to discuss it more if you like.

@ragaskar ragaskar closed this Sep 26, 2012

Is it possible, that cloning would be optional or add additional assertion?
sth. like:

toHaveBeenInvokedWith() or toHaveBeenPassedArguments()

I also find kind of hard to test functions that mutate arguments with original behavior.

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