Make TestMethodReflectionInfo.Invoke async-aware (Task-returning) in MSTest.AotReflection.SourceGeneration#9013
Draft
Evangelink wants to merge 7 commits into
Conversation
Adds a focused unit-test project for the AotReflection source generator PoC introduced in microsoft#8574. The PoC had no test coverage until now. Coverage highlights (13 tests): - Support types emission (TestClassReflectionInfo, TestMethodReflectionInfo, TestPropertyReflectionInfo, TestConstructorReflectionInfo). - Registry emission shape and namespace (MSTest.SourceGenerated). - Empty registry when no [TestClass] is present. - Skipping of static / abstract / open-generic test classes. - Constructor invoker, parameter types / names, async return shape. - Class-level attribute capture; property getter & setter delegate text. - Compile-clean snapshot (catches CS errors the generator may introduce). - Incrementality: support-types step is cached when input is unchanged. Also: - Adds MSTest.AotReflection.SourceGeneration to TestFx.slnx and MSTest.slnf (missing since microsoft#8574). - Adds [InternalsVisibleTo] for the new test project (generator class is internal sealed). Part of microsoft#1837. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…8639)
The PoC's TestClassModelBuilder built its fully-qualified type names with SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier, then fed the resulting FQN into both casts (where '?' is harmless and merely cosmetic, since the emitted setter already uses 'value!') and 'typeof(...)' expressions (where '?' on a reference type is invalid C# and produces CS8639).
Removing the flag fixes 'typeof(string?)' / 'typeof(MyRef?)' while preserving 'typeof(int?)' (nullable value types are rendered via UseSpecialTypes, which is unaffected).
Adds a focused regression test that runs the Roslyn compiler over the generated source and asserts both the textual shape ('typeof(global::Sample.TestContext)', 'typeof(string)', 'typeof(int?)') and the absence of any compile errors.
Discovered while building tests for microsoft#8574; depends on microsoft#9004 for the test infrastructure.
Part of microsoft#1837.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Today TestClassModelBuilder only enumerates members directly declared on the `[TestClass]` type. As soon as a fixture extends a base class, MSTest members (`[TestInitialize]`, `[TestCleanup]`, `[TestMethod]`, the `[TestContext]` property, ...) declared on the base disappear from the generated registry. This change makes the builder walk the inheritance chain (stopping at `System.Object`): * Methods and properties are folded from base types into the model. * Iteration is derived-first; an override or `new`-shadowed member with the same signature/name wins over the base declaration. The signature key includes ref-kinds so genuine overloads survive. * Attributes are collected across the `OverriddenMethod` / `OverriddenProperty` chain (deduped by attribute class FQN) so an `override` that does not re-apply `[TestMethod]` still sees the base attribute - matching the runtime `GetCustomAttributes(inherit: true)` semantics. * Accessibility is broadened to include `Protected` / `ProtectedOrInternal` / `ProtectedAndInternal` so abstract bases can expose their hooks to the emitted code (which lives in the consumer's assembly). * Constructors are NEVER inherited (only taken from the leaf type). Adds 9 new tests covering: inherited methods, multi-level inheritance, overridden virtual (attribute inheritance + de-dup), `new`-hidden methods, overload preservation, inherited properties, no inherited constructors, abstract-base fold-down, and not walking past `System.Object`. Part of microsoft#1837. Depends on microsoft#9004 (test project), microsoft#9005 (typeof nullable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds an AssemblyAttributes property to the emitted MSTestReflectionMetadata registry containing all attributes declared with [assembly: ...] in the same compilation. The attribute payload is built via the existing AttributeApplicationModel pipeline (reused from class/method attribute emission), so adapters can iterate without calling Assembly.GetCustomAttributes at runtime. Part of microsoft#1837. Stacked on microsoft#9004, microsoft#9005, microsoft#9006. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Part of microsoft#1837. Adds compile-time materialization of `[DataRow]` attribute applications on `[TestMethod]` members. The generator now emits a `DataRows` property on each `TestMethodReflectionInfo` containing a flat `IReadOnlyList<object?[]>` mirroring the runtime shape of `DataRowAttribute.Data`, so a consumer can iterate parameterised cases without re-reading the attributes via reflection. Highlights: - New `DataRowModel(EquatableArray<TypedConstantModel> Arguments)` capturing one row of arguments per attribute application. - `BuildDataRows` detects `Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute` applications and flattens the variadic `params object?[] moreData` tail back into the row so the emitted array matches `DataRowAttribute.Data` rather than preserving a nested array. - Inheritance-aware: reuses the inherited attribute walk introduced in microsoft#9006, so `[DataRow]` applied on a base method (when the override is virtual) is still picked up. - Emitter always emits `DataRows` (empty array for non-data-driven tests) for shape parity with the other `TestMethodReflectionInfo` properties. Deferred to a follow-up: `[DynamicData]` materialization. Resolving the data source method/property/field at compile time is materially more complex (handles `Method`/`Property`/`Field`/`AutoDetect` source kinds plus `object[]` / `IEnumerable<object[]>` return shapes) and warrants its own PR. Tests: - `Generator_EmitsEmptyDataRows_WhenMethodHasNoDataRow` - `Generator_CapturesSingleDataRow_WithScalarArgs` - `Generator_CapturesMultipleDataRows_InDeclarationOrder` - `Generator_FlattensParamsArrayInDataRow` - `Generator_HandlesNullValueInDataRow` Total: 32/32 passing. Depends on microsoft#9004, microsoft#9005, microsoft#9006, microsoft#9007. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Part of microsoft#1837. Changes the emitted `TestMethodReflectionInfo.Invoke` delegate from `Func<object?, object?[]?, object?>` to `Func<object?, object?[]?, Task>` so the caller can `await` a single Task regardless of the underlying test method's signature. Per return shape: - `void` / non-Task sync: `{ call; return Task.CompletedTask; }` — no allocation, no extra wrapping. - `Task` / `Task<T>`: `{ Task? __t = call; return __t ?? Task.CompletedTask; }` — forward the Task; tolerate a misbehaving test returning `null` rather than NRE. - `ValueTask` / `ValueTask<T>`: `{ var __vt = call; return __vt.IsCompletedSuccessfully ? Task.CompletedTask : __vt.AsTask(); }` — fast path for synchronous completion skips `AsTask()` allocation. - Non-void non-Task sync (e.g. `int Test()`): `{ _ = call; return Task.CompletedTask; }` — value discarded, side effects retained. Support type updated to declare `Invoke` as `Func<object?, object?[]?, Task>` with default `static (_, _) => Task.CompletedTask`. Both the support-types file and the registry file now import `System.Threading.Tasks`. Also drops a duplicate blank line left over from PR-A4 (SA1507). Tests: - `Generator_SupportType_DeclaresInvokeAsTaskReturning` - `Generator_InvokerForVoidMethod_ReturnsCompletedTask` - `Generator_InvokerForTaskMethod_ForwardsTask` - `Generator_InvokerForTaskOfTMethod_ForwardsTask` - `Generator_InvokerForValueTaskMethod_UnwrapsViaAsTask` - `Generator_InvokerForValueTaskOfTMethod_UnwrapsViaAsTask` - `Generator_InvokerForNonVoidSyncMethod_DiscardsResultAndReturnsCompletedTask` - `Generator_EmittedRegistry_ImportsSystemThreadingTasks` Total: 40/40 passing (1 existing test updated for new shape). Depends on microsoft#9004, microsoft#9005, microsoft#9006, microsoft#9007, microsoft#9011. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the MSTest.AotReflection.SourceGeneration PoC generator to emit a Task-returning TestMethodReflectionInfo.Invoke delegate so consumers can await a single, uniform invoker regardless of whether the underlying test method returns void, Task, Task<T>, ValueTask, ValueTask<T>, or a non-Task sync value. It also expands the emitted metadata shape (notably DataRows) and adds/updates unit tests to lock in the new emission contract as part of the broader NativeAOT support work (#1837).
Changes:
- Change emitted
TestMethodReflectionInfo.InvokefromFunc<object?, object?[]?, object?>-style toFunc<object?, object?[]?, Task>and update registry/support-type emissions accordingly. - Emit
DataRowsonTestMethodReflectionInfoby materializing[DataRow]constructor arguments at compile time. - Add/extend unit test coverage and wire the generator + new test project into solution filters.
Show a summary per file
| File | Description |
|---|---|
| TestFx.slnx | Adds the generator project and its unit test project to the full solution. |
| MSTest.slnf | Adds the generator project to the MSTest solution filter. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/Program.cs | Test runner entrypoint for the new unit test project. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs | Adds/updates tests validating Task-returning invokers, DataRows, and registry imports. |
| test/UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests/MSTest.AotReflection.SourceGeneration.UnitTests.csproj | New unit test project referencing the generator and Roslyn. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/MSTest.AotReflection.SourceGeneration.csproj | Grants unit tests access to internal generator types via InternalsVisibleTo. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Model/TestClassModel.cs | Extends models to include DataRowModel and AssemblyMetadataModel; adds DataRows to TestMethodModel. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/TestClassModelBuilder.cs | Builds inheritance-aware method/property models; collects inherited attributes; materializes [DataRow] values. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/MSTestReflectionMetadataGenerator.cs | Adds assembly-level attribute capture branch and updates registry emission payload. |
| src/Analyzers/MSTest.AotReflection.SourceGeneration/Generators/MetadataRegistryEmitter.cs | Emits Task-returning invokers, adds DataRows support type/property, and imports System.Threading.Tasks. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1837
Changes the emitted
TestMethodReflectionInfo.Invokedelegate fromFunc<object?, object?[]?, object?>toFunc<object?, object?[]?, Task>so the caller canawaita single Task regardless of the underlying test method's signature, instead of having to type-test the returnedobject?.Per return shape
void/ non-Task sync{ call; return Task.CompletedTask; }Task/Task<T>{ Task? __t = call; return __t ?? Task.CompletedTask; }ValueTask/ValueTask<T>{ var __vt = call; return __vt.IsCompletedSuccessfully ? Task.CompletedTask : __vt.AsTask(); }int Test()){ _ = call; return Task.CompletedTask; }Notable design points:
Taskpath tolerates a misbehaving test returningnullrather thanNullReferenceException-ing in the framework.ValueTaskpath preserves the synchronous-completion fast path viaIsCompletedSuccessfully, only allocating aTaskwhen the operation actually went async.Support type updated to declare
InvokeasFunc<object?, object?[]?, Task>with defaultstatic (_, _) => Task.CompletedTask. Both the support-types file and the registry file now importSystem.Threading.Tasks.Also drops a duplicate blank line left over from PR-A4 (SA1507).
Tests
Generator_SupportType_DeclaresInvokeAsTaskReturningGenerator_InvokerForVoidMethod_ReturnsCompletedTaskGenerator_InvokerForTaskMethod_ForwardsTaskGenerator_InvokerForTaskOfTMethod_ForwardsTaskGenerator_InvokerForValueTaskMethod_UnwrapsViaAsTaskGenerator_InvokerForValueTaskOfTMethod_UnwrapsViaAsTaskGenerator_InvokerForNonVoidSyncMethod_DiscardsResultAndReturnsCompletedTaskGenerator_EmittedRegistry_ImportsSystemThreadingTasksTotal: 40/40 passing (1 existing test updated for new shape).
Dependencies
Stacked on top of:
typeofnullable annotation[DataRow]materialization