Add `ThrowsAsync` for non-generic `ValueTask` by johnthcall · Pull Request #1235 · devlooped/moq

johnthcall · 2022-02-17T19:23:26Z

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 · 2022-02-17T23:24:53Z

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 · 2022-02-17T23:29:18Z

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

cyungmann · 2022-03-09T16:04:24Z

@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 · 2022-05-16T07:53:41Z

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 · 2022-07-13T20:45:16Z

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

stakx · 2023-01-03T00:34:50Z

@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 · 2023-01-03T17:56:31Z

All committers have signed the CLA.

johnthcall · 2023-01-03T17:59:59Z

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

stakx · 2023-01-03T18:52:43Z

@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.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [Moq](https://togithub.com/moq/moq) | nuget | minor | `4.18.4` -> `4.20.0` | --- ### Release Notes <details> <summary>moq/moq (Moq)</summary> ### [`v4.20.0`](https://togithub.com/moq/moq/releases/tag/v4.20.0)  #### What's Changed ##### ✨ Implemented enhancements - Add `setup.Verifiable(Times times, [string failMessage])` method by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1319](https://togithub.com/moq/moq/pull/1319) ##### 🔨 Other - Add `Mock<T>.RaiseAsync` by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1313](https://togithub.com/moq/moq/pull/1313) - Add `ThrowsAsync` for non-generic `ValueTask` by [@johnthcall](https://togithub.com/johnthcall) in [https://github.com/moq/moq/pull/1235](https://togithub.com/moq/moq/pull/1235) - Use PackageLicenseExpression instead of PackageLicenseUrl by [@wismann](https://togithub.com/wismann) in [https://github.com/moq/moq/pull/1322](https://togithub.com/moq/moq/pull/1322) - Don't throw away generic type arguments in one `mock.Protected().Verify<T>()` method overload by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1325](https://togithub.com/moq/moq/pull/1325) - [#1340](https://togithub.com/moq/moq/issues/1340) updated appveyor.yml with workaround to make builds work again by [@david-kalbermatten](https://togithub.com/david-kalbermatten) in [https://github.com/moq/moq/pull/1346](https://togithub.com/moq/moq/pull/1346) - Revamp structure, apply oss template, cleanup projects/imports by [@kzu](https://togithub.com/kzu) in [https://github.com/moq/moq/pull/1358](https://togithub.com/moq/moq/pull/1358) - Add 💜 SponsorLink support by [@kzu](https://togithub.com/kzu) in [https://github.com/moq/moq/pull/1363](https://togithub.com/moq/moq/pull/1363) - fix website url by [@tibel](https://togithub.com/tibel) in [https://github.com/moq/moq/pull/1364](https://togithub.com/moq/moq/pull/1364) #### New Contributors - [@johnthcall](https://togithub.com/johnthcall) made their first contribution in [https://github.com/moq/moq/pull/1235](https://togithub.com/moq/moq/pull/1235) - [@wismann](https://togithub.com/wismann) made their first contribution in [https://github.com/moq/moq/pull/1322](https://togithub.com/moq/moq/pull/1322) - [@david-kalbermatten](https://togithub.com/david-kalbermatten) made their first contribution in [https://github.com/moq/moq/pull/1346](https://togithub.com/moq/moq/pull/1346) - [@dependabot](https://togithub.com/dependabot) made their first contribution in [https://github.com/moq/moq/pull/1360](https://togithub.com/moq/moq/pull/1360) **Full Changelog**: moq/moq.spikes@v4.18.4...v4.20.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Testably/Testably.Architecture.Rules).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [Moq](https://togithub.com/moq/moq) | nuget | minor | `4.18.4` -> `4.20.0` | --- ### Release Notes <details> <summary>moq/moq (Moq)</summary> ### [`v4.20.0`](https://togithub.com/moq/moq/releases/tag/v4.20.0)  #### What's Changed ##### ✨ Implemented enhancements - Add `setup.Verifiable(Times times, [string failMessage])` method by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1319](https://togithub.com/moq/moq/pull/1319) ##### 🔨 Other - Add `Mock<T>.RaiseAsync` by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1313](https://togithub.com/moq/moq/pull/1313) - Add `ThrowsAsync` for non-generic `ValueTask` by [@johnthcall](https://togithub.com/johnthcall) in [https://github.com/moq/moq/pull/1235](https://togithub.com/moq/moq/pull/1235) - Use PackageLicenseExpression instead of PackageLicenseUrl by [@wismann](https://togithub.com/wismann) in [https://github.com/moq/moq/pull/1322](https://togithub.com/moq/moq/pull/1322) - Don't throw away generic type arguments in one `mock.Protected().Verify<T>()` method overload by [@stakx](https://togithub.com/stakx) in [https://github.com/moq/moq/pull/1325](https://togithub.com/moq/moq/pull/1325) - [#1340](https://togithub.com/moq/moq/issues/1340) updated appveyor.yml with workaround to make builds work again by [@david-kalbermatten](https://togithub.com/david-kalbermatten) in [https://github.com/moq/moq/pull/1346](https://togithub.com/moq/moq/pull/1346) - Revamp structure, apply oss template, cleanup projects/imports by [@kzu](https://togithub.com/kzu) in [https://github.com/moq/moq/pull/1358](https://togithub.com/moq/moq/pull/1358) - Add 💜 SponsorLink support by [@kzu](https://togithub.com/kzu) in [https://github.com/moq/moq/pull/1363](https://togithub.com/moq/moq/pull/1363) - fix website url by [@tibel](https://togithub.com/tibel) in [https://github.com/moq/moq/pull/1364](https://togithub.com/moq/moq/pull/1364) #### New Contributors - [@johnthcall](https://togithub.com/johnthcall) made their first contribution in [https://github.com/moq/moq/pull/1235](https://togithub.com/moq/moq/pull/1235) - [@wismann](https://togithub.com/wismann) made their first contribution in [https://github.com/moq/moq/pull/1322](https://togithub.com/moq/moq/pull/1322) - [@david-kalbermatten](https://togithub.com/david-kalbermatten) made their first contribution in [https://github.com/moq/moq/pull/1346](https://togithub.com/moq/moq/pull/1346) - [@dependabot](https://togithub.com/dependabot) made their first contribution in [https://github.com/moq/moq/pull/1360](https://togithub.com/moq/moq/pull/1360) **Full Changelog**: moq/moq.spikes@v4.18.4...v4.20.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/Testably/Testably.Abstractions).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

johnthcall closed this Feb 17, 2022

stakx reopened this May 16, 2022

stakx changed the title ~~Add throwsAsync for Non-Generic Task and ValueTask~~ Add ThrowsAsync for non-generic Task and ValueTask May 17, 2022

Only ValueTask ThrowsAsync

9014fef

johnthcall force-pushed the johncall/ThrowsAsyncNonGenericDelay branch from 6a5c6e8 to 9014fef Compare January 3, 2023 17:56

merge with main

76f92db

stakx approved these changes Jan 3, 2023

View reviewed changes

stakx changed the title ~~Add ThrowsAsync for non-generic Task and ValueTask~~ Add ThrowsAsync for non-generic ValueTask Jan 3, 2023

stakx merged commit 047de59 into devlooped:main Jan 3, 2023

devlooped locked and limited conversation to collaborators Sep 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `ThrowsAsync` for non-generic `ValueTask`#1235

Add `ThrowsAsync` for non-generic `ValueTask`#1235
stakx merged 2 commits intodevlooped:mainfrom
johnthcall:johncall/ThrowsAsyncNonGenericDelay

johnthcall commented Feb 17, 2022

Uh oh!

stakx commented Feb 17, 2022 •

edited

Loading

Uh oh!

johnthcall commented Feb 17, 2022

Uh oh!

cyungmann commented Mar 9, 2022

Uh oh!

stakx commented May 16, 2022 •

edited

Loading

Uh oh!

johnthcall commented Jul 13, 2022

Uh oh!

stakx commented Jan 3, 2023 •

edited

Loading

Uh oh!

CLAassistant commented Jan 3, 2023 •

edited

Loading

Uh oh!

johnthcall commented Jan 3, 2023

Uh oh!

stakx commented Jan 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

johnthcall commented Feb 17, 2022

Uh oh!

stakx commented Feb 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnthcall commented Feb 17, 2022

Uh oh!

cyungmann commented Mar 9, 2022

Uh oh!

stakx commented May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnthcall commented Jul 13, 2022

Uh oh!

stakx commented Jan 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Jan 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnthcall commented Jan 3, 2023

Uh oh!

stakx commented Jan 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stakx commented Feb 17, 2022 •

edited

Loading

stakx commented May 16, 2022 •

edited

Loading

stakx commented Jan 3, 2023 •

edited

Loading

CLAassistant commented Jan 3, 2023 •

edited

Loading