fix: add missing vstest outcomes and fix directory heuristic for RID paths#46
Merged
Merged
Conversation
…net10.0) (#41) ## Summary Makes TrxLib fully AOT-compatible by replacing `XmlSerializer` with `XDocument`/`XElement` manual parsing, adding multi-targeting, and validating with a native AOT smoke-test project in CI. ## Changes ### Library (`TrxLib`) - **Multi-target**: `netstandard2.1;net8.0;net10.0` — keeps backward compatibility while enabling `IsAotCompatible=true` on net8.0+ - **Parser rewrite**: `XmlSerializer` → `XDocument.Load()` + `XElement` navigation (fully AOT-safe, available on all targets, no `#if` branching) - **New model classes**: `Execution`, `Counters`, `ResultSummary`, `TestList`, `TestLists`, `TestEntry`, `TestEntries` - **Complete parsing**: all previously-unset fields now populated — `RunUser`, `ResultSummary`/`Counters`, `TestLists`, `TestEntries`, `UnitTest.Storage`/`Execution`, `UnitTestResult.ExecutionId`/`TestListId`/`TestType`/`RelativeResultsDirectory` - **`TestResultSet` properties**: `TestRunId`, `DeploymentRoot`, `TestSettingsName` now populated from parsed data - **Theory/parameterized test names**: FQTN now correctly appends parameter suffixes (e.g. `MethodName(arg: "value")`) - **Exception handling**: `catch (Exception)` narrowed to `catch (XmlException)` in the XML loading path - **Cleanup**: removed inert `[XmlRoot]`/`[XmlElement]`/`[XmlAttribute]` attributes and `using System.Xml.Serialization` from all model classes ### AOT Smoke Test (`TrxLib.AotSample`) - New `net10.0` console app with `PublishAot=true` that references TrxLib - Writes a minimal TRX to a temp file, parses it with `TrxParser.Parse`, and asserts the result — exercising the full `XDocument` parse path in the native binary ### CI (`.github/workflows/build-and-test.yml`) - Added `AOT Publish Validation` step to the existing matrix job (ubuntu + windows) - Runs `dotnet publish -r <rid> --self-contained` then **executes the native binary** to catch both link-time and runtime AOT regressions ## Verification - ✅ 38/38 tests pass - ✅ `dotnet build` — 0 AOT analyzer warnings on net8.0/net10.0 - ✅ `dotnet publish -r win-x64` — 0 ILLink/AOT warnings - ✅ Native binary runs and prints `TrxLib AOT validation passed.`
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 6 to 7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/upload-artifact/releases">actions/upload-artifact's releases</a>.</em></p> <blockquote> <h2>v7.0.0</h2> <h2>v7 What's new</h2> <h3>Direct Uploads</h3> <p>Adds support for uploading single files directly (unzipped). Callers can set the new <code>archive</code> parameter to <code>false</code> to skip zipping the file during upload. Right now, we only support single files. The action will fail if the glob passed resolves to multiple files. The <code>name</code> parameter is also ignored with this setting. Instead, the name of the artifact will be the name of the uploaded file.</p> <h3>ESM</h3> <p>To support new versions of the <code>@actions/*</code> packages, we've upgraded the package to ESM.</p> <h2>What's Changed</h2> <ul> <li>Add proxy integration test by <a href="https://github.com/Link"><code>@Link</code></a>- in <a href="https://github.com/actions/upload-artifact/pull/754">actions/upload-artifact#754</a></li> <li>Upgrade the module to ESM and bump dependencies by <a href="https://github.com/danwkennedy"><code>@danwkennedy</code></a> in <a href="https://github.com/actions/upload-artifact/pull/762">actions/upload-artifact#762</a></li> <li>Support direct file uploads by <a href="https://github.com/danwkennedy"><code>@danwkennedy</code></a> in <a href="https://github.com/actions/upload-artifact/pull/764">actions/upload-artifact#764</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/Link"><code>@Link</code></a>- made their first contribution in <a href="https://github.com/actions/upload-artifact/pull/754">actions/upload-artifact#754</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/upload-artifact/compare/v6...v7.0.0">https://github.com/actions/upload-artifact/compare/v6...v7.0.0</a></p> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/upload-artifact/commit/bbbca2ddaa5d8feaa63e36b76fdaad77386f024f"><code>bbbca2d</code></a> Support direct file uploads (<a href="https://github.com/actions/upload-artifact/issues/764">#764</a>)</li> <li><a href="https://github.com/actions/upload-artifact/commit/589182c5a4cec8920b8c1bce3e2fab1c97a02296"><code>589182c</code></a> Upgrade the module to ESM and bump dependencies (<a href="https://github.com/actions/upload-artifact/issues/762">#762</a>)</li> <li><a href="https://github.com/actions/upload-artifact/commit/47309c993abb98030a35d55ef7ff34b7fa1074b5"><code>47309c9</code></a> Merge pull request <a href="https://github.com/actions/upload-artifact/issues/754">#754</a> from actions/Link-/add-proxy-integration-tests</li> <li><a href="https://github.com/actions/upload-artifact/commit/02a8460834e70dab0ce194c64360c59dc1475ef0"><code>02a8460</code></a> Add proxy integration test</li> <li>See full diff in <a href="https://github.com/actions/upload-artifact/compare/v6...v7">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) You can trigger a rebase of this PR by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> > **Note** > Automatic rebases have been disabled on this pull request as it has been open for over 30 days. Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Potential fix for [https://github.com/BenjaminMichaelis/TrxLib/security/code-scanning/3](https://github.com/BenjaminMichaelis/TrxLib/security/code-scanning/3) Add an explicit `permissions` block for the `build-and-test` job in `.github/workflows/build-and-test.yml`, with the minimum required scope: - `contents: read` This is the best fix because it preserves existing behavior (checkout/build/test still work) while ensuring `GITHUB_TOKEN` cannot get unintended write privileges from repo/org defaults. Edit only the `build-and-test` job section, directly under `runs-on`. _Suggested fixes powered by Copilot Autofix. Review carefully before merging._ Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Bug 3 - vstest outcomes Error/Aborted/NotRunnable silently coerced to
NotExecuted by catch-all in TrxParser. Three tests cover the
three most impactful missing outcomes.
Bug 4 - Test project directory derived by hardcoded 3-level upward
traversal from codeBase DLL path. Breaks for RID-qualified
output paths (bin/Debug/net8.0/<rid>/Foo.dll) where 4 levels
are required.
Note: Bugs 1 (TestResultSet convenience fields) and 2 (UnitTestResult
missing executionId/testListId/relativeResultsDirectory) were already
fixed in origin/main before this branch was rebased.
Bug 3 - Add 8 missing vstest TestOutcome enum values (Error, Aborted,
NotRunnable, Disconnected, Warning, Completed, InProgress,
PassedButRunAborted) to TestOutcome.cs and add corresponding
switch arms in TrxParser.cs.
Also map null outcome attribute to TestOutcome.Error: vstest
omits the outcome= attribute when outcome is Error because Error
is ordinal 0 (the enum default) and XmlPersistence.SaveSimpleField
skips writing attributes whose value equals the default.
Bug 4 - Replace hardcoded 3-level upward traversal with a 'bin-anchor'
heuristic: walk up until reaching the 'bin' directory, then
return its parent. Correctly handles all standard .NET SDK output
layouts including RID-qualified (depth 4) and publish (depth 4/5)
paths.
Test updates:
- Strengthen Bug 3 assertions from NotBe(NotExecuted) to Be(exact value)
- Add Bug_Parse_MissingOutcomeAttribute_MapsToError (the real-world form
of the Error outcome in vstest-generated TRX files)
- Add Bug_Parse_TestProjectDirectory_IsCorrectForPublishOutputPath
- Add Bug_Parse_TestProjectDirectory_IsCorrectForRidPlusPublishOutputPath
All 59 tests pass.
Research validated by GPT-5.5 tracing the vstest source (commit ba0077af):
Converter.ToOutcome(), XmlPersistence.SaveSimpleField(), TrxLogger.cs.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes two bugs in TrxParser: (1) missing vstest TestOutcome enum values that caused 8 distinct outcome states (including Error) plus absent-attribute serialization to be silently coerced into NotExecuted, and (2) a hardcoded 3-level directory walk that misidentified the test project root for RID-qualified and publish output layouts. The TRX object model is brought in line with upstream vstest, and regression tests (TDD-style) accompany each fix.
Changes:
- Added 8 new values to
TestOutcome(Error,Aborted,NotRunnable,Disconnected,Warning,Completed,InProgress,PassedButRunAborted) with XML docs explaining each. - Extended
TrxParser's outcome switch with the new cases, and mapped a missingoutcome=attribute (null) toTestOutcome.Errorto match vstest'sXmlPersistencedefault-value omission behavior. - Replaced the fixed 3-level upward traversal with a
while-loop that walks up until it finds abindirectory, then returns its parent — covering classic, RID, publish, and RID+publish output layouts. - Added a new
KnownBugsTests.cssuite with outcome-parsing and project-directory regression tests, plus aTempTrxFilehelper.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| TrxLib/TestOutcome.cs | Adds 8 new enum values matching the upstream vstest TRX outcomes, with XML docs. |
| TrxLib/TrxParser.cs | Maps new outcomes (and missing attribute → Error); replaces fixed 3-level path traversal with a bin-seeking walk. |
| TrxLib.Tests/KnownBugsTests.cs | New regression tests for outcome parsing and RID/publish path resolution; includes TempTrxFile helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Took origin/main's sanitized machine name in theory-tests.trx (BAM-Z690/benja -> MACHINE/sample) - Removed duplicate enum values from TestOutcome.cs that were added by both this branch and origin/main's PR #43 independently; kept origin/main's ordering (Error=0, matching vstest ordinals)
- Consolidate MinimalTrxWithOutcome/MinimalTrxWithoutOutcome/MinimalTrxWithCodebase into a single MinimalTrx(string? outcome = null, string codeBase = 'test.dll') factory - Extract AssertProjectRootResolves helper; collapse 3 directory test bodies to one-liners - Remove all '// FAILS: ...' and 'going up 3 levels' comments (bugs are fixed) - Update class summary and section comments to describe current correct behavior instead of the old broken behavior
- Rename KnownBugsTests -> TrxParserRegressionTests (class + file)
- Drop 'Bug_' prefix from all 7 test method names
- Replace 'Bug 3 –' / 'Bug 4 –' section headings with descriptive
topic labels ('Outcome parsing' / 'Directory resolution')
These were downloaded locally during the vstest audit and should not be committed to the repository.
- 4 outcome Facts -> single [Theory][InlineData] (Parse_OutcomeAttribute_RoundTrips) - 3 directory Facts + helper -> [Theory][MemberData] (Parse_TestProjectDirectory_ResolvesFromBinAnchor) - Remove AssertProjectRootResolves helper and section-divider banners
The previous ternary returned the filesystem root directory when the codebase path contained no 'bin' segment, silently yielding a wrong result. Replaced with an explicit guard so TestProjectDirectory is null when the bin anchor cannot be found. Also drops the redundant null-conditional operators (dir?.Parent, dir?.Name) that were unnecessary since dir cannot be null after the while loop.
Replace == null / != null with is null / is not null, use is { } for
nullable struct unwrapping (DateTimeOffset? timings), and is true for
nullable bool comparisons.
1b6def6 to
2b14f1b
Compare
github-actions Bot
pushed a commit
to BenjaminMichaelis/VS.TestPlaylistTools
that referenced
this pull request
May 19, 2026
[//]: # (dependabot-start)⚠️ **Dependabot is rebasing this PR**⚠️ Rebasing might not happen immediately, so don't worry if this takes some time. Note: if you make any changes to this PR yourself, they will take precedence over the rebase. --- [//]: # (dependabot-end) Updated [TrxLib](https://github.com/BenjaminMichaelis/TrxLib) from 0.0.3 to 1.0.0. <details> <summary>Release notes</summary> _Sourced from [TrxLib's releases](https://github.com/BenjaminMichaelis/TrxLib/releases)._ ## 1.0.0 ## Features - Cleaned up some TRX file bugs - Now AOT compliant! - Cleaned up a lot of misc tech debt ## What's Changed * Bump actions/checkout from 5 to 6 by @dependabot[bot] in BenjaminMichaelis/TrxLib#20 * Bump actions/upload-artifact from 5 to 6 by @dependabot[bot] in BenjaminMichaelis/TrxLib#22 * Bump Microsoft.SourceLink.GitHub from 8.0.0 to 10.0.102 by @dependabot[bot] in BenjaminMichaelis/TrxLib#23 * Bump coverlet.collector from 6.0.4 to 8.0.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#24 * Bump Microsoft.SourceLink.GitHub from 10.0.102 to 10.0.103 by @dependabot[bot] in BenjaminMichaelis/TrxLib#25 * Bump AwesomeAssertions from 9.3.0 to 9.4.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#26 * Bump actions/download-artifact from 6 to 8 by @dependabot[bot] in BenjaminMichaelis/TrxLib#28 * Bump IntelliTect.Multitool from 1.5.3 to 2.0.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#29 * Bump Microsoft.NET.Test.Sdk from 18.0.1 to 18.3.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#30 * Bump Microsoft.SourceLink.GitHub from 10.0.103 to 10.0.201 by @dependabot[bot] in BenjaminMichaelis/TrxLib#31 * Bump coverlet.collector from 8.0.0 to 8.0.1 by @dependabot[bot] in BenjaminMichaelis/TrxLib#32 * Bump fastify/github-action-merge-dependabot from 3.11.2 to 3.12.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#33 * Bump Microsoft.NET.Test.Sdk from 18.3.0 to 18.4.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#34 * Bump Microsoft.SourceLink.GitHub from 10.0.201 to 10.0.202 by @dependabot[bot] in BenjaminMichaelis/TrxLib#36 * Bump coverlet.collector from 8.0.1 to 10.0.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#35 * Bump Microsoft.SourceLink.GitHub from 10.0.202 to 10.0.203 by @dependabot[bot] in BenjaminMichaelis/TrxLib#37 * Bump Microsoft.NET.Test.Sdk from 18.4.0 to 18.5.1 by @dependabot[bot] in BenjaminMichaelis/TrxLib#38 * feat: Migrate NuGet publish to trusted publishing (OIDC) by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#39 * Migrate to slnx solution file format by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#40 * fix: TRX parser data-loss bugs and FQTN derivation spec compliance by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#42 * fix: Refactor TestOutcome enum with updated summaries by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#43 * fix: add missing vstest outcomes and fix directory heuristic for RID paths by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#46 * fix: missing vstest outcomes, TestProjectDirectory heuristic, xmlns fallback by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#47 * Migrate from xUnit to TUnit and adopt Microsoft.Testing.Platform v2 by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#48 * Remove AwesomeAssertions, use TUnit built-in assertions by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#49 * Bump IntelliTect.Multitool from 2.0.0 to 2.1.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#50 * Bump TUnit from 1.44.39 to 1.45.0 by @dependabot[bot] in BenjaminMichaelis/TrxLib#52 * Bump Microsoft.SourceLink.GitHub from 10.0.203 to 10.0.300 by @dependabot[bot] in BenjaminMichaelis/TrxLib#51 * chore: align project configuration with NuGet library template best practices by @BenjaminMichaelis in BenjaminMichaelis/TrxLib#53 **Full Changelog**: BenjaminMichaelis/TrxLib@v0.0.3...v1.0.0 Commits viewable in [compare view](BenjaminMichaelis/TrxLib@v0.0.3...v1.0.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@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.
Why
Two bugs were found by comparing TrxLib's object model against the upstream vstest TRX ObjectModel (commit ba0077af), validated by tracing the vstest source through
Converter.ToOutcome()andXmlPersistence.SaveSimpleField().Bug 3 -- Unknown vstest outcomes silently coerced to
NotExecutedTrxParser's outcome switch had a catch-all_ => TestOutcome.NotExecutedthat silently swallowed 8 distinct vstest states.TestOutcome.csonly had 6 values while the upstream TRX enum has 14.Fix: Added the 8 missing enum values (
Error,Aborted,NotRunnable,Disconnected,Warning,Completed,InProgress,PassedButRunAborted) and corresponding switch arms.Non-obvious nuance: vstest never writes
outcome="Error"as a literal attribute.Erroris ordinal 0 (the enum default), soXmlPersistence.SaveSimpleField()omits the attribute entirely when outcome is Error. A<UnitTestResult>with nooutcome=attribute is the real-world form of the Error state (e.g., when attaching result files fails). The parser now mapsnull->TestOutcome.Errorinstead ofNotExecuted.This also resolves an internal inconsistency:
Counters.csalready tracked all 14 vstest outcomes at the aggregate level, butTestOutcomehad no matching per-result values.Bug 4 -- Hardcoded 3-level traversal breaks for RID-qualified and publish output paths
TrxParserderivedTestProjectDirectoryby walking exactly 3 levels up from the codeBase DLL path, which only works forbin/{config}/{tfm}/. It returns the wrong directory for:bin/{config}/{tfm}/{rid}/(depth 4 -- self-contained builds)bin/{config}/{tfm}/publish/(depth 4 -- publish output)bin/{config}/{tfm}/{rid}/publish/(depth 5 -- self-contained publish)Fix: Replaced the
for (i < 3)loop with awhilethat walks up until it finds thebindirectory, then returns its parent. This handles all standard .NET SDK output layouts regardless of depth.Tests
NotBe(NotExecuted)toBe(exact value); addedBug_Parse_MissingOutcomeAttribute_MapsToErrorfor the real vstest serialization of the Error state.