From 8aacd1c6a86ba4211b541aaf5b6d270ebb8727fa Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 1 Dec 2024 23:57:09 -0500 Subject: [PATCH] compiletest: show the difference between the normalized output and the actual output for lines which didn't match example output: ``` failures: ---- [ui] tests/ui/layout/enum.rs stdout ---- diff of stderr: - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN } + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } 2 --> $DIR/enum.rs:9:1 3 | 4 LL | enum UninhabitedVariantAlign { Note: some mismatched output was normalized before being compared - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) } - --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:9:1 + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } ``` --- src/tools/compiletest/src/runtest.rs | 136 ++++++++++++++---- src/tools/compiletest/src/runtest/coverage.rs | 16 ++- src/tools/compiletest/src/runtest/ui.rs | 2 +- 3 files changed, 119 insertions(+), 35 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 84269fd44a1e0..7b11bf3b1219c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -22,7 +22,7 @@ use crate::common::{ UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir, output_base_dir, output_base_name, output_testname_unique, }; -use crate::compute_diff::{write_diff, write_filtered_diff}; +use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::read2::{Truncated, read2_abbreviated}; @@ -2295,17 +2295,31 @@ impl<'test> TestCx<'test> { match output_kind { TestOutput::Compile => { if !self.props.dont_check_compiler_stdout { - errors += - self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); } if !self.props.dont_check_compiler_stderr { - errors += - self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stderr_kind, + &normalized_stderr, + &stderr, + &expected_stderr, + ); } } TestOutput::Run => { - errors += self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); - errors += self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); + errors += + self.compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr); } } errors @@ -2533,7 +2547,13 @@ impl<'test> TestCx<'test> { } } - fn compare_output(&self, stream: &str, actual: &str, expected: &str) -> usize { + fn compare_output( + &self, + stream: &str, + actual: &str, + actual_unnormalized: &str, + expected: &str, + ) -> usize { let are_different = match (self.force_color_svg(), expected.find('\n'), actual.find('\n')) { // FIXME: We ignore the first line of SVG files // because the width parameter is non-deterministic. @@ -2590,28 +2610,14 @@ impl<'test> TestCx<'test> { if expected.is_empty() { println!("normalized {}:\n{}\n", stream, actual); } else { - println!("diff of {stream}:\n"); - if let Some(diff_command) = self.config.diff_command.as_deref() { - let mut args = diff_command.split_whitespace(); - let name = args.next().unwrap(); - match Command::new(name) - .args(args) - .args([&expected_path, &actual_path]) - .output() - { - Err(err) => { - self.fatal(&format!( - "failed to call custom diff command `{diff_command}`: {err}" - )); - } - Ok(output) => { - let output = String::from_utf8_lossy(&output.stdout); - print!("{output}"); - } - } - } else { - print!("{}", write_diff(expected, actual, 3)); - } + self.show_diff( + stream, + &expected_path, + &actual_path, + expected, + actual, + actual_unnormalized, + ); } } else { // Delete non-revision .stderr/.stdout file if revisions are used. @@ -2633,6 +2639,76 @@ impl<'test> TestCx<'test> { if self.config.bless { 0 } else { 1 } } + /// Returns whether to show the full stderr/stdout. + fn show_diff( + &self, + stream: &str, + expected_path: &Path, + actual_path: &Path, + expected: &str, + actual: &str, + actual_unnormalized: &str, + ) { + eprintln!("diff of {stream}:\n"); + if let Some(diff_command) = self.config.diff_command.as_deref() { + let mut args = diff_command.split_whitespace(); + let name = args.next().unwrap(); + match Command::new(name).args(args).args([expected_path, actual_path]).output() { + Err(err) => { + self.fatal(&format!( + "failed to call custom diff command `{diff_command}`: {err}" + )); + } + Ok(output) => { + let output = String::from_utf8_lossy(&output.stdout); + eprint!("{output}"); + } + } + } else { + eprint!("{}", write_diff(expected, actual, 3)); + } + + // NOTE: argument order is important, we need `actual` to be on the left so the line number match up when we compare it to `actual_unnormalized` below. + let diff_results = make_diff(actual, expected, 0); + + let (mut mismatches_normalized, mut mismatch_line_nos) = (String::new(), vec![]); + for hunk in diff_results { + let mut line_no = hunk.line_number; + for line in hunk.lines { + // NOTE: `Expected` is actually correct here, the argument order is reversed so our line numbers match up + if let DiffLine::Expected(normalized) = line { + mismatches_normalized += &normalized; + mismatches_normalized += "\n"; + mismatch_line_nos.push(line_no); + line_no += 1; + } + } + } + let mut mismatches_unnormalized = String::new(); + let diff_normalized = make_diff(actual, actual_unnormalized, 0); + for hunk in diff_normalized { + if mismatch_line_nos.contains(&hunk.line_number) { + for line in hunk.lines { + if let DiffLine::Resulting(unnormalized) = line { + mismatches_unnormalized += &unnormalized; + mismatches_unnormalized += "\n"; + } + } + } + } + + let normalized_diff = make_diff(&mismatches_normalized, &mismatches_unnormalized, 0); + // HACK: instead of checking if each hunk is empty, this only checks if the whole input is empty. we should be smarter about this so we don't treat added or removed output as normalized. + if !normalized_diff.is_empty() + && !mismatches_unnormalized.is_empty() + && !mismatches_normalized.is_empty() + { + eprintln!("Note: some mismatched output was normalized before being compared"); + // FIXME: respect diff_command + eprint!("{}", write_diff(&mismatches_unnormalized, &mismatches_normalized, 0)); + } + } + fn check_and_prune_duplicate_outputs( &self, proc_res: &ProcRes, diff --git a/src/tools/compiletest/src/runtest/coverage.rs b/src/tools/compiletest/src/runtest/coverage.rs index 961a160298631..030ca5ebb2474 100644 --- a/src/tools/compiletest/src/runtest/coverage.rs +++ b/src/tools/compiletest/src/runtest/coverage.rs @@ -39,8 +39,12 @@ impl<'test> TestCx<'test> { let expected_coverage_dump = self.load_expected_output(kind); let actual_coverage_dump = self.normalize_output(&proc_res.stdout, &[]); - let coverage_dump_errors = - self.compare_output(kind, &actual_coverage_dump, &expected_coverage_dump); + let coverage_dump_errors = self.compare_output( + kind, + &actual_coverage_dump, + &proc_res.stdout, + &expected_coverage_dump, + ); if coverage_dump_errors > 0 { self.fatal_proc_rec( @@ -135,8 +139,12 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec(&err, &proc_res); }); - let coverage_errors = - self.compare_output(kind, &normalized_actual_coverage, &expected_coverage); + let coverage_errors = self.compare_output( + kind, + &normalized_actual_coverage, + &proc_res.stdout, + &expected_coverage, + ); if coverage_errors > 0 { self.fatal_proc_rec( diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index bb747c6802926..172b1e32aad3b 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -100,7 +100,7 @@ impl TestCx<'_> { ) }); - errors += self.compare_output("fixed", &fixed_code, &expected_fixed); + errors += self.compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed); } else if !expected_fixed.is_empty() { panic!( "the `//@ run-rustfix` directive wasn't found but a `*.fixed` \