fix: preserve TRX results when session cleanup fails

[Sing303]

Description

When a session-level failure reaches TUnitTestFramework.ReportUnhandledException, TUnit publishes a synthetic error node with only ErrorTestNodeStateProperty. With --report-trx, Microsoft.Testing.Platform 2.4.1 cannot determine a class name for that node and throws while generating the report:

System.InvalidOperationException: The test framework 'TUnit' with UID 'TUnitExtension'
does not support the 'ITrxReportCapability' leading to missing or incomplete TRX reports
   at Microsoft.Testing.Extensions.TrxReport.Abstractions.TrxReportEngine.GetClassAndMethodName(...)

This can discard the results of tests that already completed. The regression in this PR runs one passing test and one failing test, followed by a static ClassDataSource resource whose DisposeAsync throws. On the unmodified main at a1fa4a2, the generated TRX has 0 results, and the regression fails expecting 3.

The synthetic node now carries:

• TrxFullyQualifiedTypeNameProperty, identifying the framework because there is no user test method for a session-level failure.
• TrxExceptionProperty, using the existing FlattenedException handling so both the cleanup exception and its inner causes retain their messages and stack traces in TRX.

The report then contains 3 results: 1 passed and 2 failed (the original test failure and the session cleanup failure). The test process still exits with code 10 for the failed session. Exception propagation, cleanup behavior, and the console/IDE state-property distinction are unchanged.

The reproducer requires both a test to use the resource and the child-process-only TUNIT_TEST_THROW_ON_STATIC_RESOURCE_DISPOSAL=1 opt-in. Static data sources can be initialized even when their tests are filtered out; selecting PassingTest directly or including this fixture in another run does not activate the cleanup failure. A second regression explicitly removes the opt-in from its child process and verifies that the passing test completes the session successfully.

Related Issue

No separate issue. This PR includes the reproduction and regression test. It complements #6777: that change preserves nested exceptions, but synthetic unhandled-error nodes still lack the TRX metadata supplied for ordinary test results.

Type of Change

• Bug fix (non-breaking change that fixes an issue)

Checklist

• Read the contributing guidelines.
• Follow existing engine and integration-test conventions.
• Added a regression test and observed it fail on unmodified main before applying the fix.
• Tested source-generated and reflection execution. Both use this shared reporting path; no discovery or generator change is needed.
• Public API and generator snapshots: not applicable; neither surface changes.
• Performance: additional metadata is created only on the unhandled-exception path.
• AOT/trimming: no new reflection or dynamic code. The regression inherits the existing Reflection/AOT test matrix; local AOT rows are skipped by the repository's CI-only policy.

Testing

Windows x64, .NET SDK 10.0.401, Microsoft.Testing.Platform / TRX 2.4.1.

Built the child test application, as required by InvokableTestBase:

dotnet build tests/TUnit.TestProject/TUnit.TestProject.csproj -c Release -f net10.0

Ran the new regression and existing nested-exception TRX regressions:

dotnet test --project tests/TUnit.Engine.Tests/TUnit.Engine.Tests.csproj -c Release -f net10.0 --treenode-filter "/*/*/*TrxTests/*" --minimum-expected-tests 10 --report-trx

Result: 5 passed, 0 failed, 5 AOT rows skipped by the existing local-run policy. This includes the existing nested and aggregate exception tests from #6777 and the opt-in isolation regression, which failed before the isolation fix (expected a completed session, got a failed session).

Also ran the reproducer directly in source-generated mode:

dotnet test --project tests/TUnit.TestProject/TUnit.TestProject.csproj -c Release -f net10.0 --no-build --no-restore --treenode-filter "/*/*/UnhandledExceptionTrxTestCases/*" --environment TUNIT_TEST_THROW_ON_STATIC_RESOURCE_DISPOSAL=1 --minimum-expected-tests 2 --detailed-stacktrace --report-trx

The deliberately failing process exits with code 10. Parsed the resulting TRX and verified all three outcomes, the original test-failure message and stack, and the cleanup-failure message and stack.

Also ran only PassingTest directly in source-generated mode without the opt-in: exit code 0, one passed result, no session cleanup failure.

The full repository suite and Native AOT publish were not run locally.

Summary by CodeRabbit

• Bug Fixes
  • TRX reports now preserve passing and failing test results when shared-resource cleanup throws an exception.
  • Cleanup errors include their messages and stack traces, while test-body failures remain visible.
  • A passing test without cleanup errors no longer causes the test session to be reported as failed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: a79d6a9a-580c-4556-8bec-320b662875c1

