NSubstitute icon indicating copy to clipboard operation
NSubstitute copied to clipboard

Assertion passes when an argument is used from the substitute itself

Open Epicycle23 opened this issue 1 year ago • 1 comments

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

Epicycle23 avatar Oct 26 '23 09:10 Epicycle23

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: https://github.com/nsubstitute/NSubstitute.Analyzers/issues/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. 🤔

dtchepak avatar Oct 28 '23 10:10 dtchepak