about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAleksey Kladov <aleksey.kladov@gmail.com>2021-04-13 12:20:54 +0300
committerAleksey Kladov <aleksey.kladov@gmail.com>2021-04-13 12:21:59 +0300
commitdb2a989565ea2b3d3c06e34cd385cfb574a32fbb (patch)
tree27d88010e9441d2af973f04c9b979220b0b07ebf
parent06a633ff421b764428bb946ced914e59532fe13f (diff)
downloadrust-db2a989565ea2b3d3c06e34cd385cfb574a32fbb.tar.gz
rust-db2a989565ea2b3d3c06e34cd385cfb574a32fbb.zip
internal: don't use `#[should_panic]` for tests
-rw-r--r--crates/ide/src/goto_definition.rs18
-rw-r--r--crates/ide_assists/src/handlers/flip_comma.rs18
-rw-r--r--docs/dev/style.md10
3 files changed, 22 insertions, 24 deletions
diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs
index d057d5402d3..a04333e63e4 100644
--- a/crates/ide/src/goto_definition.rs
+++ b/crates/ide/src/goto_definition.rs
@@ -110,6 +110,13 @@ mod tests {
         assert_eq!(expected, FileRange { file_id: nav.file_id, range: nav.focus_or_full_range() });
     }
 
+    fn check_unresolved(ra_fixture: &str) {
+        let (analysis, position) = fixture::position(ra_fixture);
+        let navs = analysis.goto_definition(position).unwrap().expect("no definition found").info;
+
+        assert!(navs.is_empty(), "didn't expect this to resolve anywhere: {:?}", navs)
+    }
+
     #[test]
     fn goto_def_for_extern_crate() {
         check(
@@ -927,17 +934,12 @@ fn f() -> impl Iterator<Item$0 = u8> {}
     }
 
     #[test]
-    #[should_panic = "unresolved reference"]
     fn unknown_assoc_ty() {
-        check(
+        check_unresolved(
             r#"
-trait Iterator {
-    type Item;
-       //^^^^
-}
-
+trait Iterator { type Item; }
 fn f() -> impl Iterator<Invalid$0 = u8> {}
-            "#,
+"#,
         )
     }
 
diff --git a/crates/ide_assists/src/handlers/flip_comma.rs b/crates/ide_assists/src/handlers/flip_comma.rs
index 7f5e75d3450..99be5e090ce 100644
--- a/crates/ide_assists/src/handlers/flip_comma.rs
+++ b/crates/ide_assists/src/handlers/flip_comma.rs
@@ -66,26 +66,12 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
     fn flip_comma_before_punct() {
         // See https://github.com/rust-analyzer/rust-analyzer/issues/1619
         // "Flip comma" assist shouldn't be applicable to the last comma in enum or struct
         // declaration body.
-        check_assist_target(
-            flip_comma,
-            "pub enum Test { \
-             A,$0 \
-             }",
-            ",",
-        );
-
-        check_assist_target(
-            flip_comma,
-            "pub struct Test { \
-             foo: usize,$0 \
-             }",
-            ",",
-        );
+        check_assist_not_applicable(flip_comma, "pub enum Test { A,$0 }");
+        check_assist_not_applicable(flip_comma, "pub struct Test { foo: usize,$0 }");
     }
 
     #[test]
diff --git a/docs/dev/style.md b/docs/dev/style.md
index 468dedff20d..7c47c26b2d4 100644
--- a/docs/dev/style.md
+++ b/docs/dev/style.md
@@ -152,6 +152,16 @@ Do not reuse marks between several tests.
 
 **Rationale:** marks provide an easy way to find the canonical test for each bit of code.
 This makes it much easier to understand.
+More than one mark per test / code branch doesn't add significantly to understanding.
+
+## `#[should_panic]`
+
+Do not use `#[should_panic]` tests.
+Instead, explicitly check for `None`, `Err`, etc.
+
+**Rationale:**a `#[should_panic]` is a tool for library authors, to makes sure that API does not fail silently, when misused.
+`rust-analyzer` is not a library, we don't need to test for API misuse, and we have to handle any user input without panics.
+Panic messages in the logs from the `#[should_panic]` tests are confusing.
 
 ## Function Preconditions