Skip to content

Commit 50fd83c

Browse files
jyn514jyn514
committed
Make compiletest diffs more useful
- Outside of CI, don't show the full rustc command unless --verbose is passed. - Outside of CI, only show one of the diff and the full stderr. The heuristic for whether to show the diff is: - it takes up less than 10 lines, or - it takes up less than a tenth of the total stderr output - If we show the diff, also show the difference between the normalized output and the actual output for lines which didn't match. before: ``` failures: ---- [ui] tests/ui/layout/enum.rs stdout ---- Saved the actual stderr to "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/enum.stderr" 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 { The actual stderr differed from the expected stderr. To update references, rerun the tests and pass the `--bless` flag To only update this specific test, also pass `--test-args layout/enum.rs` error: 1 errors occurred comparing output. status: exit status: 1 command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/home/jyn/src/rust2/tests/ui/layout/enum.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/jyn/.local/lib/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/home/jyn/src/rust2/vendor" "--sysroot" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/ui/layout/enum" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" stdout: none --- stderr ------------------------------- error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) } --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:9:1 | LL | enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: size: Size(16 bytes) --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:15:1 | LL | enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: abi: ScalarPair(Initialized { value: Int(I8, false), valid_range: 0..=1 }, Initialized { value: Int(I8, false), valid_range: 0..=255 }) --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:21:1 | LL | enum ScalarPairDifferingSign { //~ERROR: abi: ScalarPair | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 3 previous errors ------------------------------------------ failures: [ui] tests/ui/layout/enum.rs ``` after: ``` failures: ---- [ui] tests/ui/layout/enum.rs stdout ---- Saved the actual stderr to "/home/jyn/src/rust2/build/x86_64-unknown-linux-gnu/test/ui/layout/enum/enum.stderr" 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 } The actual stderr differed from the expected stderr. To update references, rerun the tests and pass the `--bless` flag To only update this specific test, also pass `--test-args layout/enum.rs` error: 1 errors occurred comparing output. failures: [ui] tests/ui/layout/enum.rs [ui] tests/ui/proc-macro/load-panic-backtrace.rs ```
1 parent 5bbbc09 commit 50fd83c

File tree

3 files changed

+153
-54
lines changed

3 files changed

+153
-54
lines changed

‎src/tools/compiletest/src/runtest.rs‎

Lines changed: 134 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::sync::Arc;
1111
use std::{env, iter, str};
1212

1313
use anyhow::Context;
14+
use build_helper::ci::CiEnv;
1415
use colored::Colorize;
1516
use regex::{Captures, Regex};
1617
use tracing::*;
@@ -22,7 +23,7 @@ use crate::common::{
2223
UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir,
2324
output_base_dir, output_base_name, output_testname_unique,
2425
};
25-
use crate::compute_diff::{write_diff, write_filtered_diff};
26+
use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff};
2627
use crate::errors::{self, Error, ErrorKind};
2728
use crate::header::TestProps;
2829
use crate::read2::{Truncated, read2_abbreviated};
@@ -2292,17 +2293,31 @@ impl<'test> TestCx<'test> {
22922293
match output_kind {
22932294
TestOutput::Compile => {
22942295
if !self.props.dont_check_compiler_stdout {
2295-
errors +=
2296-
self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout);
2296+
errors += self.compare_output(
2297+
stdout_kind,
2298+
&normalized_stdout,
2299+
&proc_res.stdout,
2300+
&expected_stdout,
2301+
);
22972302
}
22982303
if !self.props.dont_check_compiler_stderr {
2299-
errors +=
2300-
self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr);
2304+
errors += self.compare_output(
2305+
stderr_kind,
2306+
&normalized_stderr,
2307+
&stderr,
2308+
&expected_stderr,
2309+
);
23012310
}
23022311
}
23032312
TestOutput::Run => {
2304-
errors += self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout);
2305-
errors += self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr);
2313+
errors += self.compare_output(
2314+
stdout_kind,
2315+
&normalized_stdout,
2316+
&proc_res.stdout,
2317+
&expected_stdout,
2318+
);
2319+
errors +=
2320+
self.compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr);
23062321
}
23072322
}
23082323
errors
@@ -2530,7 +2545,13 @@ impl<'test> TestCx<'test> {
25302545
}
25312546
}
25322547

