fix(source-gen): adapt ValueTask<T> test results

[zion-sati]

Generated test invokers return a non-generic ValueTask, but the source generator currently classifies ValueTask<T> as the same return shape and emits it directly. The generated project then fails to compile because ValueTask<T> cannot be returned as ValueTask.

Split the two return patterns and adapt ValueTask<T> through its underlying task. The regression covers ordinary and generic generated test methods, updates snapshots for every supported target framework, and executes both paths.

This was found while porting TUnit to NetWasm, a C# to WebAssembly compiler.

Validation:

• dotnet test tests/TUnit.Core.SourceGenerator.Tests/TUnit.Core.SourceGenerator.Tests.csproj -f net10.0 --treenode-filter '/*/*/BasicTests/Test'
• dotnet test tests/TUnit.TestProject/TUnit.TestProject.csproj -f net10.0 --treenode-filter '/*/*/BasicTests/*'
• dotnet test tests/TUnit.TestProject/TUnit.TestProject.csproj -f net10.0 --treenode-filter '/*/*/BasicTests/*' -- --reflection

The only other ValueTask prefix classification in the core generator selects generic data-source factories and is intentionally generic.

Summary by CodeRabbit

• Bug Fixes
  • Tests returning ValueTask<T> are now invoked correctly, including generic test methods.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3364d1dd-7800-40c5-83f4-c084d31b0179

📥 Commits

Reviewing files that changed from the base of the PR and between c73655f and 410b69b.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1b7711f6-1154-4e43-b661-2dc15f7813f1

📥 Commits

Reviewing files that changed from the base of the PR and between 7014edf and c73655f.

📒 Files selected for processing (6)

• tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.Test.DotNet10_0.verified.txt
• tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.Test.DotNet8_0.verified.txt
• tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.Test.DotNet9_0.verified.txt
• tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.Test.Net4_7.verified.txt
• tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.cs
• tests/TUnit.TestProject/ValueTaskResultTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The source generator now distinguishes ValueTask<T> from non-generic ValueTask and wraps generic results for generated invokers. Tests cover regular and generic test methods, with generated output verified across target frameworks.

Changes

ValueTask return handling

Layer / File(s)	Summary
Classify and wrap ValueTask returns src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs	The generator classifies ValueTask<T> separately and converts its result to a non-generic ValueTask in generated invokers.
Cover generic ValueTask tests tests/TUnit.TestProject/ValueTaskResultTests.cs, tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.cs, tests/TUnit.Core.SourceGenerator.Tests/ValueTaskResultTests.Test.*.verified.txt	Test declarations and generated-source verification cover regular and generic methods returning ValueTask<int>.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c7365

