Inconsistency in reconfiguration behaviour when reconfiguring a method using .Returns() vs using .When..Do

[egraff]

Describe the bug When reconfiguring a substitute using the .When().Do() pattern, the "returnThis" callback is not replaced, but rather added as an action to be called when the substitute is invoked.

To Reproduce

The following test scenarios describe the problem.

The first test (which passes) shows a substitute that is configured and reconfigured using .Returns(). The reconfiguration replaces the "returnThis" callback that was previously configured. This is proven by cnt being equal to 1 and source being equal to "second" at the end of the test. Note that without the call to .Configure(), the first "returnThis" callback will be called when the substitute is reconfigured (the sanity check will fail), but the invocation of the substitute further below still only calls the last "returnThis" (cnt would be equal to 2, not 3, and "source" would be equal to "second"). All of this is expected behaviour.

The problem is with the second test (which fails). There, a substitute is configured and reconfigured using .When() and .Do(). The reconfiguration adds the "returnThis" callback, instead of replacing it. The reconfiguration itself does not invoke the previous "returnThis" callback (the sanity check assert passes, so cnt is still equal to 0 before the invocation of the substitute). However, the invocation of the substitute invokes both the old and the new "returnThis" callback (cnt is equal to 2, not 1, so the assertion fails).

Interestingly, changing the .When() line to .When(x => x.Configure().Invoke()) makes the last test pass, but this code simply looks wrong.

[Test]
public void Works_As_Expected()
{
   /* Arrange */

   int cnt = 0;
   string source = null;

   Func<int> substitute = Substitute.For<Func<int>>();
   substitute.Invoke().Returns(
       _ =>
       {
           cnt++;
           source = "first";
           return 123;
       }
   );

   /* Act */

   // Note: without Configure(), the sanity check below fails
   substitute.Configure().Invoke().Returns(
       _ =>
       {
           cnt++;
           source = "second";
           return 123;
       }
   );

   // Sanity check (configure should not have invoked previous "returnThis")
   Assert.That(cnt, Is.Zero);

   _ = substitute();

   /* Assert */

   Assert.That(cnt, Is.EqualTo(1));
   Assert.That(source, Is.EqualTo("second"));
}

[Test]
public void Problem()
{
   /* Arrange */

   int cnt = 0;

   Action substitute = Substitute.For<Action>();
   substitute
       .When(x => x.Invoke())
       .Do(_ => cnt++);

   /* Act */

   substitute
       .When(x => x.Invoke())
       .Do(_ => cnt++);

   // Sanity check (configure should not have invoked previous "returnThis")
   Assert.That(cnt, Is.Zero);

   substitute();

   /* Assert */

   Assert.That(cnt, Is.EqualTo(1));
}

Expected behaviour Reconfiguration using .When() and .Do() should replace the "returnThis" callback, to be consistent with how reconfiguration behaves when using .Returns().

Environment:

• NSubstitute version: 4.2.2
• Platform: .NET 4.8 on Windows (but I suspect this is not platform or framework dependent behaviour).