📥 Commits

Reviewing files that changed from the base of the PR and between a1fa4a2 and bfe67b1.

📒 Files selected for processing (3)

• src/TUnit.Engine/Framework/TUnitTestFramework.cs
• tests/TUnit.Engine.Tests/UnhandledExceptionTrxTests.cs
• tests/TUnit.TestProject/UnhandledExceptionTrxTestCases.cs

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The engine reports flattened exception details in synthetic TRX error nodes while preserving the original exception for console clients. Added test cases and engine tests cover passing results, test-body failures, and shared-resource disposal errors.

Changes

Unhandled exception TRX reporting

Layer / File(s)	Summary
Flattened exception details src/TUnit.Engine/Framework/TUnitTestFramework.cs	ReportUnhandledException creates a flattened exception. Console clients receive the original exception, while other clients receive the flattened exception. Synthetic error nodes include the TRX type name and exception details.
TRX failure coverage tests/TUnit.TestProject/UnhandledExceptionTrxTestCases.cs, tests/TUnit.Engine.Tests/UnhandledExceptionTrxTests.cs	Added test cases for passing and failing tests with shared-resource disposal behavior. Engine tests check session outcomes, counts, error messages, and stack traces.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to bfe67

No specific issue is established that should prevent merging after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving TRX test results when session cleanup fails.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the TRX report,
With flattened errors, clear and short.
A passing test stays in its place,
While failures show their stack trace.
Then hops away with ears held high.}

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge; no outstanding finding or new actionable issue was established.

Summary

The PR adds TRX metadata to synthetic unhandled-exception results so a session cleanup failure does not discard completed test results. The latest changes gate the fixture’s deliberate disposal failure to the regression’s child process and add a passing-only check.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Tests[Completed tests] --> Results[Result updates]
  Cleanup[Session cleanup failure] --> Node[Synthetic error node]
  Node --> Metadata[TRX type and exception properties]
  Results --> TRX[TRX report]
  Metadata --> TRX

Loading

_{Reviews (2) · Last reviewed commit: "test: scope disposal failure to the regr..."}

[github-actions]

Review

This PR fixes a real bug: when StaticPropertyHandler.DisposeStaticPropertiesAsync throws during session cleanup, the resulting AggregateException was previously reported in a way that discarded already-completed test results. Traced the failure path (ExecuteTestsCore → TestSessionCoordinator.ExecuteTests → TestRequestHandler.HandleRequestAsync → TUnitTestFramework.ExecuteRequestAsync's catch → ReportUnhandledException) and confirmed the fix is correctly targeted:

• Console clients still receive the original (unflattened) exception; other clients get FlattenedException.Wrap(exception), consistent with the existing IDE-fold behavior in TUnitMessageBus.GetFailureStateProperty.
• The new TrxFullyQualifiedTypeNameProperty/TrxExceptionProperty additions on the synthetic error node are inert everywhere else — other reporters (JUnit, HTML, console) pattern-match known types rather than blindly enumerating properties, so nothing regresses there.
• typeof(TUnitTestFramework).FullName! is safe here (non-generic, not nested in a generic type, never null).
• The regression tests correctly exercise both the failure path and the non-failure (opt-in-removed) path, and the env-var-nulling technique used with CliWrap correctly clears inherited variables for the child process.

Suggestion: extract the shared TRX-folding logic

ReportUnhandledException in src/TUnit.Engine/Framework/TUnitTestFramework.cs:141-166 duplicates the fold-and-build-TrxExceptionProperty logic that already exists in src/TUnit.Engine/Extensions/TestExtensions.cs:225-229:

var folded = FlattenedException.Wrap(exception);
propertyBag.Add(new TrxExceptionProperty(
    folded.Message,
    string.IsNullOrEmpty(folded.StackTrace) ? null : folded.StackTrace));

Both call sites now independently implement "fold the exception, then build a TrxExceptionProperty with a null-if-empty stack trace." A future change to how TRX exceptions are folded (truncation, redaction, a different fallback rule, etc.) only has a chance of being applied consistently if both sites are remembered and updated together — which is exactly the kind of drift that let the original bug (dropped results on cleanup failure) go unnoticed. Pulling this into a small shared helper (e.g. static TrxExceptionProperty BuildTrxExceptionProperty(Exception exception) next to FlattenedException) would remove that risk and make the two TRX-reporting paths visibly share one source of truth.

This is a maintainability nit, not a correctness issue — the current code is correct and safe to merge as-is.

[thomhurst]

Thanks @Sing303 !