Fixed UnobservedTaskException after using ThrowsAsync. Fixed ReturnsAsync and ThrowsAsync with delay

[snrnats]

This closes #592, closes #593

[snrnats]

@stakx I have have a concern about changed behavior that we have discussed. We chose to keep the behavior that ReturnsAsync and ThrowsAsync keeps returning the same task because it's how it worked before. But it's not possible with delayed ReturnsAsync or ThrowsAsync. New task need to be created to start the timer over. Would it be better to implement a new consistent behavior for all ReturnsAsync and ThrowsAsync where each invocation will return a new task?

[stakx]

@snrnats: Thanks for this new PR. Thank you also for raising above question.

Would it be better to implement a new consistent behavior for all ReturnsAsync and ThrowsAsync where each invocation will return a new task?

Yes, after thinking about this a little longer, I believe this would be the better solution; for two reasons:

1. The majority of ReturnsAsync and ThrowsAsync methods already seem to cause the setup methods to return new Tasks for each invocation. (I haven't finished verifying this, you're welcome to verify this claim independently.)

2. When you manually implement a method returning some Task<TResult>, you'll also end up with an implementation that returns a new Task for each invocation unless you make a deliberate effort to the contrary:

Task<int> IFoo.GetSomeNumberAsync()  // returns a new Task each time
{
    return Task.FromResult(42);
}

Task<int> IFoo.GetSomeNumberAsync()  // returns a new Task each time
{
    var tcs = new TaskCompletionSource<int>();
    tcs.SetException(new Exception("..."));
    return tcs.Task;
}

async Task<int> IFoo.GetSomeNumberAsync()  // returns a new Task each time
{
    await Task.Delay(0);
    return 42;
}

async Task<int> IFoo.GetSomeNumberAsync()  // returns a new Task each time
{
    await Task.Delay(0);
    throw new Exception("...");
}

To return the same Task on each invocation, you'd have to hack together something more complicated, e.g. this:

private Lazy<Task<int>> cachedTask = new Lazy<Task<int>>(() =>
{
    return Task.FromResult(42);
});

Task<int> IFoo.GetSomeNumberAsync() => cachedTask.Value;

And I dare say this isn't exactly what most people will do in practice, so why should Moq?

And a third reason specific to SequenceExtensions.ThrowsAsync: Come to think of it, it doesn't even matter whether the same or a new Task is returned upon every invocation, because with a sequence, one ThrowsAsync configuration will only affect one single invocation; then the sequence moves on to the next configured step. So I was wrong in originally suggesting the Lazy business, it isn't required.

So I think I'd be OK with a breaking change here, as long as this is explicitly marked in the changelog entry, e.g.:

#### Changed

* **Breaking change:** `setup.ReturnsAsync(..., TimeSpan)` and `setup.ThrowsAsync(..., TimeSpan)` now return a different `Task` on each invocation, which wasn't previously the case (@snrnats, #nnn) 

So you'd have to find out exactly which methods are affected, and mention them all in the changelog.

[stakx]

Looks good! I'd like to request only two minor changes (line spacing, and simplifying the changelog entry).

Please allow me one comment, IMHO it would've been better to create two separate PRs, one for each issue. But there's no need now to re-do all the work you've done already; I'm fine with having one single PR that closes both issues.

Thanks for your great work so far! 👍

[stakx]

Please insert a blank line above this one (i.e. between #endregion and #region 593).

[stakx]

I was almost going to suggest that you rewrite (but please don't!) the above 3 lines to the following:

return Task.Delay(delay).ContinueWith<TResult>(task => throw exception);

to make it more similar to the other implementations; but then I realised that this would result in a needlessly more complex exception stack trace. So better leave it as it is. :-)

[stakx]

It appears I was under the false impression that those methods that always returned the same Task were in the minority. But this long list suggests that the opposite is true: reused Task results currently appear to be the norm.

In order to make the changelog entry easier to read, may I suggest that we rephrase this changelog entry to something simpler, for example:

• Breaking change: All ReturnsAsync and ThrowsAsync setup methods now consistently return a new Task on each invocation (...)

[snrnats]

I wanted to make two separate PRs. But I wasn't comfortable changing the code when I know that it still has issues:smile:

[stakx]

Looks good! 👍 (Method names in the changelog are missing code formatting (the backticks), but that can be added any time before the next release is published.)

Thanks for the great work!

[stakx]

@snrnats - I just pushed Moq 4.8.3 to NuGet, which includes your changes. Sorry it's taken so long.