From 5bb727a66a293fc77062a118e139affbd137c744 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Thu, 26 Dec 2024 23:00:13 +0000 Subject: [PATCH] compiletest: Remove/don't write empty 'expected' files --- src/tools/compiletest/src/runtest.rs | 99 +++++++++++++------ src/tools/compiletest/src/runtest/coverage.rs | 12 +-- src/tools/compiletest/src/runtest/ui.rs | 7 +- 3 files changed, 81 insertions(+), 37 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 108fde1c89955..7084c407a642c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -213,7 +213,7 @@ fn remove_and_create_dir_all(path: &Path) { fs::create_dir_all(path).unwrap(); } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] struct TestCx<'test> { config: &'test Config, props: &'test TestProps, @@ -2318,32 +2318,47 @@ impl<'test> TestCx<'test> { match output_kind { TestOutput::Compile => { if !self.props.dont_check_compiler_stdout { - errors += self.compare_output( + if self + .compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ) + .should_error() + { + errors += 1; + } + } + if !self.props.dont_check_compiler_stderr { + if self + .compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr) + .should_error() + { + errors += 1; + } + } + } + TestOutput::Run => { + if self + .compare_output( stdout_kind, &normalized_stdout, &proc_res.stdout, &expected_stdout, - ); + ) + .should_error() + { + errors += 1; } - if !self.props.dont_check_compiler_stderr { - errors += self.compare_output( - stderr_kind, - &normalized_stderr, - &stderr, - &expected_stderr, - ); + + if self + .compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr) + .should_error() + { + errors += 1; } } - TestOutput::Run => { - 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 } @@ -2576,7 +2591,14 @@ impl<'test> TestCx<'test> { actual: &str, actual_unnormalized: &str, expected: &str, - ) -> usize { + ) -> CompareOutcome { + let expected_path = + expected_output_path(self.testpaths, self.revision, &self.config.compare_mode, stream); + + if self.config.bless && actual.is_empty() && expected_path.exists() { + self.delete_file(&expected_path); + } + 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. @@ -2584,7 +2606,7 @@ impl<'test> TestCx<'test> { _ => expected != actual, }; if !are_different { - return 0; + return CompareOutcome::Same; } // Wrapper tools set by `runner` might provide extra output on failure, @@ -2600,7 +2622,7 @@ impl<'test> TestCx<'test> { used.retain(|line| actual_lines.contains(line)); // check if `expected` contains a subset of the lines of `actual` if used.len() == expected_lines.len() && (expected.is_empty() == actual.is_empty()) { - return 0; + return CompareOutcome::Same; } if expected_lines.is_empty() { // if we have no lines to check, force a full overwite @@ -2626,9 +2648,6 @@ impl<'test> TestCx<'test> { } println!("Saved the actual {stream} to {actual_path:?}"); - let expected_path = - expected_output_path(self.testpaths, self.revision, &self.config.compare_mode, stream); - if !self.config.bless { if expected.is_empty() { println!("normalized {}:\n{}\n", stream, actual); @@ -2651,15 +2670,17 @@ impl<'test> TestCx<'test> { self.delete_file(&old); } - if let Err(err) = fs::write(&expected_path, &actual) { - self.fatal(&format!("failed to write {stream} to `{expected_path:?}`: {err}")); + if !actual.is_empty() { + if let Err(err) = fs::write(&expected_path, &actual) { + self.fatal(&format!("failed to write {stream} to `{expected_path:?}`: {err}")); + } + println!("Blessing the {stream} of {test_name} in {expected_path:?}"); } - println!("Blessing the {stream} of {test_name} in {expected_path:?}"); } println!("\nThe actual {0} differed from the expected {0}.", stream); - if self.config.bless { 0 } else { 1 } + if self.config.bless { CompareOutcome::Blessed } else { CompareOutcome::Differed } } /// Returns whether to show the full stderr/stdout. @@ -2885,3 +2906,21 @@ enum AuxType { Dylib, ProcMacro, } + +/// Outcome of comparing a stream to a blessed file, +/// e.g. `.stderr` and `.fixed`. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum CompareOutcome { + /// Expected and actual outputs are the same + Same, + /// Outputs differed but were blessed + Blessed, + /// Outputs differed and an error should be emitted + Differed, +} + +impl CompareOutcome { + fn should_error(&self) -> bool { + matches!(self, CompareOutcome::Differed) + } +} diff --git a/src/tools/compiletest/src/runtest/coverage.rs b/src/tools/compiletest/src/runtest/coverage.rs index 030ca5ebb2474..56fc5baf5f248 100644 --- a/src/tools/compiletest/src/runtest/coverage.rs +++ b/src/tools/compiletest/src/runtest/coverage.rs @@ -39,16 +39,16 @@ 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( + let coverage_dump_compare_outcome = self.compare_output( kind, &actual_coverage_dump, &proc_res.stdout, &expected_coverage_dump, ); - if coverage_dump_errors > 0 { + if coverage_dump_compare_outcome.should_error() { self.fatal_proc_rec( - &format!("{coverage_dump_errors} errors occurred comparing coverage output."), + &format!("an error occurred comparing coverage output."), &proc_res, ); } @@ -139,16 +139,16 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec(&err, &proc_res); }); - let coverage_errors = self.compare_output( + let coverage_dump_compare_outcome = self.compare_output( kind, &normalized_actual_coverage, &proc_res.stdout, &expected_coverage, ); - if coverage_errors > 0 { + if coverage_dump_compare_outcome.should_error() { self.fatal_proc_rec( - &format!("{} errors occurred comparing coverage output.", coverage_errors), + &format!("an error occurred comparing coverage output."), &proc_res, ); } diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index 10528de427d09..0c6d46188e6f4 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -100,7 +100,12 @@ impl TestCx<'_> { ) }); - errors += self.compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed); + if self + .compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed) + .should_error() + { + errors += 1; + } } else if !expected_fixed.is_empty() { panic!( "the `//@ run-rustfix` directive wasn't found but a `*.fixed` \