The change fixes generated invokers for ValueTask tests without changing bare ValueTask handling. No actionable merge-blocking risk remains; merge after normal checks pass.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (4 skipped: 4…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adapting ValueTask test results in the source generator.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (4 skipped: 4 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{I’m a rabbit with a task to run,
ValueTask&lt;int&gt; now joins the fun.
Generic hops and wrappers flow,
Generated tests confirm the show.
I nibble clover, then call it done!}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Review

The fix is correct and minimal. ValueTask<T> is now its own TestReturnPattern, and the generated invoker adapts it to the non-generic ValueTask that the invoker signature requires. Both emit sites are updated: EmitConcreteInvokeBody and GenerateReturnHandling. Tightening the ValueTask check to an exact match, with StartsWith("...ValueTask<") for the generic case, removes the old ambiguity. The regression tests cover both an ordinary method and a generic method, and the snapshots are updated.

I did not build or run the tests, and the code-review skill failed, so I read the diff by hand.

Suggestions, none of them blocking:

1. Allocation. new ValueTask(x.AsTask()) allocates a Task<T> (or reuses a cached one) even when the ValueTask<T> is already completed. AGENTS.md asks for minimal allocations on execution paths. A small helper such as ValueTaskHelpers.AsValueTask<T>(ValueTask<T>) in TUnit.Core would avoid that. It could check IsCompletedSuccessfully and return default, and otherwise fall back to new ValueTask(vt.AsTask()). The generator would then emit one call and skip the inline expression. This also keeps the generated code smaller across many tests. If the helper is too much for this PR, a follow-up is fine.
2. Exceptions from a completed-faulted ValueTask<T>. AsTask() preserves the fault, so behaviour is correct. A helper following (1) would need to handle the faulted case explicitly.
3. Other emitters. Please confirm no other generator, such as hooks or data sources, has its own ValueTask StartsWith classification with the same bug. The PR only touches TestMetadataGenerator. A grep for StartsWith("System.Threading.Tasks.ValueTask") in src/TUnit.Core.SourceGenerator would settle it.
4. Reflection mode. AGENTS.md requires the source-generator and engine metadata paths to behave identically. The engine's reflection invoker probably already handles ValueTask<T> through the runtime, and the tests in TUnit.TestProject exercise it. Running the same BasicTests filter with the reflection mode enabled would confirm that.

Overall this looks good to merge once (3) is confirmed.

[greptile-apps]

Confidence Score: 5/5

[Medium risk] Fixes code generation for ValueTask-returning test methods.

The PR appears safe to merge; no outstanding findings were identified.

Summary

The PR distinguishes ValueTask<T> from non-generic ValueTask and adapts generated invokers to return a non-generic ValueTask.

• Adds source-generation snapshots for four target frameworks and execution fixtures for ordinary and generic test methods.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Test method returns ValueTask of T] --> B[Generator selects ValueTaskOfT]
  B --> C[Generated invoker calls AsTask]
  C --> D[Wraps Task in non-generic ValueTask]
  D --> E[Test engine awaits invocation]

Loading

_{Reviews (3) · Last reviewed commit: "Fix generated invokers for ValueTask<T> ..."}

[thomhurst]

@zion-sati Can we just add more validation in the analyzer to prevent users from defining test methods like this?

[thomhurst]

Actually I'm fine with this since we support a similar pattern for Task

[github-actions]

Review: LGTM.

The fix is correct and minimal. ValueTask<T> is split from ValueTask, so the previous prefix match StartsWith("...ValueTask") no longer sends both to the direct-return path. ValueTask<T> is adapted with new ValueTask(x.AsTask()), which compiles. The exact-match check on ValueTask and the ValueTask< prefix check can't overlap. The new tests cover an ordinary method and a generic method, and the snapshots are updated.

I could not run the code-review skill (it errored), so this is a manual read of the diff and I did not build or run the tests.

Minor, non-blocking points:

• AsTask() on a synchronously completed ValueTask<T> is allocation-free only when it wraps an existing Task. Otherwise it allocates a Task<T>. That is acceptable for the invoker path, but a small cached helper such as static async ValueTask Adapt<T>(ValueTask<T> v) => await v would avoid the generated code doing this per call. I would not block the PR on it.
• Please confirm the same ValueTask<T> handling exists for other emitters (for example hooks) that use return-pattern classification. The description says the only other prefix match is intentionally generic, so this is probably already covered.

Thanks for the fix and the regression coverage.

[github-actions]

Review: LGTM, with one optional suggestion.

The fix is correct. ValueTask<T> was previously classified with ValueTask and returned directly, which can't compile against a non-generic ValueTask return type. Splitting out ValueTaskOfT and adapting it in both EmitConcreteInvokeBody and GenerateReturnHandling covers both emit sites. Tightening the check to exact == "System.Threading.Tasks.ValueTask" plus StartsWith("...ValueTask<") is right. It also avoids matching unrelated types that merely start with ValueTask.

The regression tests cover an ordinary method and a generic method, with snapshots for all four TFMs. The TestProject tests actually execute both paths, which is what you want.

Optional suggestion: allocation on the hot path. .AsTask() on a ValueTask<T> that has already completed synchronously allocates a Task<T>. That cuts against the repo's guidance to minimize allocations in execution paths. A cheap alternative is a small static helper in TUnit.Core, for example static async ValueTask Await<T>(ValueTask<T> vt) => await vt.ConfigureAwait(false);. It could also short-circuit with if (vt.IsCompletedSuccessfully) { _ = vt.Result; return default; }. The generator would then emit return Helper.Await({methodCall});. That keeps the sync-completed case allocation-free and would also give one place to change if the adaptation ever needs to be different. Since test methods are usually invoked once per test, this isn't a blocker.

Nothing else to flag.

[thomhurst]

Thanks @zion-sati