NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

GetArguments is returning the latest call for all calls if we're using the same reference type

Open hartmark opened this issue 1 year ago • 3 comments
trafficstars

Describe the bug It seems nsubstitute records all calls by reference rather than cloning the object.

To Reproduce Run the XUnit test below.

Expected behaviour All tests should pass. Or we should have a code analyzer hinting us that .Received(1) with argument matcher on reference types can have unexpected behaviour.

Environment:

  • NSubstitute version: [5.1.0]
  • NSubstitute.Analyzers version: [CSharp 1.0.17]
  • Platform: [net8 windows10]

Reproducing test

using System;
using System.Linq;
using NSubstitute;
using Xunit;
using Xunit.Abstractions;

namespace UnitTests.Tests;

public class GetArgumentsTest(ITestOutputHelper testOutputHelper)
{
    [Fact]
    public void ArgumentMatcher_OnSameInstance_ValueType()
    {
        var substitute = Substitute.For<IObjectUnderTest>();

        var value = "initialValue";
        substitute.Foo(value);
        value = "newValue";
        substitute.Foo(value);

        var args = substitute.ReceivedCalls().SelectMany(x => x.GetArguments());
        testOutputHelper.WriteLine(string.Join(", ", args));
    }
    
    [Fact]
    public void ArgumentMatcher_OnSameInstance_ReferenceType_SameInstance()
    {
        var substitute = Substitute.For<IObjectUnderTest>();

        var value = new MyReferenceType { Value = "initialValue"};
        substitute.Foo(value);
        value.Value = "newValue";
        substitute.Foo(value);

        var args = substitute.ReceivedCalls().SelectMany(x => x.GetArguments());
        testOutputHelper.WriteLine(string.Join(", ", args));
        
        substitute.Received(1).Foo(Arg.Is<MyReferenceType>(x => x.Value == "initialValue"));
        substitute.Received(1).Foo(Arg.Is<MyReferenceType>(x => x.Value == "newValue"));
    }
    
    [Fact]
    public void ArgumentMatcher_OnSameInstance_ReferenceType_NewInstance()
    {
        var substitute = Substitute.For<IObjectUnderTest>();

        var value = new MyReferenceType { Value = "initialValue"};
        substitute.Foo(value);
        value = new MyReferenceType { Value = "newValue"};
        substitute.Foo(value);

        var args = substitute.ReceivedCalls().SelectMany(x => x.GetArguments());
        testOutputHelper.WriteLine(string.Join(", ", args));
        
        substitute.Received(1).Foo(Arg.Is<MyReferenceType>(x => x.Value == "initialValue"));
        substitute.Received(1).Foo(Arg.Is<MyReferenceType>(x => x.Value == "newValue"));
    }
}

public class MyReferenceType
{
    public override string ToString()
    {
        return $"{nameof(Value)}: {Value}";
    }

    public string Value { get; set; }
}

public interface IObjectUnderTest
{
    public void Foo(object argument);
}

hartmark avatar May 17 '24 09:05 hartmark

This is by design. As you already figured out, there is no deep copy.

https://nsubstitute.github.io/help/argument-matchers/

Just use When/Do in these cases, e.g.:

  [Fact]
  public void ArgumentMatcher_OnSameInstance_ReferenceType_SameInstance()
  {
    var values = new List<string>();
    var substitute = Substitute.For<IObjectUnderTest>();
    substitute.WhenForAnyArgs(s => s.Foo(Arg.Any<MyReferenceType>()))
      .Do(c =>
      {
        var reference = c.ArgAt<MyReferenceType>(0);
        values.Add(reference.Value);
      });


    var value = new MyReferenceType { Value = "initialValue" };
    substitute.Foo(value);
    value.Value = "newValue";
    substitute.Foo(value);

    values.Should().BeEquivalentTo(["initialValue", "newValue"]);
  }

GeraldLx avatar May 23 '24 09:05 GeraldLx

Alright, if it is by design I guess we can close this issue.

However, it would be nice if it was clear in the documentation that this is the behavior.

hartmark avatar May 23 '24 13:05 hartmark

Hi @hartmark , Do you have any suggestion on good places to document this? Definitely agree it would be good to make this clear.

dtchepak avatar May 28 '24 12:05 dtchepak

Closing as this is by design (and no reaction after the last question)

304NotModified avatar Oct 28 '24 18:10 304NotModified