Make TranslationLayer Native AOT-compatible#16045
Open
drewnoakes wants to merge 9 commits into
Open
Conversation
The `Microsoft.TestPlatform.VsTestConsole.TranslationLayer` and `Microsoft.TestPlatform.CommunicationUtilities` assemblies are loaded in-process by NativeAOT consumers (e.g. C# Dev Kit). NativeAOT disables reflection-based `System.Text.Json` serialization by default, causing the vstest wire protocol to fail silently during the TCP handshake and test discovery. ## Changes ### Source-generated `JsonSerializerContext` Add `TestPlatformJsonContext` with `[JsonSerializable]` attributes for every type that crosses the wire: payload DTOs, envelope DTOs, collection types, and `PayloadedMessage<T>` for each concrete `T` used by `VsTestConsoleRequestSender`. Set `TypeInfoResolver` on the base `JsonSerializerOptions` so STJ has compile-time metadata available without runtime reflection. ### AOT-safe serialization converters - **`TestObjectBaseConverterFactory`**: replace `MakeGenericType` + `Activator.CreateInstance` with a singleton non-generic converter. - **`ObjectConverter.Write`**, **`ObjectDictionaryConverter.Write`**, **`TestObjectConverter.Write`**, **`TestCaseConverterV2.Write`**, **`TestResultConverterV2.Write`**: replace `JsonSerializer.Serialize(writer, value, value.GetType(), options)` with direct primitive writes via a centralized `WritePropertyValue()` helper that handles string, int, long, double, float, bool, DateTimeOffset, DateTime, Guid, Uri, with `ToString()` fallback. ### Envelope DTO fixes - Change `MessageEnvelope.Payload` and `VersionedMessageEnvelope.Payload` from `object?` to `JsonElement?` so pre-serialized payloads are embedded as nested JSON objects rather than double-encoded strings. - Make `MessageEnvelope`, `VersionedMessageEnvelope`, `VersionedMessageForSerialization`, and `PayloadedMessage<T>` `internal` (were `private`) so the source generator can reference them. - Use `JsonSerializer.SerializeToElement(payload, payload.GetType(), options)` to serialize payloads with runtime type dispatch before embedding in envelopes. ### AOT/trim analyzers enabled Set `IsAotCompatible=true`, `EnableTrimAnalyzer=true`, and `EnableAotAnalyzer=true` on the `net8.0` TFM for both `TranslationLayer` and `CommunicationUtilities` projects. Both build with zero IL2xxx/IL3xxx warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Chain source-gen context with DefaultJsonTypeInfoResolver so types not covered by the source-gen context fall back to reflection in non-AoT builds, while NativeAOT consumers use the source-gen metadata. - Pass JsonSerializerOptions through WritePropertyValue so the default case uses the resolver chain instead of bare reflection. - Add ExceptionConverter to handle Exception de/serialization properly since source-gen uses the parameterless constructor which cannot populate the read-only Message/StackTrace properties. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The converter was always creating a TestCase regardless of typeToConvert, which caused InvalidCastException when deserializing other TestObject subclasses. Now uses Activator.CreateInstance(typeToConvert) for concrete types, falling back to TestCase for abstract types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All JsonSerializer.Serialize/Deserialize calls in converters and JsonDataSerializer now go through StjSafe, which centralizes the [UnconditionalSuppressMessage] attributes. This prevents the NativeAOT linker in consuming projects (e.g., C# DevKit) from emitting IL2026 trimming warnings for these call sites. The suppressions are safe because all JsonSerializerOptions instances are configured with TestPlatformJsonContext (source-gen) as the primary TypeInfoResolver. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Makes the TranslationLayer and CommunicationUtilities assemblies NativeAOT/trim-compatible by introducing a source-generated JsonSerializerContext, routing STJ calls through suppression wrappers, and restructuring envelope DTOs and converters to avoid reflection-based polymorphic serialization.
Changes:
- Added
TestPlatformJsonContext(source-gen) andStjSafewrappers; chained source-gen resolver withDefaultJsonTypeInfoResolverfallback. - Changed envelope
Payloadfromobject?toJsonElement?, addedExceptionConverter, and rewroteTestObjectBaseConverterto avoidMakeGenericType/Activator.CreateInstancepatterns; centralized property-value writing inWritePropertyValue. - Enabled
IsAotCompatible/trim/AOT analyzers on both projects (net8.0+) and downgraded pre-existing trim warnings inCommunicationUtilities.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.TestPlatform.VsTestConsole.TranslationLayer/Microsoft.TestPlatform.VsTestConsole.TranslationLayer.csproj | Enable AOT/trim analyzers for net8.0+. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Microsoft.TestPlatform.CommunicationUtilities.csproj | Enable AOT/trim analyzers and downgrade pre-existing warnings. |
| src/Microsoft.TestPlatform.CommunicationUtilities/TestPlatformJsonContext.cs | New source-generated JSON context listing wire types. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/StjSafe.cs | Suppression wrappers around JsonSerializer APIs. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/ExceptionConverter.cs | New converter preserving Message/InnerException on deserialization. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectBaseConverter.cs | Replaced generic converter with single non-generic converter + WritePropertyValue helper. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/ObjectConverter.cs | Use WritePropertyValue for object/dictionary values. |
| src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestCaseConverter.cs, TestCaseConverterV2.cs, TestResultConverter.cs, TestResultConverterV2.cs, TestObjectConverter.cs, TestExecutionContextConverter.cs, TestRunChangedEventArgsConverter.cs, TestRunCompleteEventArgsConverter.cs, AfterTestRunEndResultConverter.cs, DiscoveryCriteriaConverter.cs, AttachmentConverters.cs | Routed STJ calls through StjSafe; avoid value.GetType() overload. |
| src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.Stj.cs | Configured TypeInfoResolver, switched envelope payload to JsonElement?, registered ExceptionConverter, made nested DTOs internal. |
Member
|
Looks good, on quick look. Can we get an integration test (can be marked as compatibility to avoid running it too much, that will actually build app in native aot, check that there are no warnings and will run a test dll project via that)? (Compatibility tests don't run in PR build, nor do they build automatically during asset build. LMK if you need more help. ) |
- ExceptionConverter: restore HResult and Source in Read (writable properties), document type-erasure and StackTrace limitations. - WritePropertyValue: add missing primitive cases for short, ushort, uint, ulong, byte, sbyte, decimal, char, and enums to avoid falling through to SerializeToElement for types not in the source-gen context. - TestObjectBaseConverter: revert Activator.CreateInstance to new TestCase() to avoid reflection incompatible with NativeAOT; document that this path only handles generic property bags on the wire. - SerializePayloadCore: document why the fast path was intentionally collapsed (object? Payload is incompatible with AoT). - TestPlatformJsonContext: add maintenance checklist for new payload types, clarify why Dictionary<string,object> entries are needed alongside the custom converters. - StjSafe: add DEBUG assert verifying options.TypeInfoResolver is configured, to catch misuse as the codebase evolves. - Update TestObjectConverterTests to deserialize as TestObject (not TestableTestObject) and verify custom properties by ID rather than counting all properties (TestCase carrier has built-in properties). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Introduce RemoteException that carries the original ClassName and StackTraceString from the remote process. ToString() renders the full original diagnostic output (type name, message, stack trace, inner exception chain) so that callers see the same information they would from the original exception. The ExceptionConverter Write path also handles round-tripping: if the exception is already a RemoteException, it preserves the stored ClassName and RemoteStackTrace rather than emitting the wrapper type. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The fast-path branch was already doing the same thing as the slow-path after the AoT changes (both serialize to JsonElement then embed in envelope). Merge into a single code path and remove the now-unused DisableFastJson field and Utilities using. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds a test asset (NativeAotTranslationLayerConsumer) that is a minimal
console app referencing the TranslationLayer with PublishAot=true.
The NativeAotCompatibilityTests test publishes this app with NativeAOT
and asserts that no IL2026/IL3050 linker warnings originate from the
CommunicationUtilities.Serialization namespace. Pre-existing warnings
from Jsonite, ObjectModel, and DefaultJsonTypeInfoResolver are excluded.
Marked with [TestCategory("Compatibility")] so it doesn't run in
every PR build (NativeAOT publish takes ~4 minutes).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/Microsoft.TestPlatform.CommunicationUtilities/JsonDataSerializer.Stj.cs:1
- This change removes the previous ‘fast’ serialization path and now always performs a two-step
object -> JsonElement -> envelopeflow, which is materially more expensive (allocations + extra traversal) for high-volume message traffic. If NativeAOT safety is the driver, consider keeping the previous direct serialization path for non-AOT scenarios (e.g., when dynamic code/reflection is supported) and only using the JsonElement-based path when running under AOT constraints; that preserves throughput while still meeting AOT requirements.
// Copyright (c) Microsoft Corporation. All rights reserved.
- Narrow TestObjectBaseConverter.CanConvert to only typeof(TestObject), not all TestObject subtypes. TestCase/TestResult have their own converters and no other subtypes flow through the wire protocol. - Remove Enum special case from WritePropertyValue — let enums fall through to SerializeToElement which respects JsonSerializerOptions (avoids ulong overflow and bypassing custom enum converters). - Test asset: remove unused usings, touch VsTestConsoleWrapper type and payload types so the linker analyzes the full TranslationLayer graph. - Tests: use Assert.IsNotNull before TestProperty.Find instead of null-forgiving operator. - Tests: serialize as TestObject (declared wire type) not TestableTestObject. - Integration test: add second WaitForExit() call to drain async output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
Added. All feedback addressed. |
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.
Motivation
C# DevKit embeds the VS Test translation layer and is compiled with NativeAOT. The current STJ serialization code relies on reflection-based
JsonSerializeroverloads, which produce IL2026/IL3050 linker warnings and risk runtime failures when reflection is trimmed.What this PR does
Adds a source-generated
JsonSerializerContextand restructures the serialization layer so that NativeAOT consumers can use the translation layer APIs without trimming warnings or runtime errors.Source-generated JSON context (
TestPlatformJsonContext)[JsonSerializable]attributesDefaultJsonTypeInfoResolverviaJsonTypeInfoResolver.Combine()so non-AoT consumers (vstest.console.exe) fall back to reflection for types outside the contextSerialization fixes
MessageEnvelope,VersionedMessageEnvelope) fromobject? PayloadtoJsonElement? Payloadto avoid polymorphic serializationSerializePayloadCorenow usespayload.GetType()so STJ sees the runtime type instead of objectWritePropertyValuepassesJsonSerializerOptionsthrough so the default case uses the resolver chain instead of bare reflectionCustom converters
ExceptionConverter: usesException(string, Exception)constructor to preserveMessageduring deserialization (the source-gen parameterless constructor path leaves it at the default value)TestObjectBaseConverter: instantiates the correcttypeToConvertinstead of always creatingTestCaseIL2026/IL3050 warning suppression (StjSafe)
JsonSerializer.Serialize/Deserialize/SerializeToElementcalls in converters route through a thin StjSafe wrapper with[UnconditionalSuppressMessage]attributesJsonSerializerOptionsare configured withTestPlatformJsonContextas the primaryTypeInfoResolverProject configuration
IsAotCompatible,EnableTrimAnalyzer,EnableAotAnalyzeron bothCommunicationUtilitiesandTranslationLayerprojects (net8.0+)Jsonite,DiscoveryCriteriaConverterreflection) are downgraded viaWarningsNotAsErrorsScope
The
CommunicationUtilitiesassembly is shared between the server (vstest.console.exe, not AoT) and the client (TranslationLayer, C# DevKit, AoT). Only client-side code paths need to be AoT-clean. Server-only code (e.g.,DiscoveryCriteriaConverterwith reflection, V1 converters) is not in the client path and is handled by the reflection fallback resolver.