From 2d9dc1ab9e0d25bf7fa6b8696f8909a561dd267f Mon Sep 17 00:00:00 2001 From: Tailoo Date: Thu, 18 Jun 2026 22:46:27 +0200 Subject: [PATCH 1/2] fix(dotnet): stop duplicating failures on failing test runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On failing `dotnet test` runs the orchestrator prepended the full raw stdout ahead of the filtered summary. The `Failed Tests:` section already reproduces each failure (name + message + clipped stack) parsed from TRX/console, so every failure was printed twice — inflating output +65% vs raw, scaling linearly with failure count (issue #2501). Gate the raw-stdout prepend behind `test_needs_raw_fallback`: skip it when the structured section carries detail, keep it when the filter is blind (no failures parsed, or a parsed failure has empty detail — e.g. a self-closing with no , or a build failure / crash where nothing reaches failed_tests). Extracts two pure, unit-tested fns (`test_needs_raw_fallback`, `compose_failure_output`) so the orchestration decision is covered without running dotnet. Closes #2501 Co-Authored-By: Claude Opus 4.8 --- src/cmds/dotnet/dotnet_cmd.rs | 181 +++++++++++++++++++++++++++++----- 1 file changed, 159 insertions(+), 22 deletions(-) diff --git a/src/cmds/dotnet/dotnet_cmd.rs b/src/cmds/dotnet/dotnet_cmd.rs index f90db8b02..5105cc500 100644 --- a/src/cmds/dotnet/dotnet_cmd.rs +++ b/src/cmds/dotnet/dotnet_cmd.rs @@ -138,7 +138,7 @@ fn run_dotnet_with_binlog(subcommand: &str, args: &[String], verbose: u8) -> Res let raw = format!("{}\n{}", result.stdout, result.stderr); let command_success = result.success(); - let filtered = match subcommand { + let (filtered, needs_raw_fallback) = match subcommand { "build" => { let binlog_summary = if should_expect_binlog && binlog_path.exists() { normalize_build_summary( @@ -151,7 +151,7 @@ fn run_dotnet_with_binlog(subcommand: &str, args: &[String], verbose: u8) -> Res let raw_summary = normalize_build_summary(binlog::parse_build_from_text(&raw), command_success); let summary = merge_build_summaries(binlog_summary, raw_summary); - format_build_output(&summary, &binlog_path) + (format_build_output(&summary, &binlog_path), true) } "test" => { // First try to parse from binlog/console output @@ -181,11 +181,18 @@ fn run_dotnet_with_binlog(subcommand: &str, args: &[String], verbose: u8) -> Res let raw_diagnostics = normalize_build_summary(binlog::parse_build_from_text(&raw), command_success); let test_build_summary = merge_build_summaries(binlog_diagnostics, raw_diagnostics); - format_test_output( - &summary, - &test_build_summary.errors, - &test_build_summary.warnings, - &binlog_path, + // The `Failed Tests:` section already carries failure detail parsed from + // TRX/console; skip the raw-stdout prepend when it would only duplicate it. + // See issue #2501. + let needs_raw = test_needs_raw_fallback(&summary); + ( + format_test_output( + &summary, + &test_build_summary.errors, + &test_build_summary.warnings, + &binlog_path, + ), + needs_raw, ) } "restore" => { @@ -203,24 +210,21 @@ fn run_dotnet_with_binlog(subcommand: &str, args: &[String], verbose: u8) -> Res let (raw_errors, raw_warnings) = binlog::parse_restore_issues_from_text(&raw); - format_restore_output(&summary, &raw_errors, &raw_warnings, &binlog_path) + ( + format_restore_output(&summary, &raw_errors, &raw_warnings, &binlog_path), + true, + ) } - _ => raw.clone(), + _ => (raw.clone(), true), }; - let output_to_print = if !command_success { - let stdout_trimmed = result.stdout.trim(); - let stderr_trimmed = result.stderr.trim(); - if !stdout_trimmed.is_empty() { - format!("{}\n\n{}", stdout_trimmed, filtered) - } else if !stderr_trimmed.is_empty() { - format!("{}\n\n{}", stderr_trimmed, filtered) - } else { - filtered - } - } else { - filtered - }; + let output_to_print = compose_failure_output( + command_success, + needs_raw_fallback, + &result.stdout, + &result.stderr, + &filtered, + ); println!("{}", output_to_print); @@ -1076,6 +1080,50 @@ fn format_build_output(summary: &binlog::BuildSummary, _binlog_path: &Path) -> S .join("\n") } +/// Decides whether the raw stdout/stderr should be prepended ahead of the +/// filtered `dotnet test` summary on a failing run. +/// +/// On failure the orchestrator can prepend the raw command output as a safety +/// net. For `test`, the filtered `Failed Tests:` section already reproduces each +/// failure (name + message + clipped stack) parsed from TRX/console, so the +/// prepend would only duplicate every failure block — the source of the +65% +/// inflation in issue #2501. +/// +/// Keep the raw fallback only when the structured section can't stand on its own: +/// no failures were parsed, or some parsed failure has no detail (filter blind). +fn test_needs_raw_fallback(summary: &binlog::TestSummary) -> bool { + summary.failed_tests.is_empty() + || summary.failed_tests.iter().any(|t| t.details.is_empty()) +} + +/// Composes the final output for a (possibly failing) run: the filtered summary, +/// optionally prefixed with raw stdout/stderr as a fallback. +/// +/// On success, or when `needs_raw_fallback` is false, only the filtered summary +/// is emitted. Otherwise the raw stdout (or stderr if stdout is empty) is +/// prepended so nothing is lost when the filter couldn't capture the failure. +fn compose_failure_output( + command_success: bool, + needs_raw_fallback: bool, + stdout: &str, + stderr: &str, + filtered: &str, +) -> String { + if command_success || !needs_raw_fallback { + return filtered.to_string(); + } + + let stdout_trimmed = stdout.trim(); + let stderr_trimmed = stderr.trim(); + if !stdout_trimmed.is_empty() { + format!("{}\n\n{}", stdout_trimmed, filtered) + } else if !stderr_trimmed.is_empty() { + format!("{}\n\n{}", stderr_trimmed, filtered) + } else { + filtered.to_string() + } +} + /// Format the test summary for stdout. /// /// `_binlog_path` is intentionally unused — the binlog is a temporary file @@ -1405,6 +1453,95 @@ mod tests { assert!(output.contains("MyTests.ShouldFail")); } + // Regression tests for issue #2501: on failing test runs the raw stdout was + // prepended in front of the filtered `Failed Tests:` section, duplicating every + // failure block (+65% vs raw). `test_needs_raw_fallback` must suppress the raw + // prepend when the structured section already carries failure detail, while + // keeping it when the filter couldn't capture the failures. + + #[test] + fn test_needs_raw_fallback_false_when_failures_have_detail() { + let summary = binlog::TestSummary { + passed: 717, + failed: 5, + skipped: 0, + total: 722, + project_count: 1, + failed_tests: vec![binlog::FailedTest { + name: "MyTests.HasRestriction".to_string(), + details: vec!["Assert.True() Failure".to_string()], + }], + duration_text: Some("2 s".to_string()), + }; + assert!(!test_needs_raw_fallback(&summary)); + } + + #[test] + fn test_needs_raw_fallback_true_when_no_failures_parsed() { + // Build failure / crash: command failed but nothing landed in failed_tests. + let summary = binlog::TestSummary { + failed: 1, + total: 1, + ..Default::default() + }; + assert!(test_needs_raw_fallback(&summary)); + } + + #[test] + fn test_needs_raw_fallback_true_when_a_failure_lacks_detail() { + // Self-closing with no : name only, no detail. + let summary = binlog::TestSummary { + failed: 1, + total: 1, + failed_tests: vec![binlog::FailedTest { + name: "MyTests.NoDetail".to_string(), + details: Vec::new(), + }], + ..Default::default() + }; + assert!(test_needs_raw_fallback(&summary)); + } + + #[test] + fn test_compose_failure_output_drops_raw_when_no_fallback_needed() { + // The raw stdout contains the inline failure; the filtered section also + // contains it. With needs_raw_fallback=false, the failure must appear once. + let raw_stdout = " failed MyTests.HasRestriction\n Assert.True() Failure"; + let filtered = + "Failed Tests:\n MyTests.HasRestriction\n Assert.True() Failure\n\nfail dotnet test: 717 passed, 5 failed"; + let output = compose_failure_output(false, false, raw_stdout, "", filtered); + + assert_eq!(output, filtered); + assert_eq!(output.matches("HasRestriction").count(), 1); + } + + #[test] + fn test_compose_failure_output_prepends_raw_when_fallback_needed() { + let raw_stdout = "Build FAILED.\n Program.cs(1,1): error CS1002"; + let filtered = "fail dotnet test: 0 passed, 1 failed"; + // command_success=false, needs_raw_fallback=true → raw is prepended. + let output = compose_failure_output(false, true, raw_stdout, "", filtered); + + assert!(output.starts_with("Build FAILED.")); + assert!(output.ends_with(filtered)); + } + + #[test] + fn test_compose_failure_output_uses_stderr_when_stdout_empty() { + let filtered = "fail dotnet test: 0 passed, 1 failed"; + let output = compose_failure_output(false, true, " ", "boom on stderr", filtered); + + assert!(output.starts_with("boom on stderr")); + assert!(output.ends_with(filtered)); + } + + #[test] + fn test_compose_failure_output_returns_filtered_on_success() { + let filtered = "ok dotnet test: 722 tests passed"; + let output = compose_failure_output(true, true, "ignored raw", "ignored", filtered); + assert_eq!(output, filtered); + } + #[test] fn test_format_test_output_surfaces_warnings() { let summary = binlog::TestSummary { From 5e7eab5846cfe2de1f0d0c2a7d6c38c8de6c65e5 Mon Sep 17 00:00:00 2001 From: Tailoo Date: Sun, 21 Jun 2026 22:41:00 +0200 Subject: [PATCH 2/2] fix(dotnet): keep raw fallback when parsed failures incomplete test_needs_raw_fallback dropped the raw stdout prepend whenever every parsed failure carried detail, but never checked the list was complete against summary.failed. A run reporting 5 failures but parsing only 3 detailed blocks would silently lose the 2 unparsed failures. Add a `failed_tests.len() < summary.failed` clause so the raw fallback is kept when the structured section is numerically incomplete. Add a regression test and fix an inconsistent existing fixture (failed: 5 with a single parsed entry) that the new clause correctly flagged. Co-Authored-By: Claude Opus 4.8 --- src/cmds/dotnet/dotnet_cmd.rs | 48 +++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/src/cmds/dotnet/dotnet_cmd.rs b/src/cmds/dotnet/dotnet_cmd.rs index 5105cc500..232f957af 100644 --- a/src/cmds/dotnet/dotnet_cmd.rs +++ b/src/cmds/dotnet/dotnet_cmd.rs @@ -1090,9 +1090,12 @@ fn format_build_output(summary: &binlog::BuildSummary, _binlog_path: &Path) -> S /// inflation in issue #2501. /// /// Keep the raw fallback only when the structured section can't stand on its own: -/// no failures were parsed, or some parsed failure has no detail (filter blind). +/// no failures were parsed, the parsed list is shorter than `summary.failed` +/// (some failures never made it into the section), or some parsed failure has no +/// detail (filter blind). fn test_needs_raw_fallback(summary: &binlog::TestSummary) -> bool { summary.failed_tests.is_empty() + || summary.failed_tests.len() < summary.failed || summary.failed_tests.iter().any(|t| t.details.is_empty()) } @@ -1461,21 +1464,56 @@ mod tests { #[test] fn test_needs_raw_fallback_false_when_failures_have_detail() { + // Every reported failure was parsed and carries detail: the structured + // section stands alone, so the raw prepend is dropped (issue #2501). + let failed_tests: Vec = (0..5) + .map(|i| binlog::FailedTest { + name: format!("MyTests.Case{i}"), + details: vec!["Assert.True() Failure".to_string()], + }) + .collect(); let summary = binlog::TestSummary { passed: 717, failed: 5, skipped: 0, total: 722, project_count: 1, - failed_tests: vec![binlog::FailedTest { - name: "MyTests.HasRestriction".to_string(), - details: vec!["Assert.True() Failure".to_string()], - }], + failed_tests, duration_text: Some("2 s".to_string()), }; assert!(!test_needs_raw_fallback(&summary)); } + #[test] + fn test_needs_raw_fallback_true_when_parsed_list_incomplete() { + // summary.failed reports 5, but only 3 blocks were parsed (each with + // detail). The 2 missing failures live only in raw stdout — keep the + // fallback so they aren't silently dropped. + let summary = binlog::TestSummary { + passed: 717, + failed: 5, + skipped: 0, + total: 722, + project_count: 1, + failed_tests: vec![ + binlog::FailedTest { + name: "MyTests.One".to_string(), + details: vec!["Assert.True() Failure".to_string()], + }, + binlog::FailedTest { + name: "MyTests.Two".to_string(), + details: vec!["Assert.Equal() Failure".to_string()], + }, + binlog::FailedTest { + name: "MyTests.Three".to_string(), + details: vec!["Assert.Null() Failure".to_string()], + }, + ], + duration_text: Some("2 s".to_string()), + }; + assert!(test_needs_raw_fallback(&summary)); + } + #[test] fn test_needs_raw_fallback_true_when_no_failures_parsed() { // Build failure / crash: command failed but nothing landed in failed_tests.