Make ReflectionFree the default source-gen mode + harden AOT acceptance tests

[Evangelink]

What

Follow-up to the #9769 investigation (source-gen dogfooding gap).

1. Make ReflectionFree the default MSTestSourceGenMode (was Rooting) in MSTest.TestAdapter.targets.
   • The source generator is only active for Native AOT / trim consumers (PublishAot, or a manual MSTest.SourceGeneration reference) — the ClassicEngine path never references it — so the blast radius is exactly the AOT/trim audience that benefits most.
   • ReflectionFree is a superset of Rooting (materialized attributes + delegate invokers on top of type/method rooting), so it is the safer default for that audience.
   • The SourceGeneration acceptance metadata mode now passes -p:MSTestSourceGenMode=Rooting explicitly so the Rooting path stays covered now that it is no longer the default.
2. Harden NativeAotTests to explicitly set MSTestSourceGenMode=ReflectionFree and corrected the stale comments (the asset previously claimed reflection-free but ran Rooting).

Why not enable warnAsError for reflection-free mode

Investigated per the discussion. The IL2026/IL2072 trim errors in #9769 come from Microsoft.TestPlatform.ObjectModel.TestObject.ConvertPropertyFrom/To<T>, dragged in by MSTest.TestAdapter and reached through the native MTP path (TestingPlatformAdapter/MSTestTestNodeConverter, MSTestFilterContext) which uses the VSTest TestCase/TestProperty types directly. That reachability is independent of MSTestSourceGenMode — source-gen mode only changes user-test metadata acquisition, not the platform↔VSTest plumbing. So a blanket warnAsError cannot pass in either mode until that ObjectModel reachability is removed. That fix is intentionally out of scope here (owned by @Evangelink per the issue) and is the real unblock for the reported strict build.

Pipeline fix (pre-existing, unrelated to the source-gen change)

CI was already red on main before this PR due to two undeclared internal-API surfaces flagged by RS0051. Fixed here in a separate commit so this PR can go green:

