Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest: Remove empty 'expected' files when blessing #134808

Merged
merged 1 commit into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 69 additions & 30 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -2576,15 +2591,22 @@ 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.
(true, Some(nl_e), Some(nl_a)) => expected[nl_e..] != actual[nl_a..],
_ => expected != actual,
};
if !are_different {
return 0;
return CompareOutcome::Same;
}

// Wrapper tools set by `runner` might provide extra output on failure,
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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:?}");
jieyouxu marked this conversation as resolved.
Show resolved Hide resolved
}
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.
Expand Down Expand Up @@ -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)
}
}
12 changes: 6 additions & 6 deletions src/tools/compiletest/src/runtest/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand Down Expand Up @@ -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,
);
}
Expand Down
7 changes: 6 additions & 1 deletion src/tools/compiletest/src/runtest/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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` \
Expand Down
Loading