fix(dotnet): deduplicate failing test output (#2501)#2511
Closed
outofrange-consulting wants to merge 1 commit into
Closed
fix(dotnet): deduplicate failing test output (#2501)#2511outofrange-consulting wants to merge 1 commit into
outofrange-consulting wants to merge 1 commit into
Conversation
On a failing `dotnet test` run, run_dotnet_with_binlog prepended the entire raw stdout in front of the filtered output. The raw stdout already prints every per-test failure inline (xUnit v3 / MTP), and format_test_output independently rebuilds those same failures into its `Failed Tests:` section — so each failure was printed twice, inflating output by ~65% on the reported benchmark (the opposite of RTK's purpose). The more failures, the worse the linear duplication. Extract the output-composition decision into a pure, unit-testable `compose_dotnet_output`. For `test`, drop the raw-stdout prepend only when the structured section is a complete substitute — i.e. failed_tests is non-empty and every failure carries detail. When any failure lacks detail (self-closing UnitTestResult, crash with no TRX, counts-unavailable run) or for build/restore, the raw fallback is kept exactly as before, so no information is ever lost. The `.all()` guard (not `.any()`) preserves the raw message for name-only failures. The bug lived in the orchestration path, which had no test coverage; the existing tests only exercised format_test_output in isolation. Add eight regression tests covering every row of the analysis, including a benchmark -shaped case asserting each failure string appears exactly once.
Author
|
Closing as fixed in #2502 |
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.
Summary
Fixes #2501 —
rtk dotnet testprinted every failure twice on failing runs, inflating output by ~65% on the reported benchmark (the opposite of RTK's purpose). The regression was specific to failing runs; all-green runs were unaffected.Root cause
In
run_dotnet_with_binlog(src/cmds/dotnet/dotnet_cmd.rs), the failure branch prepended the entire raw stdout in front of the filtered output:For
test, that collides with the filter's own output. The raw stdout already prints every per-test failure inline (xUnit v3 / MTP), andformat_test_outputindependently rebuilds the same failures into itsFailed Tests:section — so each failure appeared once from the raw prepend and once from the structured section.build/restoresource failures from a single place, so they never duplicated.Fix
Extract the output-composition decision into a pure, unit-testable
compose_dotnet_output. Fortest, the raw-stdout prepend is dropped only when the structured section is a complete substitute — i.e.failed_testsis non-empty and every failure carries detail. Otherwise the raw fallback is kept exactly as before, so nothing is ever lost:failed_testsdetails(self-closingUnitTestResult)build/restoreThe
.all(|t| !t.details.is_empty())guard (not.any()) preserves the raw message for name-only failures. Behavior is byte-for-byte identical to the previous inline logic in every case except the de-duplicated one.Tests
The bug lived in the orchestration path, which had no test coverage (existing tests only exercised
format_test_outputin isolation). Added 8 regression tests covering every row above, including a benchmark-shaped case asserting the failure assertion string appears exactly once (the issue's measured proof: raw=5, buggy rtk=10, fixed=5).Verification