• BaseSerializer.ReadFields / WriteListPayload<T> (added by Deduplicate DotnetTest IPC serializer 3-phase boilerplate #9774) — the file is linked into Microsoft.Testing.Platform and the HangDump / Retry / TrxReport / Extensions.MSBuild extensions; each project's InternalAPI.Unshipped.txt needed the declarations.
• PlatformServicesConfigurationAdapter and its members (from Drop MSTest's VSTest bridge dependency on the MTP path #9755) in MSTest.TestAdapter, which only surface on the Windows / net462 TFMs the Linux analysis build never compiled.

Review feedback addressed

• Restored the UTF-8 BOM (utf-8-bom per .editorconfig) on the five .cs files whose BOM was accidentally stripped while editing their leading lines.
• Clarified the rooting-generator comment to cover the unset-property fallback.

Notes

• Comment updates across SourceGenModeHelper, ReflectionMetadataGenerator, MetadataMode, AcceptanceSourceGen, and a source-gen unit test to reflect the new default.

Refs #9769

[Copilot]

Pull request overview

Updates MSTest’s AOT/trim source-generation defaults by making ReflectionFree the shipped default MSTestSourceGenMode, while keeping the Rooting path explicitly covered in acceptance/unit tests and clarifying related documentation/comments across the repo.

Changes:

• Switch default MSTestSourceGenMode in MSTest.TestAdapter.targets from Rooting to ReflectionFree.
• Harden acceptance coverage by explicitly passing/pinning MSTestSourceGenMode for both source-gen variants and Native AOT.
• Refresh analyzer/generator/test comments to reflect the new default and mode-selection behavior.

Show a summary per file

File	Description
test/Utilities/Microsoft.Testing.TestInfrastructure/MetadataMode.cs	Updates enum doc comment to reflect ReflectionFree is now the shipped default for AOT source-gen mode.
test/Utilities/Microsoft.Testing.TestInfrastructure/AcceptanceSourceGen.cs	Ensures both acceptance build variants pass MSTestSourceGenMode explicitly (including Rooting) to preserve coverage independent of product defaults.
test/UnitTests/MSTest.SourceGeneration.UnitTests/MSTestReflectionMetadataGeneratorTests.cs	Clarifies unit-test comments around generator fallback behavior when the property is unset.
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/NativeAotTests.cs	Pins MSTestSourceGenMode=ReflectionFree in the AOT asset and expands rationale for not enabling warnAsError.
src/Analyzers/Shared/SourceGenModeHelper.cs	Updates helper documentation to reflect ReflectionFree is the shipped default (from adapter targets) and that “unset” falls back to rooting behavior.
src/Analyzers/MSTest.SourceGeneration/Generators/ReflectionMetadataGenerator.cs	Updates comment explaining which generator emits under which mode (and should account for the “unset” fallback).
src/Adapter/MSTest.TestAdapter/buildTransitive/common/MSTest.TestAdapter.targets	Changes default MSTestSourceGenMode to ReflectionFree and updates the explanatory comment accordingly.

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 6
• Review effort level: Low

[github-actions]

🔴 Build Failure Analysis

Result	❌ FAILED
Duration	159.4s
Errors	6 (2 distinct)
Warnings	0
Workflow run	View

Root Cause: Missing Internal API declarations (RS0051)

Two new protected static methods on BaseSerializer are not declared in the project's InternalAPI.Unshipped.txt. The internal API analyzer (RS0051) flags them as errors.

Symbol	File	Line
BaseSerializer.ReadFields(Stream, Func<ushort, int, bool>)	BaseSerializer.cs#L359	359
BaseSerializer.WriteListPayload<T>(Stream, ushort, T[]?, Action<Stream, T>)	BaseSerializer.cs#L380	380

Each error is repeated 3× (once per target framework: net8.0, net9.0, netstandard2.0).

⚠️ Not caused by this PR

These methods were introduced by PR #9774 (commit 89174a9 — "Deduplicate DotnetTest IPC serializer 3-phase boilerplate") which merged into main before this PR was rebased. The omission of their API declarations is a pre-existing issue on the base branch.

Suggested Fix

Add the following two lines to src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt:

static Microsoft.Testing.Platform.IPC.Serializers.BaseSerializer.ReadFields(System.IO.Stream! stream, System.Func<ushort, int, bool>! tryReadField) -> void
static Microsoft.Testing.Platform.IPC.Serializers.BaseSerializer.WriteListPayload<T>(System.IO.Stream! stream, ushort fieldId, T[]? list, System.Action<System.IO.Stream!, T>! writeItem) -> void

This can be done either:

1. In this PR (add a fixup commit), or
2. Separately on main via a follow-up fix to PR Deduplicate DotnetTest IPC serializer 3-phase boilerplate #9774.

After the fix, re-running the build should succeed — no other errors or warnings were detected.

🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · 200.4 AIC · ⌖ 6.77 AIC · ⊞ 7.3K · [◷]( · ◷)

[Copilot]

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Medium

[Copilot]

Review details

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Evangelink]

/backport to rel/4.3

[github-actions]

Started backporting to rel/4.3: https://github.com/microsoft/testfx/actions/runs/29034521432

[github-actions]

@Evangelink backporting to rel/4.3 failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Make ReflectionFree the default MSTest source-gen mode
Applying: Harden NativeAotTests to explicitly exercise reflection-free source gen
Applying: Address PR review: restore UTF-8 BOM and clarify rooting-mode comment
Applying: Fix pre-existing RS0051 InternalAPI-tracking breakage on main
Using index info to reconstruct a base tree...
A	src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Unshipped.txt
A	src/Platform/Microsoft.Testing.Extensions.HangDump/InternalAPI/InternalAPI.Unshipped.txt
A	src/Platform/Microsoft.Testing.Extensions.MSBuild/InternalAPI.Unshipped.txt
A	src/Platform/Microsoft.Testing.Extensions.Retry/InternalAPI/InternalAPI.Unshipped.txt
A	src/Platform/Microsoft.Testing.Extensions.TrxReport/InternalAPI/InternalAPI.Unshipped.txt
A	src/Platform/Microsoft.Testing.Platform/InternalAPI/InternalAPI.Unshipped.txt
Falling back to patching base and 3-way merge...
Applying: Remove stale BridgedConfiguration entries from InternalAPI.Shipped.txt
Using index info to reconstruct a base tree...
A	src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Shipped.txt
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Shipped.txt deleted in HEAD and modified in Remove stale BridgedConfiguration entries from InternalAPI.Shipped.txt.  Version Remove stale BridgedConfiguration entries from InternalAPI.Shipped.txt of src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Shipped.txt left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0005 Remove stale BridgedConfiguration entries from InternalAPI.Shipped.txt
Error: The process '/usr/bin/git' failed with exit code 128

NOTE: A PR will be created, but needs to be revised manually!

[github-actions]

Note

🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

Verdict Table

#	Dimension	Verdict
1	Algorithmic Correctness	✅ Clean
2	Threading & Concurrency	N/A
3	Security & IPC Contract Safety	N/A
4	Public API & Binary Compatibility	✅ Clean
5	Performance & Allocations	N/A
6	Cross-TFM Compatibility	✅ Clean
7	Resource & IDisposable Management	N/A
8	Defensive Coding at Boundaries	N/A
9	Error Handling & Diagnostics	N/A
10	Test Coverage & Quality	✅ Clean
11	Naming, Readability & Documentation	✅ Clean
12	Code Style & Conventions	✅ Clean
13	File Encoding & Formatting	✅ Clean
14	MSBuild / Project File Hygiene	✅ Clean
15	Localization	N/A
16	Public API Declaration	N/A
17	Init Accessor Rule	N/A
18	TODO / FIXME Comment Policy	✅ Clean
19	CLI Options & Help Text Consistency	N/A
20	Scope Discipline	✅ Clean
21	Test Infrastructure Correctness	✅ Clean
22	Miscellaneous / Code Health	✅ Clean

Overall Assessment

All clear — no issues found. This is a well-structured PR with correct, self-consistent changes.

Key observations

1. Default change is safe: MSTestSourceGenMode switching from Rooting → ReflectionFree in MSTest.TestAdapter.targets only affects the AOT/trim audience that opts into the source generator. The Condition="'$(MSTestSourceGenMode)' == ''" guard preserves explicit consumer overrides. ReflectionFree is a superset of Rooting, making it the safer default.

2. Generator fallback is correctly documented: SourceGenModeHelper.IsReflectionFree() logic is unchanged — it returns false when the property is unset (no MSBuild targets imported), falling back to rooting. The updated comments across ReflectionMetadataGenerator.cs, SourceGenModeHelper.cs, and the unit test accurately describe this two-layer default (MSBuild default vs. generator fallback).

3. Critical test infrastructure fix in AcceptanceSourceGen.cs: Changing the SourceGeneration variant from string.Empty → " -p:MSTestSourceGenMode=Rooting" is essential — without it, the Rooting test path would silently run ReflectionFree (the new product default) and lose coverage of the Rooting generator. The GetOutputSubFolder switch expression guard prevents MetadataMode.Reflection from reaching this path.

4. NativeAotTests pinning is good practice: Explicitly setting <MSTestSourceGenMode>ReflectionFree</MSTestSourceGenMode> in the test asset makes the test resilient to future default changes while clearly documenting intent.

5. BOM restorations are correct: The five // Copyright line-1 diffs are UTF-8 BOM restorations matching the .editorconfig utf-8-bom rule.

Merge recommendation

✅ No blocking or major issues. The PR is ready to merge once CI completes green.

[Copilot]

Review details

Comments suppressed due to low confidence (1)

src/Adapter/MSTest.TestAdapter/InternalAPI/InternalAPI.Shipped.txt:249

• InternalAPI.Shipped.txt still declares Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBridgedTestFramework, but there are no source references/definitions for this type anywhere in the repo (a full search only finds it in this API file). Leaving stale entries here makes the declared internal surface inaccurate and can trigger RS0052 (“declared API … could not be found”) on analysis builds. Remove the MSTestBridgedTestFramework entries here (and also remove any other MSTestBridgedTestFramework entries elsewhere in the file, e.g. overrides) so the tracked surface matches the current code.

Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBannerCapability
Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBannerCapability.GetBannerMessageAsync() -> System.Threading.Tasks.Task<string?>!
Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBannerCapability.MSTestBannerCapability(Microsoft.Testing.Platform.Services.IPlatformInformation! platformInformation) -> void
Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBridgedTestFramework
Microsoft.VisualStudio.TestTools.UnitTesting.MSTestBridgedTestFramework.MSTestBridgedTestFramework(Microsoft.VisualStudio.TestTools.UnitTesting.MSTestExtension! mstestExtension, System.Func<System.Collections.Generic.IEnumerable<System.Reflection.Assembly!>!>! getTestAssemblies, System.IServiceProvider! serviceProvider, Microsoft.Testing.Platform.Capabilities.TestFramework.ITestFrameworkCapabilities! capabilities) -> void

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Low

[Copilot]

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 0 new
• Review effort level: Medium

[github-actions]

🧪 Test quality grade — PR #9777

Grade	Test	Notes
A (90–100)	mod MSTestReflectionMetadataGeneratorTests. Generator_ EmitsNothing_ InRootingMode	Clear AAA; negative assertions on Diagnostics and GeneratedSources verify generator silence in rooting mode.
A (90–100)	mod NativeAotTests. NativeAotTests_ WillRunWithExitCodeZero	Four assertions span publish output and test execution; pinning MSTestSourceGenMode in the embedded template guards against silent pass if the product default changes.
A (90–100)	mod SourceGenerationNonAotTests. SourceGenerationNonAot_ BuildsAndRunsTests_ WithExitCodeZero	Glob widening makes the generated-file check mode-agnostic; four assertions span the full build-generate-run chain.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · 68.5 AIC · ⌖ 9.56 AIC · ⊞ 9.5K · [◷]( · ◷)

[Copilot]

Review details

• Files reviewed: 11/11 changed files
• Comments generated: 1
• Review effort level: Medium