Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hartmark opened this issue May 17, 2024 · 3 comments

Comments

@hartmark
Copy link

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);
}
@GeraldLx
Copy link

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"]);
  }

@hartmark
Copy link
Author

hartmark commented May 23, 2024

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.

@dtchepak
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants