feat(mocks): accept an async factory in Returns() on async members - #6503
#6503feat(mocks): accept an async factory in Returns() on async members#6503thomhurst merged 3 commits intomainthomhurst/TUnit:mainfrom fix/6495-async-returns-factorythomhurst/TUnit:fix/6495-async-returns-factoryCopy head branch name to clipboard
Conversation
Returns() on an async member only offered the synchronous Func<T>
overload, so `.Returns(async () => { await Task.Delay(x); return y; })`
failed with CS4010. The only shape that compiled was a blocking
Thread.Sleep, which returns an already-completed task to the caller —
so production code racing the call against a timeout could never let the
timeout win, making that whole class of test unwritable.
Emit a Returns overload alongside each existing ReturnsAsync factory
overload, parameterless and typed-parameter alike, routing through the
same ReturnsRaw path. The factory's task is handed back as-is and stays
pending until it completes.
Fixes #6495
Greptile SummaryThis PR adds async-factory
Confidence Score: 3/5The PR is not yet safe to merge because the reported Both new overload forms are enclosed in Files Needing Attention: src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs, tests/TUnit.Mocks.Tests/Issue6495Tests.cs
|
| Filename | Overview |
|---|---|
| src/TUnit.Mocks.SourceGenerator/Builders/MockMembersBuilder.cs | Adds the async-factory aliases with ambiguity protection, but conditionally removes the advertised feature from .NET 8 generated mocks. |
| tests/TUnit.Mocks.Tests/Issue6495Tests.cs | Covers pending-task behavior, values, invocation frequency, typed factories, and overload binding on .NET 9+, while leaving the new API untested and unavailable on .NET 8. |
| tests/TUnit.Mocks.SourceGenerator.Tests/Snapshots/Interface_With_Async_Methods.verified.txt | Confirms the generated overload shapes and their .NET 9 conditional compilation boundaries. |
Reviews (3): Last reviewed commit: "fix(mocks): gate the async Returns alias..." | Re-trigger Greptile
Code reviewNice, focused fix — routing the new Confirmed issue: the new overload is source-breaking for reference-typed async results
When public Test Returns(Func<IDialogReference?> factory) { ... }
public Test Returns(Func<Task<IDialogReference?>> factory) { ... }
...
t.Returns(() => null); // CS0121: ambiguous
t.Returns(() => throw new InvalidOperationException()); // CS0121: ambiguousBoth are idiomatic mocking patterns (stub a null result, or stub a call that should throw) that existing consumers may already be using against reference-typed async members. This wouldn't be caught by the PR's own test suite because Why this matters architecturally: the collision isn't a one-off bug in this diff, it's a consequence of overloading
Either way, this is worth resolving before merge since it's a compile-breaking regression for existing callers, not just a theoretical edge case — Everything else looks solid
|
…das unambiguous Review finding: for a reference-typed async result, a lambda whose body pins nothing — Returns(() => null), Returns(() => throw ...) — converts equally well to Func<T> and to the new Func<Task<T>>, making previously valid setups CS0121. Reproduced against IReferenceResultService before adding the attribute. [OverloadResolutionPriority(-1)] on the async factory overloads breaks that tie back to the pre-existing synchronous meaning. A genuine async lambda is unaffected: it is not convertible to Func<T> at all, so it remains the only applicable candidate.
|
Both reviews are right — confirmed the ambiguity before fixing it. With the new overloads and no tie-break, three of the new reference-typed cases fail to compile: Fixed in bb4843d by putting
This keeps the fix uniform across all async members instead of restricting it to non-nullable value types, which would have left the reference-typed case — the more common one for a mocked service — still needing
Added to |
Code reviewFollow-up on my previous review: the source-breaking ambiguity I (and Greptile) flagged for Verified the fix:
No remaining issues from either review. This is good to merge as far as the two previously-raised concerns go. |
Review finding: OverloadResolutionPriorityAttribute only reaches a consumer's compilation on net9.0+. TUnit.Mocks polyfills it internally for its own build, so the type is not visible to a net8.0 consumer and the generated attribute would be CS0246 — which is why the framework-operation polyfills in this file already sit behind #if NET9_0_OR_GREATER. The alias is inseparable from the priority (without it, reference-typed results get CS0121 on null/throw lambdas), so the whole overload is now net9.0+. net8.0 keeps ReturnsAsync, which already hands the task back as-is and covers the timeout-race case from the issue. Not reachable by an in-repo test: Directory.Build.props adds Polyfill to every project here, so our own net8.0 build resolves the attribute that an external consumer would not.
| // See issue #6495. | ||
| writer.AppendLine(); | ||
| writer.AppendLine("#if NET9_0_OR_GREATER"); | ||
| writer.AppendLine("/// <summary>Configure a typed computed async return value using the actual method parameters. The returned task is handed back as-is, so an async factory stays pending until it completes.</summary>"); |
There was a problem hiding this comment.
Async Returns remains unavailable on net8
When a .NET 8 consumer configures an async member with Returns(async (...) => ...), this conditional removes both new async-factory overload forms from the generated mock, so the lambda is still considered against the synchronous factory overload and compilation fails with CS4010.
Code reviewFollow-up on the two prior reviews on this PR: the only change since my last comment is What changed and why it's right:
No remaining issues. Good to merge as far as all three reviews (Greptile's and my two) go. |
…factories Rebase conflict resolution: #6503 added the Func<Task<T>> Returns overloads to the same generated call-wrapper types this branch moved into TUnit.Mocks.Generated. Regenerated rather than hand-merged.
…ce (#6504) * fix(mocks): emit the setup surface into the globally-imported namespace Setup, verify and event extensions were generated beside the mocked type. An extension member is only visible when its namespace is imported, so mocking a type whose namespace the test had not `using`'d compiled the Mock() call — that entry point already lives in TUnit.Mocks — but hid every member setup behind CS1061, which reads as "this member isn't supported". Reported as a name collision between two same-named ICluster interfaces; it reproduces with a single mock and no collision involved. Emit the extension classes and the call wrappers they return into TUnit.Mocks.Generated, which TUnit.Mocks.targets adds as a global using. Their names are derived from the fully qualified type name, so types sharing a short name across namespaces stay distinct. The impl, factory and wrapper types keep their current placement, as do the out/ref setter delegates the impl references by short name. Fixes #6494 * test(mocks): snapshot the non-span ref-struct out/ref delegate emission Review nit: no snapshot covered the one construct that emits a namespace-scoped delegate, which is precisely the path this PR split out of the members loop. The new fixture pins both halves — the delegates stay in the mocked type's namespace (the impl references them by short name) while the setup surface moves to TUnit.Mocks.Generated. * test(mocks): refresh snapshots after rebasing onto the async Returns factories Rebase conflict resolution: #6503 added the Func<Task<T>> Returns overloads to the same generated call-wrapper types this branch moved into TUnit.Mocks.Generated. Regenerated rather than hand-merged.
Fixes #6495
Problem
Returnson an async member is generated over the unwrapped return type, so its factory overload isFunc<int>— an async lambda has nowhere to bind. The shape that does compile,Returns(() => { Thread.Sleep(1000); return 42; }), blocks the caller and hands back an already-completed task, soawait Task.WhenAny(mock.SomeMethodAsync(), Task.Delay(timeout))can never let the timeout branch win. That makes timeout-handling tests unwritable — the reporter kept NSubstitute for one test because of it.ReturnsAsync(Func<Task<T>>)already did the right thing, but nothing at theReturnscall site pointed there.Fix
Emit a
Returnsoverload alongside every existingReturnsAsyncfactory overload — the parameterless form inEmitReturnsAsyncOverloadsand the typed-parameter form inGenerateTypedReturnsAsyncOverload— routing through the sameReturnsRawpath. The factory's task is handed back as-is, so it stays pending until it completes.Covers
Task<T>,ValueTask<T>,TaskandValueTaskmembers. The synchronousFunc<T>overload is untouched;ReturnsAsyncis unchanged.Tests
tests/TUnit.Mocks.Tests/Issue6495Tests.cs— 8 cases: the timeout race (asserting the returned task is incomplete and that a 50ms timeout beats it), value propagation, per-call invocation, the typed-parameter form,ValueTask<T>andTaskmembers, plus guards that the synchronousReturnsfactory andReturnsAsyncstill behave as before.TUnit.Mocks.Tests1168 passed. Generator snapshots regenerated (8 files, additive only) and passing on net8.0/net9.0/net10.0. Analyzers 30, Http 54, Logging 31.