From a41f31e27069587f1a39b7191f398c4395d8bb6b Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 20 Jul 2022 13:28:29 +0000 Subject: [PATCH 1/5] Create a more compact diff format --- Cargo.lock | 46 ++---------------- ui_test/Cargo.lock | 46 ++---------------- ui_test/Cargo.toml | 2 +- ui_test/src/diff.rs | 111 ++++++++++++++++++++++++++++++++++++++++++++ ui_test/src/lib.rs | 4 +- 5 files changed, 120 insertions(+), 89 deletions(-) create mode 100644 ui_test/src/diff.rs diff --git a/Cargo.lock b/Cargo.lock index f660ed7762..5117a733d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -26,15 +26,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "ansi_term" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" -dependencies = [ - "winapi", -] - [[package]] name = "atty" version = "0.2.14" @@ -223,21 +214,11 @@ dependencies = [ "lazy_static", ] -[[package]] -name = "ctor" -version = "0.1.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f877be4f7c9f246b183111634f75baa039715e3f46ce860677d3b19a69fb229c" -dependencies = [ - "quote", - "syn", -] - [[package]] name = "diff" -version = "0.1.12" +version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e25ea47919b1560c4e3b7fe0aaab9becf5b84a10325ddf7db0f0ba5e1026499" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" [[package]] name = "env_logger" @@ -426,15 +407,6 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18a6dbe30758c9f83eb00cbea4ac95966305f5a7772f3f42ebfc7fc7eddbd8e1" -[[package]] -name = "output_vt100" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "628223faebab4e3e40667ee0b2336d34a5b960ff60ea743ddfdbcf7770bcfb66" -dependencies = [ - "winapi", -] - [[package]] name = "owo-colors" version = "3.4.0" @@ -487,18 +459,6 @@ version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed0cfbc8191465bed66e1718596ee0b0b35d5ee1f41c5df2189d0fe8bde535ba" -[[package]] -name = "pretty_assertions" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c89f989ac94207d048d92db058e4f6ec7342b0971fc58d1271ca148b799b3563" -dependencies = [ - "ansi_term", - "ctor", - "diff", - "output_vt100", -] - [[package]] name = "proc-macro2" version = "1.0.39" @@ -762,8 +722,8 @@ dependencies = [ "color-eyre", "colored", "crossbeam", + "diff", "lazy_static", - "pretty_assertions", "regex", "rustc_version", "serde", diff --git a/ui_test/Cargo.lock b/ui_test/Cargo.lock index 9addea9b19..52d45c24bc 100644 --- a/ui_test/Cargo.lock +++ b/ui_test/Cargo.lock @@ -26,15 +26,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "ansi_term" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" -dependencies = [ - "winapi", -] - [[package]] name = "atty" version = "0.2.14" @@ -217,21 +208,11 @@ dependencies = [ "lazy_static", ] -[[package]] -name = "ctor" -version = "0.1.22" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f877be4f7c9f246b183111634f75baa039715e3f46ce860677d3b19a69fb229c" -dependencies = [ - "quote", - "syn", -] - [[package]] name = "diff" -version = "0.1.12" +version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0e25ea47919b1560c4e3b7fe0aaab9becf5b84a10325ddf7db0f0ba5e1026499" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" [[package]] name = "eyre" @@ -321,15 +302,6 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "18a6dbe30758c9f83eb00cbea4ac95966305f5a7772f3f42ebfc7fc7eddbd8e1" -[[package]] -name = "output_vt100" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "628223faebab4e3e40667ee0b2336d34a5b960ff60ea743ddfdbcf7770bcfb66" -dependencies = [ - "winapi", -] - [[package]] name = "owo-colors" version = "3.4.0" @@ -342,18 +314,6 @@ version = "0.2.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e0a7ae3ac2f1173085d398531c705756c94a4c56843785df85a60c1a0afac116" -[[package]] -name = "pretty_assertions" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c89f989ac94207d048d92db058e4f6ec7342b0971fc58d1271ca148b799b3563" -dependencies = [ - "ansi_term", - "ctor", - "diff", - "output_vt100", -] - [[package]] name = "proc-macro2" version = "1.0.39" @@ -535,8 +495,8 @@ dependencies = [ "color-eyre", "colored", "crossbeam", + "diff", "lazy_static", - "pretty_assertions", "regex", "rustc_version", "serde", diff --git a/ui_test/Cargo.toml b/ui_test/Cargo.toml index bb14eb7ecf..3513ae4286 100644 --- a/ui_test/Cargo.toml +++ b/ui_test/Cargo.toml @@ -12,7 +12,7 @@ rustc_version = "0.4" colored = "2" # Features chosen to match those required by env_logger, to avoid rebuilds regex = { version = "1.5.5", default-features = false, features = ["perf", "std"] } -pretty_assertions = "1.2.1" +diff = "0.1.13" crossbeam = "0.8.1" lazy_static = "1.4.0" serde = { version = "1.0", features = ["derive"] } diff --git a/ui_test/src/diff.rs b/ui_test/src/diff.rs new file mode 100644 index 0000000000..3efb2882e9 --- /dev/null +++ b/ui_test/src/diff.rs @@ -0,0 +1,111 @@ +use colored::*; +use diff::{chars, lines, Result, Result::*}; + +#[derive(Default)] +struct DiffState<'a> { + skipped_lines: usize, + /// When we see a removed line, we don't print it, we + /// keep it around to compare it with the next added line. + prev_left: Option<&'a str>, +} + +impl<'a> DiffState<'a> { + fn print_skip(&mut self) { + if self.skipped_lines != 0 { + eprintln!("... {} lines skipped", self.skipped_lines); + } + self.skipped_lines = 0; + } + + fn print_prev(&mut self) { + if let Some(l) = self.prev_left.take() { + self.print_left(l); + } + } + + fn print_left(&self, l: &str) { + eprintln!("{}{}", "-".red(), l.red()); + } + + fn print_right(&self, r: &str) { + eprintln!("{}{}", "+".green(), r.green()); + } + + fn row(&mut self, row: Result<&'a str>) { + match row { + Left(l) => { + self.print_skip(); + self.print_prev(); + self.prev_left = Some(l); + } + Both(..) => { + self.print_prev(); + self.skipped_lines += 1 + } + Right(r) => { + if let Some(l) = self.prev_left.take() { + // If the lines only add chars or only remove chars, display an inline diff + let diff = chars(l, r); + let mut seen_l = false; + let mut seen_r = false; + for char in &diff { + match char { + Left(l) if !l.is_whitespace() => seen_l = true, + Right(r) if !r.is_whitespace() => seen_r = true, + _ => {} + } + } + if seen_l && seen_r { + // the line both adds and removes chars, print both lines, but highlight their differences + eprint!("{}", "-".red()); + for char in &diff { + match char { + Left(l) => eprint!("{}", l.to_string().red()), + Right(_) => {} + Both(l, _) => eprint!("{}", l), + } + } + eprintln!(); + eprint!("{}", "+".green()); + for char in &diff { + match char { + Left(_) => {} + Right(r) => eprint!("{}", r.to_string().green()), + Both(l, _) => eprint!("{}", l), + } + } + eprintln!(); + } else { + eprint!("{}", "~".yellow()); + for char in diff { + match char { + Left(l) => eprint!("{}", l.to_string().red()), + Both(l, _) => eprint!("{}", l), + Right(r) => eprint!("{}", r.to_string().green()), + } + } + eprintln!(); + } + } else { + self.print_skip(); + self.print_right(r); + } + } + } + } + + fn finish(self) { + if self.skipped_lines != 0 { + eprintln!("... {} lines skipped ...", self.skipped_lines); + } + eprintln!() + } +} + +pub fn print_diff(expected: &str, actual: &str) { + let mut state = DiffState::default(); + for row in lines(expected, actual) { + state.row(row); + } + state.finish(); +} diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 06a84cfbf3..6646464d9d 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -19,6 +19,7 @@ use crate::dependencies::build_dependencies; use crate::parser::{Comments, Condition}; mod dependencies; +mod diff; mod parser; mod rustc_stderr; #[cfg(test)] @@ -261,8 +262,7 @@ pub fn run_tests(mut config: Config) -> Result<()> { eprintln!("{}", "error pattern found in success test".red()), Error::OutputDiffers { path, actual, expected } => { eprintln!("actual output differed from expected {}", path.display()); - eprintln!("{}", pretty_assertions::StrComparison::new(expected, actual)); - eprintln!() + diff::print_diff(expected, actual); } Error::ErrorsWithoutPattern { path: None, msgs } => { eprintln!( From c1ce7eb6862e12383fe427a17e23c507045b527c Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 20 Jul 2022 13:33:33 +0000 Subject: [PATCH 2/5] Avoid printing `1 lines skipped` --- ui_test/src/diff.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ui_test/src/diff.rs b/ui_test/src/diff.rs index 3efb2882e9..987fa7251e 100644 --- a/ui_test/src/diff.rs +++ b/ui_test/src/diff.rs @@ -4,6 +4,10 @@ use diff::{chars, lines, Result, Result::*}; #[derive(Default)] struct DiffState<'a> { skipped_lines: usize, + /// When we skip a line, remember it, in case + /// we end up only skipping one line. In that case we just + /// print the line instead of `... skipped one line ...` + last_skipped_line: Option<&'a str>, /// When we see a removed line, we don't print it, we /// keep it around to compare it with the next added line. prev_left: Option<&'a str>, @@ -11,8 +15,10 @@ struct DiffState<'a> { impl<'a> DiffState<'a> { fn print_skip(&mut self) { - if self.skipped_lines != 0 { - eprintln!("... {} lines skipped", self.skipped_lines); + match self.skipped_lines { + 0 => {} + 1 => eprintln!(" {}", self.last_skipped_line.unwrap()), + _ => eprintln!("... {} lines skipped", self.skipped_lines), } self.skipped_lines = 0; } @@ -38,8 +44,9 @@ impl<'a> DiffState<'a> { self.print_prev(); self.prev_left = Some(l); } - Both(..) => { + Both(l, _) => { self.print_prev(); + self.last_skipped_line = Some(l); self.skipped_lines += 1 } Right(r) => { From 511f6ba5ca2d14bbdcc7e1f4769e7c5a431e4992 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 21 Jul 2022 08:04:31 +0000 Subject: [PATCH 3/5] Deduplicate skipped lines printing --- ui_test/src/diff.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ui_test/src/diff.rs b/ui_test/src/diff.rs index 987fa7251e..297799a5e7 100644 --- a/ui_test/src/diff.rs +++ b/ui_test/src/diff.rs @@ -18,7 +18,7 @@ impl<'a> DiffState<'a> { match self.skipped_lines { 0 => {} 1 => eprintln!(" {}", self.last_skipped_line.unwrap()), - _ => eprintln!("... {} lines skipped", self.skipped_lines), + _ => eprintln!("... {} lines skipped ...", self.skipped_lines), } self.skipped_lines = 0; } @@ -101,10 +101,8 @@ impl<'a> DiffState<'a> { } } - fn finish(self) { - if self.skipped_lines != 0 { - eprintln!("... {} lines skipped ...", self.skipped_lines); - } + fn finish(mut self) { + self.print_skip(); eprintln!() } } From 62e6df04c7cbcbb9b5a62f31c2236c944a30a718 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 25 Jul 2022 14:43:44 +0000 Subject: [PATCH 4/5] Show some context around a diff --- ui_test/src/diff.rs | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/ui_test/src/diff.rs b/ui_test/src/diff.rs index 297799a5e7..b7d1c7bae5 100644 --- a/ui_test/src/diff.rs +++ b/ui_test/src/diff.rs @@ -3,24 +3,43 @@ use diff::{chars, lines, Result, Result::*}; #[derive(Default)] struct DiffState<'a> { - skipped_lines: usize, - /// When we skip a line, remember it, in case - /// we end up only skipping one line. In that case we just - /// print the line instead of `... skipped one line ...` - last_skipped_line: Option<&'a str>, + /// When we skip lines, remember the last `CONTEXT` ones to + /// display after the "skipped N lines" message + skipped_lines: Vec<&'a str>, /// When we see a removed line, we don't print it, we /// keep it around to compare it with the next added line. prev_left: Option<&'a str>, } +/// How many lines of context are displayed around the actual diffs +const CONTEXT: usize = 2; + impl<'a> DiffState<'a> { fn print_skip(&mut self) { - match self.skipped_lines { - 0 => {} - 1 => eprintln!(" {}", self.last_skipped_line.unwrap()), - _ => eprintln!("... {} lines skipped ...", self.skipped_lines), + let half = self.skipped_lines.len() / 2; + if half < CONTEXT { + for line in self.skipped_lines.drain(..) { + eprintln!(" {line}"); + } + } else { + for line in self.skipped_lines.iter().take(CONTEXT) { + eprintln!(" {line}"); + } + let skipped = self.skipped_lines.len() - CONTEXT * 2; + match skipped { + 0 => {} + 1 => eprintln!(" {}", self.skipped_lines[CONTEXT]), + _ => eprintln!("... {skipped} lines skipped ..."), + } + for line in self.skipped_lines.iter().rev().take(CONTEXT).rev() { + eprintln!(" {line}"); + } } - self.skipped_lines = 0; + self.skipped_lines.clear(); + } + + fn skip(&mut self, line: &'a str) { + self.skipped_lines.push(line); } fn print_prev(&mut self) { @@ -46,8 +65,7 @@ impl<'a> DiffState<'a> { } Both(l, _) => { self.print_prev(); - self.last_skipped_line = Some(l); - self.skipped_lines += 1 + self.skip(l); } Right(r) => { if let Some(l) = self.prev_left.take() { From 9a231652efad3761473134960b0e4dee75b960f1 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 27 Jul 2022 14:24:09 +0000 Subject: [PATCH 5/5] Document all the things --- ui_test/src/diff.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ui_test/src/diff.rs b/ui_test/src/diff.rs index b7d1c7bae5..d4271184ef 100644 --- a/ui_test/src/diff.rs +++ b/ui_test/src/diff.rs @@ -18,19 +18,25 @@ impl<'a> DiffState<'a> { fn print_skip(&mut self) { let half = self.skipped_lines.len() / 2; if half < CONTEXT { + // Print all the skipped lines if the amount of context desired is less than the amount of lines for line in self.skipped_lines.drain(..) { eprintln!(" {line}"); } } else { + // Print an initial `CONTEXT` amount of lines. for line in self.skipped_lines.iter().take(CONTEXT) { eprintln!(" {line}"); } let skipped = self.skipped_lines.len() - CONTEXT * 2; match skipped { + // When the amount of skipped lines is exactly `CONTEXT * 2`, we already + // print all the context and don't actually skip anything. 0 => {} + // Instead of writing a line saying we skipped one line, print that one line 1 => eprintln!(" {}", self.skipped_lines[CONTEXT]), _ => eprintln!("... {skipped} lines skipped ..."), } + // Print the last `CONTEXT` amount of lines. for line in self.skipped_lines.iter().rev().take(CONTEXT).rev() { eprintln!(" {line}"); } @@ -68,8 +74,9 @@ impl<'a> DiffState<'a> { self.skip(l); } Right(r) => { + // When there's an added line after a removed line, we'll want to special case some print cases. + // FIXME(oli-obk): also do special printing modes when there are multiple lines that only have minor changes. if let Some(l) = self.prev_left.take() { - // If the lines only add chars or only remove chars, display an inline diff let diff = chars(l, r); let mut seen_l = false; let mut seen_r = false; @@ -81,7 +88,8 @@ impl<'a> DiffState<'a> { } } if seen_l && seen_r { - // the line both adds and removes chars, print both lines, but highlight their differences + // The line both adds and removes chars, print both lines, but highlight their differences instead of + // drawing the entire line in red/green. eprint!("{}", "-".red()); for char in &diff { match char { @@ -101,6 +109,7 @@ impl<'a> DiffState<'a> { } eprintln!(); } else { + // The line only adds or only removes chars, print a single line highlighting their differences. eprint!("{}", "~".yellow()); for char in diff { match char {