Merge #5159

5159: Don't fail expect tests in rewrite mode r=matklad a=matklad

bors r+

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

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},
@@ -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,24 +25,48 @@ 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 {
-        file: file!(),
+        position: $crate::Position {
-        line: line!(),
+            file: file!(),
-        column: column!(),
+            line: line!(),
-        data: $lit,
+            column: column!(),
+        },
+        data: $data,
     }};
     [[]] => { $crate::expect![[""]] };
 }
 
+/// expect_file!["/crates/foo/test_data/bar.html"]
+#[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,
     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 {
@@ -51,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);
@@ -69,7 +93,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();
@@ -87,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,
@@ -95,27 +138,39 @@ struct Runtime {
 static RT: Lazy<Mutex<Runtime>> = 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());
-        let mut updated = "";
         if update_expect() {
-            updated = " (updated)";
+            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;
         }
-        let print_help = !rt.help_printed && !update_expect();
+        rt.panic(expect.position.to_string(), expected, actual);
-        rt.help_printed = true;
+    }
 
+    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");
 
         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[1m\x1b[34m-->\x1b[0m {}
 {}
 \x1b[1mExpect\x1b[0m:
 ----
@@ -132,7 +187,7 @@ impl Runtime {
 {}
 ----
 ",
-            updated, expect.file, expect.line, expect.column, 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(()));
@@ -147,7 +202,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 }

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<T> Option<T> {
 }
 "#
         .trim(),
-        "crates/ra_ide/src/snapshots/highlighting.html",
+        expect_file!["crates/ra_ide/test_data/highlighting.html"],
         false,
     );
 }
@@ -114,7 +115,7 @@ fn bar() {
 }
 "#
         .trim(),
-        "crates/ra_ide/src/snapshots/rainbow_highlighting.html",
+        expect_file!["crates/ra_ide/test_data/rainbow_highlighting.html"],
         true,
     );
 }
@@ -167,7 +168,7 @@ fn main() {
     );
 }"##
         .trim(),
-        "crates/ra_ide/src/snapshots/highlight_injection.html",
+        expect_file!["crates/ra_ide/test_data/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/test_data/highlight_strings.html"],
         false,
     );
 }
@@ -278,7 +279,7 @@ fn main() {
 }
 "#
         .trim(),
-        "crates/ra_ide/src/snapshots/highlight_unsafe.html",
+        expect_file!["crates/ra_ide/test_data/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/test_data/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);
+    expect.assert_eq(actual_html)
-    fs::write(dst_file, &actual_html).unwrap();
-    assert_eq_text!(expected_html, actual_html);
 }

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::<ast::MacroCall>(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,
     };