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.

Sign up for GitHub

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: show the difference between the normalized output and the actual output for lines which didn't match #133733

Merged
merged 1 commit into from
Dec 9, 2024
Merged

compiletest: show the difference between the normalized output and the actual output for lines which didn't match #133733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
136 changes: 106 additions & 30 deletions src/tools/compiletest/src/runtest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
16 changes: 12 additions & 4 deletions src/tools/compiletest/src/runtest/coverage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest/ui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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` \
Expand Down
Loading