From 977b688144fb997fdf5dd5bdb587e4f357d853f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 1 Jul 2020 10:37:55 +0200 Subject: [PATCH 1/4] Don't fail tests when updating snapshot --- crates/expect/src/lib.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/expect/src/lib.rs b/crates/expect/src/lib.rs index dd7b96aab2..d678b18178 100644 --- a/crates/expect/src/lib.rs +++ b/crates/expect/src/lib.rs @@ -2,7 +2,7 @@ //! https://github.com/rust-analyzer/rust-analyzer/pull/5101 use std::{ collections::HashMap, - env, fmt, fs, + env, fmt, fs, mem, ops::Range, panic, path::{Path, PathBuf}, @@ -97,24 +97,25 @@ static RT: Lazy> = Lazy::new(Default::default); impl Runtime { fn fail(expect: &Expect, expected: &str, actual: &str) { let mut rt = RT.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); - let mut updated = ""; if update_expect() { - updated = " (updated)"; + println!( + "\x1b[1m\x1b[92mupdating\x1b[0m: {}:{}:{}", + expect.file, expect.line, expect.column + ); rt.per_file .entry(expect.file) .or_insert_with(|| FileRuntime::new(expect)) .update(expect, actual); + return; } - let print_help = !rt.help_printed && !update_expect(); - rt.help_printed = true; - + let print_help = !mem::replace(&mut rt.help_printed, true); let help = if print_help { HELP } else { "" }; let diff = Changeset::new(actual, expected, "\n"); println!( "\n -\x1b[1m\x1b[91merror\x1b[97m: expect test failed\x1b[0m{} +\x1b[1m\x1b[91merror\x1b[97m: expect test failed\x1b[0m \x1b[1m\x1b[34m-->\x1b[0m {}:{}:{} {} \x1b[1mExpect\x1b[0m: @@ -132,7 +133,7 @@ impl Runtime { {} ---- ", - updated, expect.file, expect.line, expect.column, help, expected, actual, diff + expect.file, expect.line, expect.column, help, expected, actual, diff ); // Use resume_unwind instead of panic!() to prevent a backtrace, which is unnecessary noise. panic::resume_unwind(Box::new(())); From 82838f5eda6ee98cebb9574ceef36544f1a45a4d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 1 Jul 2020 10:43:11 +0200 Subject: [PATCH 2/4] Cleanup --- crates/expect/src/lib.rs | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/expect/src/lib.rs b/crates/expect/src/lib.rs index d678b18178..3a92b36e2f 100644 --- a/crates/expect/src/lib.rs +++ b/crates/expect/src/lib.rs @@ -29,9 +29,11 @@ fn update_expect() -> bool { #[macro_export] macro_rules! expect { [[$lit:literal]] => {$crate::Expect { - file: file!(), - line: line!(), - column: column!(), + position: $crate::Position { + file: file!(), + line: line!(), + column: column!(), + }, data: $lit, }}; [[]] => { $crate::expect![[""]] }; @@ -39,10 +41,21 @@ macro_rules! expect { #[derive(Debug)] pub struct Expect { + pub position: Position, + pub data: &'static str, +} + +#[derive(Debug)] +pub struct Position { pub file: &'static str, pub line: u32, pub column: u32, - pub data: &'static str, +} + +impl fmt::Display for Position { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}:{}:{}", self.file, self.line, self.column) + } } impl Expect { @@ -69,7 +82,7 @@ impl Expect { let mut target_line = None; let mut line_start = 0; for (i, line) in lines_with_ends(file).enumerate() { - if i == self.line as usize - 1 { + if i == self.position.line as usize - 1 { let pat = "expect![["; let offset = line.find(pat).unwrap(); let literal_start = line_start + offset + pat.len(); @@ -98,12 +111,9 @@ impl Runtime { fn fail(expect: &Expect, expected: &str, actual: &str) { let mut rt = RT.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); if update_expect() { - println!( - "\x1b[1m\x1b[92mupdating\x1b[0m: {}:{}:{}", - expect.file, expect.line, expect.column - ); + println!("\x1b[1m\x1b[92mupdating\x1b[0m: {}", expect.position); rt.per_file - .entry(expect.file) + .entry(expect.position.file) .or_insert_with(|| FileRuntime::new(expect)) .update(expect, actual); return; @@ -116,7 +126,7 @@ impl Runtime { println!( "\n \x1b[1m\x1b[91merror\x1b[97m: expect test failed\x1b[0m - \x1b[1m\x1b[34m-->\x1b[0m {}:{}:{} + \x1b[1m\x1b[34m-->\x1b[0m {} {} \x1b[1mExpect\x1b[0m: ---- @@ -133,7 +143,7 @@ impl Runtime { {} ---- ", - expect.file, expect.line, expect.column, help, expected, actual, diff + expect.position, help, expected, actual, diff ); // Use resume_unwind instead of panic!() to prevent a backtrace, which is unnecessary noise. panic::resume_unwind(Box::new(())); @@ -148,7 +158,7 @@ struct FileRuntime { impl FileRuntime { fn new(expect: &Expect) -> FileRuntime { - let path = workspace_root().join(expect.file); + let path = workspace_root().join(expect.position.file); let original_text = fs::read_to_string(&path).unwrap(); let patchwork = Patchwork::new(original_text.clone()); FileRuntime { path, original_text, patchwork } From adf624b433277a0106f5354bb7d62ab1a04f216b Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 1 Jul 2020 11:19:40 +0200 Subject: [PATCH 3/4] Add file support to expect --- crates/expect/src/lib.rs | 60 ++++++++++++++++--- .../ra_ide/src/syntax_highlighting/tests.rs | 22 ++++--- crates/rust-analyzer/src/handlers.rs | 2 +- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/crates/expect/src/lib.rs b/crates/expect/src/lib.rs index 3a92b36e2f..3f293f5d57 100644 --- a/crates/expect/src/lib.rs +++ b/crates/expect/src/lib.rs @@ -14,7 +14,7 @@ use once_cell::sync::Lazy; use stdx::{lines_with_ends, trim_indent}; const HELP: &str = " -You can update all `expect![[]]` tests by: +You can update all `expect![[]]` tests by running: env UPDATE_EXPECT=1 cargo test @@ -25,26 +25,37 @@ fn update_expect() -> bool { env::var("UPDATE_EXPECT").is_ok() } -/// expect![[""]] +/// expect![[r#"inline snapshot"#]] #[macro_export] macro_rules! expect { - [[$lit:literal]] => {$crate::Expect { + [[$data:literal]] => {$crate::Expect { position: $crate::Position { file: file!(), line: line!(), column: column!(), }, - data: $lit, + data: $data, }}; [[]] => { $crate::expect![[""]] }; } +/// expect_file!["/crates/foo/test_data/foo.rs"] +#[macro_export] +macro_rules! expect_file { + [$path:literal] => {$crate::ExpectFile { path: $path }}; +} + #[derive(Debug)] pub struct Expect { pub position: Position, pub data: &'static str, } +#[derive(Debug)] +pub struct ExpectFile { + pub path: &'static str, +} + #[derive(Debug)] pub struct Position { pub file: &'static str, @@ -64,7 +75,7 @@ impl Expect { if &trimmed == actual { return; } - Runtime::fail(self, &trimmed, actual); + Runtime::fail_expect(self, &trimmed, actual); } pub fn assert_debug_eq(&self, actual: &impl fmt::Debug) { let actual = format!("{:#?}\n", actual); @@ -100,6 +111,25 @@ impl Expect { } } +impl ExpectFile { + pub fn assert_eq(&self, actual: &str) { + let expected = self.read(); + if actual == expected { + return; + } + Runtime::fail_file(self, &expected, actual); + } + fn read(&self) -> String { + fs::read_to_string(self.abs_path()).unwrap_or_default().replace("\r\n", "\n") + } + fn write(&self, contents: &str) { + fs::write(self.abs_path(), contents).unwrap() + } + fn abs_path(&self) -> PathBuf { + workspace_root().join(self.path) + } +} + #[derive(Default)] struct Runtime { help_printed: bool, @@ -108,7 +138,7 @@ struct Runtime { static RT: Lazy> = Lazy::new(Default::default); impl Runtime { - fn fail(expect: &Expect, expected: &str, actual: &str) { + fn fail_expect(expect: &Expect, expected: &str, actual: &str) { let mut rt = RT.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); if update_expect() { println!("\x1b[1m\x1b[92mupdating\x1b[0m: {}", expect.position); @@ -118,7 +148,21 @@ impl Runtime { .update(expect, actual); return; } - let print_help = !mem::replace(&mut rt.help_printed, true); + rt.panic(expect.position.to_string(), expected, actual); + } + + fn fail_file(expect: &ExpectFile, expected: &str, actual: &str) { + let mut rt = RT.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + if update_expect() { + println!("\x1b[1m\x1b[92mupdating\x1b[0m: {}", expect.path); + expect.write(actual); + return; + } + rt.panic(expect.path.to_string(), expected, actual); + } + + fn panic(&mut self, position: String, expected: &str, actual: &str) { + let print_help = !mem::replace(&mut self.help_printed, true); let help = if print_help { HELP } else { "" }; let diff = Changeset::new(actual, expected, "\n"); @@ -143,7 +187,7 @@ impl Runtime { {} ---- ", - expect.position, help, expected, actual, diff + position, help, expected, actual, diff ); // Use resume_unwind instead of panic!() to prevent a backtrace, which is unnecessary noise. panic::resume_unwind(Box::new(())); diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index b7fad97197..f19628485d 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -1,6 +1,7 @@ use std::fs; -use test_utils::{assert_eq_text, project_dir, read_text}; +use expect::{expect_file, ExpectFile}; +use test_utils::project_dir; use crate::{mock_analysis::single_file, FileRange, TextRange}; @@ -91,7 +92,7 @@ impl Option { } "# .trim(), - "crates/ra_ide/src/snapshots/highlighting.html", + expect_file!["crates/ra_ide/src/snapshots/highlighting.html"], false, ); } @@ -114,7 +115,7 @@ fn bar() { } "# .trim(), - "crates/ra_ide/src/snapshots/rainbow_highlighting.html", + expect_file!["crates/ra_ide/src/snapshots/rainbow_highlighting.html"], true, ); } @@ -167,7 +168,7 @@ fn main() { ); }"## .trim(), - "crates/ra_ide/src/snapshots/highlight_injection.html", + expect_file!["crates/ra_ide/src/snapshots/highlight_injection.html"], false, ); } @@ -250,7 +251,7 @@ fn main() { println!("{ничоси}", ничоси = 92); }"# .trim(), - "crates/ra_ide/src/snapshots/highlight_strings.html", + expect_file!["crates/ra_ide/src/snapshots/highlight_strings.html"], false, ); } @@ -278,7 +279,7 @@ fn main() { } "# .trim(), - "crates/ra_ide/src/snapshots/highlight_unsafe.html", + expect_file!["crates/ra_ide/src/snapshots/highlight_unsafe.html"], false, ); } @@ -354,7 +355,7 @@ macro_rules! noop { } "# .trim(), - "crates/ra_ide/src/snapshots/highlight_doctest.html", + expect_file!["crates/ra_ide/src/snapshots/highlight_doctest.html"], false, ); } @@ -362,11 +363,8 @@ macro_rules! noop { /// Highlights the code given by the `ra_fixture` argument, renders the /// result as HTML, and compares it with the HTML file given as `snapshot`. /// Note that the `snapshot` file is overwritten by the rendered HTML. -fn check_highlighting(ra_fixture: &str, snapshot: &str, rainbow: bool) { +fn check_highlighting(ra_fixture: &str, expect: ExpectFile, rainbow: bool) { let (analysis, file_id) = single_file(ra_fixture); - let dst_file = project_dir().join(snapshot); let actual_html = &analysis.highlight_as_html(file_id, rainbow).unwrap(); - let expected_html = &read_text(&dst_file); - fs::write(dst_file, &actual_html).unwrap(); - assert_eq_text!(expected_html, actual_html); + expect.assert_eq(actual_html) } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 25bcd80af8..607a95682a 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -415,7 +415,7 @@ pub(crate) fn handle_runnables( let source_file = snap.analysis.parse(file_id)?; algo::find_node_at_offset::(source_file.syntax(), offset) .and_then(|it| it.path()?.segment()?.name_ref()) - .map_or(false, |it| it.text() == "expect") + .map_or(false, |it| it.text() == "expect" || it.text() == "expect_file") } None => false, }; From 05d67a9a0efafb3dd5087aad17d75aa88aa85178 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 1 Jul 2020 11:25:22 +0200 Subject: [PATCH 4/4] Move test data to test_data directory --- crates/expect/src/lib.rs | 2 +- crates/ra_ide/src/syntax_highlighting/tests.rs | 12 ++++++------ .../snapshots => test_data}/highlight_doctest.html | 0 .../snapshots => test_data}/highlight_injection.html | 0 .../snapshots => test_data}/highlight_strings.html | 0 .../snapshots => test_data}/highlight_unsafe.html | 0 .../{src/snapshots => test_data}/highlighting.html | 0 .../rainbow_highlighting.html | 0 8 files changed, 7 insertions(+), 7 deletions(-) rename crates/ra_ide/{src/snapshots => test_data}/highlight_doctest.html (100%) rename crates/ra_ide/{src/snapshots => test_data}/highlight_injection.html (100%) rename crates/ra_ide/{src/snapshots => test_data}/highlight_strings.html (100%) rename crates/ra_ide/{src/snapshots => test_data}/highlight_unsafe.html (100%) rename crates/ra_ide/{src/snapshots => test_data}/highlighting.html (100%) rename crates/ra_ide/{src/snapshots => test_data}/rainbow_highlighting.html (100%) diff --git a/crates/expect/src/lib.rs b/crates/expect/src/lib.rs index 3f293f5d57..a5e26faded 100644 --- a/crates/expect/src/lib.rs +++ b/crates/expect/src/lib.rs @@ -39,7 +39,7 @@ macro_rules! expect { [[]] => { $crate::expect![[""]] }; } -/// expect_file!["/crates/foo/test_data/foo.rs"] +/// expect_file!["/crates/foo/test_data/bar.html"] #[macro_export] macro_rules! expect_file { [$path:literal] => {$crate::ExpectFile { path: $path }}; diff --git a/crates/ra_ide/src/syntax_highlighting/tests.rs b/crates/ra_ide/src/syntax_highlighting/tests.rs index f19628485d..aa7c887d67 100644 --- a/crates/ra_ide/src/syntax_highlighting/tests.rs +++ b/crates/ra_ide/src/syntax_highlighting/tests.rs @@ -92,7 +92,7 @@ impl Option { } "# .trim(), - expect_file!["crates/ra_ide/src/snapshots/highlighting.html"], + expect_file!["crates/ra_ide/test_data/highlighting.html"], false, ); } @@ -115,7 +115,7 @@ fn bar() { } "# .trim(), - expect_file!["crates/ra_ide/src/snapshots/rainbow_highlighting.html"], + expect_file!["crates/ra_ide/test_data/rainbow_highlighting.html"], true, ); } @@ -168,7 +168,7 @@ fn main() { ); }"## .trim(), - expect_file!["crates/ra_ide/src/snapshots/highlight_injection.html"], + expect_file!["crates/ra_ide/test_data/highlight_injection.html"], false, ); } @@ -251,7 +251,7 @@ fn main() { println!("{ничоси}", ничоси = 92); }"# .trim(), - expect_file!["crates/ra_ide/src/snapshots/highlight_strings.html"], + expect_file!["crates/ra_ide/test_data/highlight_strings.html"], false, ); } @@ -279,7 +279,7 @@ fn main() { } "# .trim(), - expect_file!["crates/ra_ide/src/snapshots/highlight_unsafe.html"], + expect_file!["crates/ra_ide/test_data/highlight_unsafe.html"], false, ); } @@ -355,7 +355,7 @@ macro_rules! noop { } "# .trim(), - expect_file!["crates/ra_ide/src/snapshots/highlight_doctest.html"], + expect_file!["crates/ra_ide/test_data/highlight_doctest.html"], false, ); } diff --git a/crates/ra_ide/src/snapshots/highlight_doctest.html b/crates/ra_ide/test_data/highlight_doctest.html similarity index 100% rename from crates/ra_ide/src/snapshots/highlight_doctest.html rename to crates/ra_ide/test_data/highlight_doctest.html diff --git a/crates/ra_ide/src/snapshots/highlight_injection.html b/crates/ra_ide/test_data/highlight_injection.html similarity index 100% rename from crates/ra_ide/src/snapshots/highlight_injection.html rename to crates/ra_ide/test_data/highlight_injection.html diff --git a/crates/ra_ide/src/snapshots/highlight_strings.html b/crates/ra_ide/test_data/highlight_strings.html similarity index 100% rename from crates/ra_ide/src/snapshots/highlight_strings.html rename to crates/ra_ide/test_data/highlight_strings.html diff --git a/crates/ra_ide/src/snapshots/highlight_unsafe.html b/crates/ra_ide/test_data/highlight_unsafe.html similarity index 100% rename from crates/ra_ide/src/snapshots/highlight_unsafe.html rename to crates/ra_ide/test_data/highlight_unsafe.html diff --git a/crates/ra_ide/src/snapshots/highlighting.html b/crates/ra_ide/test_data/highlighting.html similarity index 100% rename from crates/ra_ide/src/snapshots/highlighting.html rename to crates/ra_ide/test_data/highlighting.html diff --git a/crates/ra_ide/src/snapshots/rainbow_highlighting.html b/crates/ra_ide/test_data/rainbow_highlighting.html similarity index 100% rename from crates/ra_ide/src/snapshots/rainbow_highlighting.html rename to crates/ra_ide/test_data/rainbow_highlighting.html