Make EventSourceLoggerTest serialization trim safe#128347
Draft
Copilot wants to merge 4 commits into
Draft
Conversation
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8005f0c8-ab92-4a96-98f3-fa106dd58689 Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8005f0c8-ab92-4a96-98f3-fa106dd58689 Co-authored-by: MichalStrehovsky <13110571+MichalStrehovsky@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
MichalStrehovsky
May 19, 2026 00:19
View session
Removed ActiveIssue attribute for issue 73438 from multiple test methods.
Member
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
Removes a trim/AOT unsafe JsonConvert.SerializeObject fallback in EventSourceLoggerTest's JSON payload helper, replacing it with explicit JsonTextWriter-based serialization that handles the specific payload shapes (dictionaries, key/value sequences, object arrays, primitives, enums, nulls). This allows the test project to re-enable the AOT analyzer and removes several ActiveIssue AOT skips.
Changes:
- Replaced
JsonConvert.SerializeObject(withIL2026suppression) with manualWriteComplexValue/WriteDictionary/WriteValuehelpers and expandedIsPrimitiveto cover all primitives, enums,decimal,DateTime, andGuid. - Removed
[ActiveIssue(... IsNativeAot)]attributes on four tests now that the unsafe path is gone. - Removed the
EnableAotAnalyzer=falseopt-out from the test csproj.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Logging.EventSource/tests/EventSourceLoggerTest.cs | Replaces reflection-based JSON fallback with trim-safe manual serialization; widens IsPrimitive; removes NativeAOT ActiveIssue skips. |
| src/libraries/Microsoft.Extensions.Logging.EventSource/tests/Microsoft.Extensions.Logging.EventSource.Tests.csproj | Removes EnableAotAnalyzer=false opt-out now that the suppressed IL2026 call is gone. |
This was referenced May 19, 2026
rosebyte
approved these changes
May 19, 2026
Member
|
The test was failing with trimming despite no trimming warnings. This was indeed a product bug like I wrote in #127449 (comment). |
Member
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Fixes #73438.
EventSourceLoggerTestusedJsonConvert.SerializeObjectbehind anIL2026suppression for complex EventSource payloads. This path was not trim/AOT safe.Manual complex payload serialization
JsonConvert.SerializeObjectfallback withJsonTextWriter-based serialization for the payload shapes emitted by these tests.Analyzer coverage