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

Assertion passes when an argument is used from the substitute itself #749

Open
Epicycle23 opened this issue Oct 26, 2023 · 1 comment
Open
Labels
bug Reported problem with NSubstitute behaviour

Comments

@Epicycle23
Copy link

Epicycle23 commented Oct 26, 2023

Describe the bug
When passing a parameter from the substitute itself to a (received) assertion it passes although it clearly shouldn't.

To Reproduce

        public interface ICandidate
        {
            string Name { get; set; }
            void Perform(string input, IEnumerable<string> values);
        }
        [TestMethod]
        public void Substitute_Received_ShouldNotPass()
        {
            var candidate = Substitute.For<ICandidate>();
            candidate.Perform(candidate.Name, new[] { "a" });
            candidate.Received(1).Perform(candidate.Name, Arg.Is<IEnumerable<string>>(new[] { "b" }));
        }

Expected behaviour
The example test should not pass due to the not matching second argument.

Environment:

  • NSubstitute version: 5.1.0.0
  • NSubstitute.Analyzers version: CSharp 1.0.16
  • Platform: .net framework 4.8 on windows
@dtchepak dtchepak added the bug Reported problem with NSubstitute behaviour label Oct 27, 2023
@dtchepak
Copy link
Member

Hi @Epicycle23 ,

Thanks for raising this. This behaviour is due to re-entrant substitute use, which is an unfortunate limitation of the trick NSubstitute's syntax uses (see also: nsubstitute/NSubstitute.Analyzers#12).

When running the Received call, NSubsitute sees something like this:

candidate.Perform(candidate.Name, new[] { "a" });
candidate
    .Received() // puts sub in "assert next call was received" mode
    .Perform(    // Perform is not run yet, as args need to be evaluated...
        candidate.Name // <-- this is the next call, and it WAS received in the line above (second line of the test)
        ...
   )  // assert passes as candidate.Name was received

We can see this by commenting out the second line:

    var candidate = Substitute.For<ICandidate>();
    // candidate.Perform("", new[] { "a" });
    candidate.Received().Perform(candidate.Name, Arg.Is<IEnumerable<string>>(new[] { "b" }));
/*
ReceivedCallsException : Expected to receive a call matching:
        Name
Actually received no matching calls.
*/

To fix this it is best to pass the values directly to the substitute:

    var candidate = Substitute.For<ICandidate>();
    candidate.Perform("test", new[] { "a" });
    candidate.Received().Perform("test", Arg.Is<IEnumerable<string>>(new[] { "b" }));
/*
.ReceivedCallsException : Expected to receive a call matching:
        Perform("test", String[])
Actually received no matching calls.
Received 1 non-matching call (non-matching arguments indicated with '*' characters):
        Perform("test", *String[]*)
*/

@tpodolak is it possible for Analyzers to detect this case with Received similar to how it works for Returns? Not sure if this is one of those cases you tried but it ended up taking too long for the analysis to run or one that could not be detected reliably. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Reported problem with NSubstitute behaviour
Projects
None yet
Development

No branches or pull requests

2 participants