feat: Initial AOT compatibility metadata#79
Open
BenjaminMichaelis wants to merge 11 commits into
Open
Conversation
Multi-target production libraries to netstandard2.1 and net10.0, enable IsAotCompatible for TrxToPlaylistConverter on net8+, and document current AOT status. Also fix nullable annotations for Path.GetDirectoryName in playlist builders.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR starts AOT-related packaging work by adding modern .NET targeting while preserving netstandard2.1 compatibility, and documents the current AOT status.
Changes:
- Multi-targets the two production libraries to
netstandard2.1;net10.0. - Adds conditional
IsAotCompatiblemetadata to the TRX converter project. - Updates README AOT status and fixes nullable annotations for playlist output directories.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
VSTestPlaylistTools/VSTestPlaylistTools.csproj |
Adds net10.0 alongside netstandard2.1. |
VSTestPlaylistTools/PlaylistV2/PlaylistV2Builder.cs |
Makes Path.GetDirectoryName(...) result nullable-aware. |
VSTestPlaylistTools/PlaylistV1/PlaylistV1Builder.cs |
Makes Path.GetDirectoryName(...) result nullable-aware. |
VSTestPlaylistTools.TrxToPlaylistConverter/VSTestPlaylistTools.TrxToPlaylistConverter.csproj |
Adds net10.0 targeting and conditional AOT compatibility metadata. |
README.md |
Documents AOT compatibility status and caveats. |
Comments suppressed due to low confidence (1)
VSTestPlaylistTools.TrxToPlaylistConverter/VSTestPlaylistTools.TrxToPlaylistConverter.csproj:11
- Declaring this package AOT-compatible is not accurate while its public conversion methods call
PlaylistV1Builder.SaveToFile/ToXmlString, which go throughPlaylistRoot.Serializeand createXmlSerializerat runtime inVSTestPlaylistTools/PlaylistV1/PlaylistRoot.cs. That is the same runtime XML serialization path the PR notes exclude from AOT support for the main package, so consumers can get a false AOT-compatible signal and still fail or miss warnings when using these APIs under Native AOT.
<PropertyGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net8.0'))">
<IsAotCompatible>true</IsAotCompatible>
</PropertyGroup>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add net8.0 target to TrxToPlaylistConverter so .NET 8/9 consumers resolve an AOT-analyzed asset, and update README wording to match actual target frameworks.
…mpatibility - Remove all XmlSerializer usage from VSTestPlaylistTools V1 and V2 playlists - Create PlaylistV2Serializer internal helper for recursive XML read/write - Remove [XmlRoot]/[XmlAttribute]/[XmlElement]/[XmlIgnore] from model classes - Enable IsAotCompatible=true on VSTestPlaylistTools for net8.0+ - Add AOT smoke test project (VSTestPlaylistTools.TrxToPlaylistConverter.AotSmokeTest) - Add aot-compatibility CI job with publish, run, and nupkg TFM verification steps - Fix BooleanRule Match attribute validation (throw on invalid/missing values) - Fix XmlReader exception wrapping in FromXmlReader public methods
Replace TrxLib (netstandard2.1, non-AOT) with native System.Xml.Linq TRX parsing and a new TestOutcome enum, making VSTestPlaylistTools.TrxToPlaylistConverter truly AOT-compatible. - Add TestOutcome.cs: public enum replacing TrxLib.TestOutcome - Add TrxFileParser.cs: internal AOT-safe TRX parser via System.Xml.Linq - Remove TrxLib PackageReference from the library project - Update trx-to-vsplaylist to use the new TestOutcome enum - Add explicit TrxLib ref to test project (for assertion helpers only)
The 'Verify nupkg TFMs' step used bash syntax (subshell assignment, test, unzip) but the workflow default shell is pwsh, causing the step to fail on ubuntu-latest with a 'not recognized as a cmdlet' error. Add \shell: bash\ to that step so it runs under bash as intended.
- Add missing TRX outcome values to TestOutcome enum (Error, Aborted, NotRunnable, Warning, Completed, InProgress, Disconnected) - Skip results with unknown outcome strings in TrxFileParser instead of coercing to NotExecuted - Throw InvalidOperationException in PlaylistV2Serializer.WriteRule for unrecognized Rule subclasses (prevent silent data loss) - Update README to reflect both packages now carry IsAotCompatible metadata - Fix CI: add -r linux-x64 to restore step for RID-specific assets - Fix CI: restore converter project before pack to provide all TFM assets - Add TRX parsing exercise to AOT smoke test (covers TrxFileParser.Parse)
- Add internal PlaylistXmlHelper with EnsureDirectory and StringToReader helpers; used by V1/V2 Builder (SaveToFile) and V1/V2 Parser (FromString) - Use StringReader instead of MemoryStream+StreamReader for string->TextReader conversion (no unnecessary UTF-8 encode/decode round-trip) - EnsureDirectory drops redundant Directory.Exists check; CreateDirectory is already idempotent - Extract private FilterResults in TrxToPlaylistConverter, used by both ConvertTrxToPlaylist and ConvertMultipleTrxToPlaylist
BenjaminMichaelis
changed the title
BenjaminMichaelis
changed the title
Without a net8.0 asset, .NET 8 consumers of VSTestPlaylistTools resolve the netstandard2.1 fallback and never receive the IsAotCompatible=true metadata. This aligns both library packages: netstandard2.1;net8.0;net10.0.
- Update README AOT section so VSTestPlaylistTools target list includes net8.0 - Harden TRX outcome parsing with Enum.IsDefined to skip undefined numeric enum values
- Enforce Enum.IsDefined in CLI outcome parsing to reject undefined numeric values - Add focused test covering --outcome 999 returning failure - Improve NotRunnable XML-doc summary grammar in TestOutcome
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.
Why
This starts the production-scope AOT compatibility work so consumers on modern .NET can use analyzer-backed targets while retaining existing netstandard compatibility.
What changed
netstandard2.1;net10.0:VSTestPlaylistToolsVSTestPlaylistTools.TrxToPlaylistConverterIsAotCompatibleforVSTestPlaylistTools.TrxToPlaylistConverteronnet8.0+targets.string?forPath.GetDirectoryName(...)results.Notes for reviewers
VSTestPlaylistToolsis intentionally not markedIsAotCompatibleyet. Its XML serializer paths still rely on runtimeXmlSerializerbehavior, so full AOT compatibility for that package is a follow-up item.