Calling When on partial substitute fails for async methods

[siewers]

Describe the bug
ReSharper test runner fails executing the test, when calling When on a partial substitute.

Error output:

2018.10.19 00:32:26.157    WARN The active test run was aborted. Reason: 
2018.10.19 00:32:27.884   ERROR Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host. at System.Net.Sockets.NetworkStream.Read(Span`1 destination) at System.Net.Sockets.NetworkStream.ReadByte() at System.IO.BinaryReader.ReadByte() at System.IO.BinaryReader.Read7BitEncodedInt() at System.IO.BinaryReader.ReadString() at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.LengthPrefixCommunicationChannel.NotifyDataAvailable() at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TcpClientExtensions.MessageLoopAsync(TcpClient client, ICommunicationChannel channel, Action`1 errorHandler, CancellationToken cancellationToken) Source: System.Net.Sockets HResult: -2146232800 Inner Exception: An existing connection was forcibly closed by the remote host HResult: -2147467259

--- EXCEPTION #1/1 [LoggerException]
Message = “
  Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host.
     at System.Net.Sockets.NetworkStream.Read(Span`1 destination)
     at System.Net.Sockets.NetworkStream.ReadByte()
     at System.IO.BinaryReader.ReadByte()
     at System.IO.BinaryReader.Read7BitEncodedInt()
     at System.IO.BinaryReader.ReadString()
     at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.LengthPrefixCommunicationChannel.NotifyDataAvailable()
     at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TcpClientExtensions.MessageLoopAsync(TcpClient client, ICommunicationChannel channel, Action`1 errorHandler, CancellationToken cancellationToken)
  Source: System.Net.Sockets
  HResult: -2146232800
  
  Inner Exception: An existing connection was forcibly closed by the remote host
  HResult: -2147467259
”
ExceptionPath = Root
ClassName = JetBrains.Util.LoggerException
HResult = COR_E_APPLICATION=80131600
StackTraceString = “
  at JetBrains.ReSharper.UnitTestFramework.DotNetCore.DotNetVsTest.DotNetVsTestExecution.OnRunComplete(ExecutionCompletePayload payload)
     at Appccelerate.StateMachine.Machine.ActionHolders.ArgumentActionHolder`1.Execute(Object argument)
     at Appccelerate.StateMachine.Machine.Transitions.Transition`2.PerformActions(ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.Transitions.Transition`2.Fire(IState`2 source, IState`2 target, ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.Transitions.Transition`2.Fire(IState`2 source, IState`2 target, ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.Transitions.Transition`2.Fire(IState`2 source, IState`2 target, ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.Transitions.Transition`2.Fire(ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.States.State`2.Fire(ITransitionContext`2 context)
     at Appccelerate.StateMachine.Machine.StateMachine`2.Fire(TEvent eventId, Object eventArgument)
     at Appccelerate.StateMachine.ActiveStateMachine`2.ProcessEventQueue(CancellationToken cancellationToken)
     at Appccelerate.StateMachine.ActiveStateMachine`2.<Start>b__32_0()
     at System.Threading.Tasks.Task.InnerInvoke()
     at System.Threading.Tasks.Task.Execute()
     at System.Threading.Tasks.Task.ExecutionContextCallback(Object obj)
     at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
     at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
     at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
     at System.Threading.Tasks.Task.ExecuteEntry(Boolean bPreventDoubleExecution)
     at System.Threading.Tasks.ThreadPoolTaskScheduler.LongRunningThreadWork(Object obj)
     at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
     at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
     at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
     at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
     at System.Threading.ThreadHelper.ThreadStart(Object obj)
”

To Reproduce

[TestFixture]
public class StuffDoerTests
{
    [Test]
    public async Task CanDoStuff()
    {
        var stuffDoer = Substitute.ForPartsOf<StuffDoer>();
        // This does not work
        stuffDoer.When(async s => await s.DoStuff(Arg.Any<string>())).DoNotCallBase();
        // This works but produces a compiler warning: CS4014: Async method invocation without await expression
        // stuffDoer.When(s => s.DoStuff(Arg.Any<string>())).DoNotCallBase();
        stuffDoer.DoStuff(Arg.Any<string>()).Returns("StuffValue");

        string stuff = await stuffDoer.GetStuff("Input");

        Assert.That(stuff, Is.EqualTo("StuffValueInput"));
    }
}

public class StuffDoer
{
    public async Task<string> GetStuff(string input)
    {
        string stuff = await DoStuff(input);
        return stuff + input;
    }

    public virtual async Task<string> DoStuff(string value)
    {
        return await Task.Run(() => value.GetHashCode().ToString());
    }
}

Expected behaviour
I would expect the test to pass but it fails.

Environment:

• NSubstitute version: [e.g. 3.1.0.130] (pre-release). Stable release exhibits same behavior.
• Platform: dotnetcore2.0 project on Windows 10
• Test runner: ReSharper 2018.2.3
• Visual Studio 2017 15.8.7

[dtchepak]

Thanks for the excellent report and reproduction example. 👍

I can confirm this also happens with VS for Mac test runner using stable and pre-release NSub.

Environment:

• NSubstitute 3.1.0.130, 3.1.0 stable
• Platform: dotnetcore 2.0 on Mac
• Test runner: Visual Studio for Mac 7.7 build 1470

I got this trace in the output:

Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at NSubTestWorkshop.StuffDoerTests.<>c.<<CanDoStuff>b__0_0>d.MoveNext() in /Users/dtchepak/dev/git/NSubTestWorkshop/UnitTest1.cs:line 15
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
The application was terminated by a signal: SIGABRT

[zvirja]

@siewers Thanks for the report. In short, you should not await in the When() callback - we expect that return value for the invoked method is never used and often return null there.

I fired a PR to add When() overload which consumes Task or ValueTask<T>, so you will no longer get the compilation warning if write without await.

[tpodolak]

@zvirja @dtchepak
Should we add analyzer for async usage in When method?

[zvirja]

@tpodolak Good catch. I think it sort of makes sense. Analyzer should prevent async statements and ask to use overload taking Func<T, Task>.