2533-
fn compare_output(&self, stream: &str, actual: &str, expected: &str) -> usize {
2548+
fn compare_output(
2549+
&self,
2550+
stream: &str,
2551+
actual: &str,
2552+
actual_unnormalized: &str,
2553+
expected: &str,
2554+
) -> usize {
25342555
let are_different = match (self.force_color_svg(), expected.find('\n'), actual.find('\n')) {
25352556
// FIXME: We ignore the first line of SVG files
25362557
// because the width parameter is non-deterministic.
@@ -2590,27 +2611,16 @@ impl<'test> TestCx<'test> {
25902611
if expected.is_empty() {
25912612
println!("normalized {}:\n{}\n", stream, actual);
25922613
} else {
2593-
println!("diff of {stream}:\n");
2594-
if let Some(diff_command) = self.config.diff_command.as_deref() {
2595-
let mut args = diff_command.split_whitespace();
2596-
let name = args.next().unwrap();
2597-
match Command::new(name)
2598-
.args(args)
2599-
.args([&expected_path, &actual_path])
2600-
.output()
2601-
{
2602-
Err(err) => {
2603-
self.fatal(&format!(
2604-
"failed to call custom diff command `{diff_command}`: {err}"
2605-
));
2606-
}
2607-
Ok(output) => {
2608-
let output = String::from_utf8_lossy(&output.stdout);
2609-
print!("{output}");
2610-
}
2611-
}
2612-
} else {
2613-
print!("{}", write_diff(expected, actual, 3));
2614+
let show_full = self.show_diff(
2615+
stream,
2616+
&expected_path,
2617+
&actual_path,
2618+
expected,
2619+
actual,
2620+
actual_unnormalized,
2621+
);
2622+
if show_full || self.config.verbose {
2623+
println!("{}", ProcRes::render(stream, actual));
26142624
}
26152625
}
26162626
} else {
@@ -2633,6 +2643,85 @@ impl<'test> TestCx<'test> {
26332643
if self.config.bless { 0 } else { 1 }
26342644
}
26352645

2646+
/// Returns whether to show the full stderr/stdout.
2647+
fn show_diff(
2648+
&self,
2649+
stream: &str,
2650+
expected_path: &Path,
2651+
actual_path: &Path,
2652+
expected: &str,
2653+
actual: &str,
2654+
actual_unnormalized: &str,
2655+
) -> bool {
2656+
let context_size = 3;
2657+
// 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.
2658+
let diff_results = make_diff(actual, expected, context_size);
2659+
let diff_lines: usize = diff_results.iter().map(|hunk| hunk.lines.len()).sum();
2660+
let show_diff = diff_lines < 10 || diff_lines < expected.lines().count() / 10;
2661+
if !show_diff && !CiEnv::is_ci() {
2662+
// don't even bother showing a diff if too many lines are changed, it won't be helpful.
2663+
let diff_cmd = self.config.diff_command.as_deref().unwrap_or("diff");
2664+
println!(
2665+
"too many lines changed; hiding diff. use `{diff_cmd} {expected_path:?} {actual_path:?}` to view anyway."
2666+
);
2667+
return true;
2668+
}
2669+
2670+
println!("diff of {stream}:\n");
2671+
if let Some(diff_command) = self.config.diff_command.as_deref() {
2672+
let mut args = diff_command.split_whitespace();
2673+
let name = args.next().unwrap();
2674+
match Command::new(name).args(args).args([expected_path, actual_path]).output() {
2675+
Err(err) => {
2676+
self.fatal(&format!(
2677+
"failed to call custom diff command `{diff_command}`: {err}"
2678+
));
2679+
}
2680+
Ok(output) => {
2681+
let output = String::from_utf8_lossy(&output.stdout);
2682+
print!("{output}");
2683+
}
2684+
}
2685+
} else {
2686+
print!("{}", write_diff(expected, actual, context_size));
2687+
}
2688+
2689+
let (mut mismatches_normalized, mut mismatch_line_nos) = (String::new(), vec![]);
2690+
for hunk in diff_results {
2691+
let mut line_no = hunk.line_number;
2692+
for line in hunk.lines {
2693+
// NOTE: `Expected` is actually correct here, the argument order is reversed so our line numbers match up
2694+
if let DiffLine::Expected(normalized) = line {
2695+
mismatches_normalized += &normalized;
2696+
mismatches_normalized += "\n";
2697+
mismatch_line_nos.push(line_no);
2698+
line_no += 1;
2699+
}
2700+
}
2701+
}
2702+
let mut mismatches_unnormalized = String::new();
2703+
let diff_normalized = make_diff(actual, actual_unnormalized, 0);
2704+
for hunk in diff_normalized {
2705+
if mismatch_line_nos.contains(&hunk.line_number) {
2706+
for line in hunk.lines {
2707+
if let DiffLine::Resulting(unnormalized) = line {
2708+
mismatches_unnormalized += &unnormalized;
2709+
mismatches_unnormalized += "\n";
2710+
}
2711+
}
2712+
}
2713+
}
2714+
2715+
let normalized_diff = make_diff(&mismatches_normalized, &mismatches_unnormalized, 0);
2716+
if !normalized_diff.is_empty() {
2717+
println!("Note: some mismatched output was normalized before being compared");
2718+
// FIXME: respect diff_command
2719+
print!("{}", write_diff(&mismatches_unnormalized, &mismatches_normalized, 0));
2720+
}
2721+
2722+
CiEnv::is_ci()
2723+
}
2724+
26362725
fn check_and_prune_duplicate_outputs(
26372726
&self,
26382727
proc_res: &ProcRes,
@@ -2749,28 +2838,28 @@ pub struct ProcRes {
27492838
}
27502839

27512840
impl ProcRes {
2752-
pub fn print_info(&self) {
2753-
fn render(name: &str, contents: &str) -> String {
2754-
let contents = json::extract_rendered(contents);
2755-
let contents = contents.trim_end();
2756-
if contents.is_empty() {
2757-
format!("{name}: none")
2758-
} else {
2759-
format!(
2760-
"\
2761-
--- {name} -------------------------------\n\
2762-
{contents}\n\
2763-
------------------------------------------",
2764-
)
2765-
}
2841+
pub fn render(name: &str, contents: &str) -> String {
2842+
let contents = json::extract_rendered(contents);
2843+
let contents = contents.trim_end();
2844+
if contents.is_empty() {
2845+
format!("{name}: none")
2846+
} else {
2847+
format!(
2848+
"\
2849+
--- {name} -------------------------------\n\
2850+
{contents}\n\
2851+
------------------------------------------",
2852+
)
27662853
}
2854+
}
27672855

2856+
pub fn print_info(&self) {
27682857
println!(
27692858
"status: {}\ncommand: {}\n{}\n{}\n",
27702859
self.status,
27712860
self.cmdline,
2772-
render("stdout", &self.stdout),
2773-
render("stderr", &self.stderr),
2861+
Self::render("stdout", &self.stdout),
2862+
Self::render("stderr", &self.stderr),
27742863
);
27752864
}
27762865

‎src/tools/compiletest/src/runtest/coverage.rs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,12 @@ impl<'test> TestCx<'test> {
3939
let expected_coverage_dump = self.load_expected_output(kind);
4040
let actual_coverage_dump = self.normalize_output(&proc_res.stdout, &[]);
4141

42-
let coverage_dump_errors =
43-
self.compare_output(kind, &actual_coverage_dump, &expected_coverage_dump);
42+
let coverage_dump_errors = self.compare_output(
43+
kind,
44+
&actual_coverage_dump,
45+
&proc_res.stdout,
46+
&expected_coverage_dump,
47+
);
4448

4549
if coverage_dump_errors > 0 {
4650
self.fatal_proc_rec(
@@ -135,8 +139,12 @@ impl<'test> TestCx<'test> {
135139
self.fatal_proc_rec(&err, &proc_res);
136140
});
137141

138-
let coverage_errors =
139-
self.compare_output(kind, &normalized_actual_coverage, &expected_coverage);
142+
let coverage_errors = self.compare_output(
143+
kind,
144+
&normalized_actual_coverage,
145+
&proc_res.stdout,
146+
&expected_coverage,
147+
);
140148

141149
if coverage_errors > 0 {
142150
self.fatal_proc_rec(

‎src/tools/compiletest/src/runtest/ui.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::HashSet;
22
use std::fs::OpenOptions;
33
use std::io::Write;
44

5+
use build_helper::ci::CiEnv;
56
use rustfix::{Filter, apply_suggestions, get_suggestions_from_json};
67
use tracing::debug;
78

@@ -100,7 +101,7 @@ impl TestCx<'_> {
100101
)
101102
});
102103

103-
errors += self.compare_output("fixed", &fixed_code, &expected_fixed);
104+
errors += self.compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed);
104105
} else if !expected_fixed.is_empty() {
105106
panic!(
106107
"the `//@ run-rustfix` directive wasn't found but a `*.fixed` \
@@ -116,10 +117,11 @@ impl TestCx<'_> {
116117
"To only update this specific test, also pass `--test-args {}`",
117118
relative_path_to_file.display(),
118119
);
119-
self.fatal_proc_rec(
120-
&format!("{} errors occurred comparing output.", errors),
121-
&proc_res,
122-
);
120+
self.error(&format!("{} errors occurred comparing output.", errors));
121+
if self.config.verbose || CiEnv::is_ci() {
122+
println!("status: {}\ncommand: {}", proc_res.status, proc_res.cmdline);
123+
}
124+
std::panic::resume_unwind(Box::new(()));
123125
}
124126

125127
let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);

0 commit comments

Comments
 (0)