Skip to content

fix: TRX parser data-loss bugs and FQTN derivation spec compliance#42

Merged
BenjaminMichaelis merged 4 commits into
mainfrom
benjaminmichaelis/trex-trxlib-bug-analysis
May 17, 2026
Merged

fix: TRX parser data-loss bugs and FQTN derivation spec compliance#42
BenjaminMichaelis merged 4 commits into
mainfrom
benjaminmichaelis/trex-trxlib-bug-analysis

Conversation

@BenjaminMichaelis
Copy link
Copy Markdown
Owner

Summary

Fixes several data-loss and correctness bugs discovered by deep comparison against the upstream Microsoft.TestPlatform TRX object model.

Changes

1. XML serialization annotations (data-loss fixes)

Several model classes were missing [XmlAttribute]/[XmlElement] annotations, causing silent data loss during deserialization:

  • Counters — added [XmlAttribute] on all 16 counter properties
  • Execution — added [XmlAttribute("id")]
  • ResultSummary — added [XmlAttribute] + [XmlElement] for Counters and Output
  • TestList — added [XmlAttribute] on Name and Id
  • TestLists — added [XmlElement("TestList")] on Items
  • TestEntry — added [XmlAttribute] on all three properties
  • TestEntries — added [XmlElement("TestEntry")] on Items

2. Missing properties on core model classes

  • TestRun — added RunUser, ResultSummary, TestLists, TestEntries
  • UnitTestResult — added ExecutionId, TestListId, TestType, RelativeResultsDirectory
  • UnitTest — added Storage, Execution

3. Parser fixes

  • TestResultSet.TestRunId was never populated — now set from testRun.Id
  • FQTN for theory tests now preserves the parameter suffix (e.g. Method(param: 42))
  • TestResult.ToString() no longer throws ArgumentOutOfRangeException for unknown outcome values

4. Spec-compliant FQTN derivation (spec bug fix)

The TRX spec provides TestMethod.className and TestMethod.name as authoritative fields. Previously TestResult re-derived ClassName/Namespace by splitting the FQTN on '.', which breaks when theory-test parameter values contain dots (e.g. Method(param: "foo.bar")).

When testMethod is available, FullyQualifiedClassName/ClassName/Namespace are now derived from testMethod.ClassName directly. The dot-split heuristic is kept as a fallback.

Test approach

All fixes were developed TDD-style: failing tests written first, then implementation to make them pass.

  • 3 commits: red tests → green parser/model fixes → green FQTN spec fix
  • 49 tests pass, 0 regressions

Add stub model classes and 10 failing parser tests plus 1 unit test
that cover bugs confirmed against existing sample TRX files:

Model additions (no parser wiring yet — tests intentionally RED):
- Execution, Counters, ResultSummary, TestList, TestLists, TestEntry,
  TestEntries stub classes
- TestRun: RunUser, ResultSummary, TestLists, TestEntries properties
- UnitTestResult: ExecutionId, TestListId, TestType,
  RelativeResultsDirectory properties
- UnitTest: Storage, Execution properties

Failing tests (red until parser is fixed):
- TestRunId never populated from TestRun.Id
- runUser attribute on <TestRun> silently discarded
- <ResultSummary outcome=...> element never parsed
- <Counters> inside ResultSummary never parsed
- <TestLists> element never parsed
- <TestEntries> element never parsed
- executionId attribute on <UnitTestResult> discarded
- storage + <Execution> on <UnitTest> discarded (single combined test)
- Theory test parameter suffix dropped from FQTN
- TestResult.ToString() throws ArgumentOutOfRangeException for
  unknown/future TestOutcome enum values
- Add [XmlAttribute]/[XmlElement] to all new model classes:
  Counters, Execution, ResultSummary, TestList, TestLists,
  TestEntry, TestEntries
- Wire up new properties on TestRun (runUser, ResultSummary,
  TestLists, TestEntries), UnitTestResult (executionId, testListId,
  testType, relativeResultsDirectory), UnitTest (storage, Execution)
