Add ThrowsAsync for non-generic ValueTask

[johnthcall]

There are scenarios where interface returning Task or ValueTask should mimic an exception with or without a delay. This change aligns the ThrowsAsync for Task and ValueTask with Task and ValueTask .

[stakx]

One of the goals of #1126 was to no longer have to maintain a separate bunch of {Returns,Throws}Async methods. You can now set up a task's .Result and use the regular Returns, Throws methods. Or, in the case of delayed exception throwing, why not simply do this:

mock.Setup(m => m.FooAsync()).Returns(async () =>
{
    await Task.Delay(...);
    throw new BarException(...);
});

I'm not sure I see the point of adding any further specialized methods if the same thing can already be easily done using the existing methods.

[johnthcall]

Thanks @stakx, hadn't seen this change, will abandon this!

[cyungmann]

@stakx what is the best way to setup throwing an exception for a non-generic ValueTask returning method, where there is no .Result property to call?

[stakx]

Sorry for the late reply.

what is the best way to setup throwing an exception for a non-generic ValueTask returning method, where there is no .Result property to call?

Gotcha. Right now, perhaps the "easiest" way to set up a non-generic task to throw once await-ed would be:

mock.Setup(m => m.DoSomethingAsync()).Returns(async () => throw ...);

But obviously that isn't very nice.

I think I previously made an error in reasoning where I thought that for non-generic (i.e. "void") tasks, it wouldn't make a difference whether you set up the task's result vs. setting up the method's return value. I was at the time thinking about the distinction between .Callback / .CallbackAsync, where there usually wouldn't be any discernible difference anyway, as tasks would start executing right away in both cases. Worse, having a .CallbackAsync would be confusing because it might suggest that the callback only starts executing once the task returned from the method is await-ed. From there, it seems I erroneously jumped to the conclusion that the same would be true for .Throws / .ThrowsAsync, and that therefore the latter wouldn't be needed either.

I can see now that perhaps we do need .ThrowsAsync, at least for the time being, until we can have something that fits the new .Result-style setup method nicely. (I was previously considering a special Await-like operator for setup expressions, see #1008 and #1123).

I'll try to look into this soon.

[johnthcall]

Hey @stakx, any update here, it would be great to use this functionality.

[stakx]

@johnthcall, sorry for the very long radio silence. I needed to take some time off programming-related activities in my spare time due to my daytime work. I'm now going through everything that's accumulated since.

I'd be happy to merge the public static IReturnsResult<TMock> ThrowsAsync<TMock>(this IReturns<TMock, ValueTask> mock, Exception exception) method overload, but I do not feel comfortable about the method overloads accepting TimeSpans.

• I'm not sure that time-dependent (test) code should be encouraged, and thus I'm hesitant about adding any more of that to the Moq core library than what we've already got. (I don't think we should necessarily fall for the sunk cost fallacy here.)
• I don't really like the ReturnsAsync / MethodAsync overloads that are already there. They may have made sense before we had async / await, but nowadays that code is so easy to write yourself: e.g. mock.Setup(...).Returns(async () => { await Task.Delay(...); throw new Exception(...); }). At the same time, they add a ton of repetition to the library that we'll have to maintain.
• There's some details about the way those existing methods have been written that I don't like either. E.g. how they call the first argument mock when they're really extensions to a setup. Or how they spawn some detached tasks to set a TaskCompletionSource<>'s result after a delay. I think at the very least, all those methods should now be re-written in a more straightforward manner using async / await.

Any chance that you would be willing to reduce your PR to just that first method overload, and drop all the TimeSpan ones? (If so, you'd also need to rebase the PR on top of current main.)

[CLAassistant]

All committers have signed the CLA.

[johnthcall]

@stakx I've removed everything but the public static IReturnsResult<TMock> ThrowsAsync<TMock>(this IReturns<TMock, ValueTask> mock, Exception exception)

[stakx]

@johnthcall, thank you, looks good to merge. It looks like the project wants you to sign that CLA since this is your first submission, so I'll wait & let you take care of that.