Nsubstitute: Argument matcher of received call does not match object state during call

Created on 27 Apr 2018  路  5Comments  路  Source: nsubstitute/NSubstitute

In the below test I want to check if the Act(person) method is called with a person with name John. However if the name of the object is changed between the received call and the assertion, the new name is taken. I would expect that the argument matcher takes the state of the object when the call was made.

So I would expect the test below to be successful, however it fails.

```C#
[TestFixture]
public class Test
{
[Test]
public void CheckReceived()
{
//ARRANGE
var action = Substitute.For();
var person = new Person(){Name = "John"};

    //ACT
    action.Act(person);
    person.Name = "Doe";

    //ASSERT
    action.Received().Act(Arg.Is<Person>(p => p.Name.Equals("John")));
}

}

public interface IAction
{
void Act(Person person);
}

public class Person
{
public string Name { get; set; }
}
```
The source code of this test is available here: https://github.com/Dommicentl/NSubstituteTest

Most helpful comment

By the way, you can use the following workaround:

 // arrange
var action = Substitute.For<IAction>();
var person = new Person(){Name = "John"};

bool received = false;
action
    .When(x => x.Act(Arg.Is<Person>(p => p.Name.Equals("John"))))
    .Do(_ => received = true);

// act
action.Act(person);
person.Name = "Doe";

// assert
Assert.True(received);

All 5 comments

Yeah, currently we capture the value by reference (for reference types), so the sample above fails. I'd strongly argue against capturing the full state of the argument as:

  • that will heavily affect the performance - object graph might be really deep;
  • we'll face over 9000 obstacles with serialization. For instance, I'm not sure that proxies generated by Castle are serializable (scenario if you use substitute as an arg).

Rather, if you feel that it affects you heavily during testing, make your domain objects immutable 馃槄

That's my opinion. Let's see what @alexandrnikitin and @dtchepak think.

By the way, you can use the following workaround:

 // arrange
var action = Substitute.For<IAction>();
var person = new Person(){Name = "John"};

bool received = false;
action
    .When(x => x.Act(Arg.Is<Person>(p => p.Name.Equals("John"))))
    .Do(_ => received = true);

// act
action.Act(person);
person.Name = "Doe";

// assert
Assert.True(received);

Yes we are indeed currently using a workaround with the .When().Do() construct similar to the one @remember664 describes.

The current behavior just seems counter-intuitive and it took us a while before realizing what the problem was.

Thanks for the question @Dommicentl. As @zvirja mentioned there are a number of reasons this behaviour exists, not least of which is that C# is pass-by-reference for classes (or at least, by pass-by-value-of-reference 馃槃) so if sub.Received().ThisCall(abc) did not use the reference for abc that would introduce its own confusion (if a property of abc changed is this no longer the same object? Should the assertion fail?).

If you change Person to a struct it will get passed by value and the test will pass as you originally expected.

Sorry for the confusion. If it is any consolation I've found after being bitten by this once (I have!) then I've found that I never made the same assumption again, the effort to debug it the first time stuck in my memory. 馃槵馃槃

A slight tweak to @remember664's excellent suggestion, we can also use Arg.Do to catch the name at the time of call:

[Fact]
public void CheckReceived() {
    //ARRANGE
    var action = Substitute.For<IAction>();
    var person = new Person() { Name = "Leendert" };
    string nameUsedInCall = null;
    action.Act(Arg.Do((Person p) => nameUsedInCall = p.Name));

    //ACT
    action.Act(person);
    person.Name = "Dommicent";

    //ASSERT
    Assert.Equal("Leendert", nameUsedInCall); 
    // implicitly tests call was received at least once as `nameUsedInCall` is no longer `null`.
}

This can be useful for performing more complex assertions on arguments (like detailed list inspection and other things that get a bit wordy with the Arg.Is syntax).

The slight tweak @dtchepak suggested is indeed how we currently solve this issue. If that is indeed the way to go to assert this type of calls we will continue using it.

I understand the reasons you explain behind the current design. Thank you for the explanation.

Was this page helpful?
0 / 5 - 0 ratings