- Set TestResultSet.TestRunId from testRun.Id in TrxParser
- Fix FQTN logic to preserve theory-test parameter suffixes while
  still correctly handling the case where Name already contains the
  full FQTN
- Fix TestResult.ToString() to return unicode fallback instead of
  throwing ArgumentOutOfRangeException for unknown outcome values

All 48 tests pass (0 regressions).
…t-splitting FQTN

Per the TRX spec, TestMethod.className and TestMethod.name are authoritative
fields for the class and method name. Splitting the FQTN on '.' is unreliable
when theory-test parameter values contain dots (e.g. (param: "foo.bar")).

When testMethod is available, derive FullyQualifiedClassName, ClassName, and
Namespace directly from testMethod.ClassName (one LastIndexOf call, no
ambiguity). TestName is then the FQTN with the ClassName prefix stripped,
which preserves the full parameter suffix for theory tests.

The dot-split heuristic is kept as a fallback for when testMethod is not
supplied (e.g. direct construction in unit tests or non-TRX callers).

Adds a failing-then-passing test: Theory_test_with_dotted_param_is_parsed_correctly_when_testMethod_provided
Copilot AI review requested due to automatic review settings May 16, 2026 22:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes TRX parsing/data-model correctness issues by aligning TrxLib’s XML-serializable object model and FQTN derivation logic with vstest/TRX semantics, preventing silent deserialization data loss and improving result identity for theory tests.

Changes:

  • Added missing TRX model fields and XML serialization attributes/elements to prevent data loss during deserialization.
  • Fixed parser output to populate TestResultSet.TestRunId and preserve theory-test parameter suffixes in FQTN.
  • Updated TestResult inference to prefer authoritative TestMethod.className and made ToString() resilient to unknown TestOutcome values.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
TrxLib/UnitTestResult.cs Adds missing TRX UnitTestResult attributes (execution/list/type/attachments metadata).
TrxLib/UnitTest.cs Adds storage and <Execution> mapping for unit test definitions.
TrxLib/TrxParser.cs Fixes FQTN construction (theory params) and populates TestRunId on the result set.
TrxLib/TestRun.cs Adds run metadata and sections (runUser, ResultSummary, TestLists, TestEntries).
TrxLib/TestResult.cs Uses TestMethod.className for class/namespace derivation and avoids ToString() throwing on unknown outcomes.
TrxLib/TestLists.cs Introduces model for <TestLists> deserialization.
TrxLib/TestList.cs Introduces model for individual <TestList> entries.
TrxLib/TestEntries.cs Introduces model for <TestEntries> deserialization.
TrxLib/TestEntry.cs Introduces model for individual <TestEntry> links (testId/executionId/testListId).
TrxLib/ResultSummary.cs Introduces model for <ResultSummary> including counters/output.
TrxLib/Execution.cs Introduces model for <Execution id="..."/> under unit tests.
TrxLib/Counters.cs Introduces model for <Counters ...> attributes to preserve vstest-computed counts.
TrxLib.Tests/TrxParserTests.cs Adds regression coverage for previously dropped TRX attributes/elements and theory FQTN preservation.
TrxLib.Tests/TestResultTests.cs Adds coverage for dotted theory params and ToString() behavior with unknown outcomes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread TrxLib/TrxParser.cs Outdated
Comment thread TrxLib.Tests/TrxParserTests.cs Outdated
…ment

- Add StringComparison.Ordinal to both StartsWith calls in TrxParser.cs to
  avoid locale-dependent behavior (e.g. Turkish-I problem)
- Update stale 'should be RED until fixed' comment in TrxParserTests.cs to
  reflect that these are now passing regression tests
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

@BenjaminMichaelis BenjaminMichaelis merged commit de56d1a into main May 17, 2026
7 checks passed
@BenjaminMichaelis BenjaminMichaelis deleted the benjaminmichaelis/trex-trxlib-bug-analysis branch May 17, 2026 03:42
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>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=TrxLib&package-manager=nuget&previous-version=0.0.3&new-version=1.0.0)](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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants