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

Received method does not store the original parameters of the call #851

Open
sashafp10 opened this issue Dec 5, 2024 · 2 comments
Open

Comments

@sashafp10
Copy link

sashafp10 commented Dec 5, 2024

Describe the bug
When I mock a method that accepts some objects as parameters more than once, I would have expected a number of invocations, but the parameters would be incorrect for each invocation except the last one.

To Reproduce

public class Paging
{
    public int PageSize { get; private set; }
    public int Offset { get; private set; }

    public Paging(int pageSize, int offset)
    {
        PageSize = pageSize;
        Offset = offset;
    }

    public void NextPage()
    {
        Offset += PageSize;
    }

    public override string ToString()
    {
        return $"PageSize: {PageSize}, Offset: {Offset}";
    }
}

public class PagePrinter
{
    public virtual void PrintPage(Paging paging)
    {
        Console.WriteLine(paging);
    }
}

public class PageLister
{
    private readonly PagePrinter _printer;

    public PageLister(PagePrinter printer)
    {
        _printer = printer;
    }

    public void ToPage(int times, Paging paging)
    {
        for (int i = 0; i < times; i++)
        {
            paging.NextPage();
            _printer.PrintPage(paging);
        }
    }
}

[Fact]
public void TestMultiplePagePrinting2()
{
    // Arrange
    var mockPrinter = Substitute.For<PagePrinter>();
    var pager = new Paging(10, 0);
    var pageLister = new PageLister(mockPrinter);
    int numberOfPages = 5;

    // Act
    pageLister.ToPage(numberOfPages, pager);

    // Assert
    mockPrinter.Received(numberOfPages).PrintPage(Arg.Any<Paging>());

    for (int i = 1; i <= numberOfPages; i++)
    {
        int expectedOffset = 10 * i;
        mockPrinter.Received(1).PrintPage(Arg.Is<Paging>(p => p.Offset == expectedOffset));
    }

    Received.InOrder(() =>
    {
        for (int i = 1; i <= numberOfPages; i++)
        {
            int expectedOffset = 10 * i;
            mockPrinter.PrintPage(Arg.Is<Paging>(p => p.Offset == expectedOffset && p.ToString() != null));
        }
    });
}

In the test log you would see that all invocations are with the same parameters.

Expected behaviour
In the test log, you will see that all invocations have the same parameters. I should not need to change the main implementation to fit the tests.

Environment:

  • NSubstitute version: 5.3.0
  • Platform: .net8, windows 11

Additional context
The current behavior can be explained easily because I mutate my parameter object during the main functionality. However, this should not affect the invocation history.

@dtchepak
Copy link
Member

dtchepak commented Dec 8, 2024

Hi @sashafp10 ,

Thanks for raising this and the great repro case.

We've encountered this before. The issue is that storing deep copies of parameters on every call is terrible for performance. Very open to suggestions on how to deal with this. 🙇

One alternative is using When to snapshot the arguments used during the test. Not sure this is an acceptable option for you, but does allow the code to be tested without changing production code to fit the tests.

@sashafp10
Copy link
Author

Hello @dtchepak ,

Thanks for the reply.
You are right - it impacts the performance. The best option I can see here is to allow to turn on such behavior on a per-function basis. I mean, to allow users to choose whether to use this custom params handling or the default (the current) one. In this case, it will not impact the current performance but allow using of the new feature at the same time.

Please let me know your thoughts on this.

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

2 participants