]> git.lizzy.rs Git - rust.git/commitdiff
internal: document that we don't #[ignore] tests
authorAleksey Kladov <aleksey.kladov@gmail.com>
Mon, 14 Jun 2021 19:55:05 +0000 (22:55 +0300)
committerAleksey Kladov <aleksey.kladov@gmail.com>
Tue, 15 Jun 2021 08:46:47 +0000 (11:46 +0300)
crates/hir_def/src/nameres/collector.rs
crates/hir_ty/src/tests/coercion.rs
crates/hir_ty/src/tests/traits.rs
crates/ide/src/doc_links.rs
crates/ide/src/hover.rs
crates/ide_assists/src/handlers/fix_visibility.rs
crates/ide_diagnostics/src/handlers/incorrect_case.rs
docs/dev/style.md

index 6fab58f159387a4b31429886bf3dfbcb96d7bacd..4ae02e5769efd96b1102a85354f8aeaed7055b57 100644 (file)
@@ -1992,8 +1992,8 @@ fn do_collect_defs(db: &dyn DefDatabase, def_map: DefMap) -> DefMap {
         collector.def_map
     }
 
-    fn do_resolve(code: &str) -> DefMap {
-        let (db, _file_id) = TestDB::with_single_file(code);
+    fn do_resolve(not_ra_fixture: &str) -> DefMap {
+        let (db, _file_id) = TestDB::with_single_file(not_ra_fixture);
         let krate = db.test_crate();
 
         let edition = db.crate_graph()[krate].edition;
@@ -2013,16 +2013,21 @@ macro_rules! foo {
         );
     }
 
-    #[ignore] // this test does succeed, but takes quite a while :/
+    #[ignore]
     #[test]
     fn test_macro_expand_will_stop_2() {
+        // FIXME: this test does succeed, but takes quite a while: 90 seconds in
+        // the release mode. That's why the argument is not an ra_fixture --
+        // otherwise injection highlighting gets stuck.
+        //
+        // We need to find a way to fail this faster.
         do_resolve(
             r#"
-        macro_rules! foo {
-            ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
-        }
-        foo!(KABOOM);
-        "#,
+macro_rules! foo {
+    ($($ty:ty)*) => { foo!($($ty)* $($ty)*); }
+}
+foo!(KABOOM);
+"#,
         );
     }
 }
index 71047703d18ee1c50de74a7f03de5caf105fea47..4f859fc85223719ca5d1ec8f255738424b3d6094 100644 (file)
@@ -741,10 +741,24 @@ fn test() {
 }
 
 #[test]
-// The rust reference says this should be possible, but rustc doesn't implement
-// it. We used to support it, but Chalk doesn't.
-#[ignore]
 fn coerce_unsize_trait_object_to_trait_object() {
+    // FIXME: The rust reference says this should be possible, but rustc doesn't
+    // implement it. We used to support it, but Chalk doesn't. Here's the
+    // correct expect:
+    //
+    //     424..609 '{     ...bj2; }': ()
+    //     434..437 'obj': &dyn Baz<i8, i16>
+    //     459..461 '&S': &S<i8, i16>
+    //     460..461 'S': S<i8, i16>
+    //     471..474 'obj': &dyn Bar<usize, i8, i16>
+    //     496..499 'obj': &dyn Baz<i8, i16>
+    //     509..512 'obj': &dyn Foo<i8, usize>
+    //     531..534 'obj': &dyn Bar<usize, i8, i16>
+    //     544..548 'obj2': &dyn Baz<i8, i16>
+    //     570..572 '&S': &S<i8, i16>
+    //     571..572 'S': S<i8, i16>
+    //     582..583 '_': &dyn Foo<i8, usize>
+    //     602..606 'obj2': &dyn Baz<i8, i16>
     check_infer_with_mismatches(
         r#"
         #[lang = "sized"]
@@ -773,21 +787,24 @@ fn test() {
             let _: &dyn Foo<_, _> = obj2;
         }
         "#,
-        expect![[r"
+        expect![[r#"
             424..609 '{     ...bj2; }': ()
             434..437 'obj': &dyn Baz<i8, i16>
             459..461 '&S': &S<i8, i16>
             460..461 'S': S<i8, i16>
-            471..474 'obj': &dyn Bar<usize, i8, i16>
+            471..474 'obj': &dyn Bar<{unknown}, {unknown}, {unknown}>
             496..499 'obj': &dyn Baz<i8, i16>
-            509..512 'obj': &dyn Foo<i8, usize>
-            531..534 'obj': &dyn Bar<usize, i8, i16>
+            509..512 'obj': &dyn Foo<{unknown}, {unknown}>
+            531..534 'obj': &dyn Bar<{unknown}, {unknown}, {unknown}>
             544..548 'obj2': &dyn Baz<i8, i16>
             570..572 '&S': &S<i8, i16>
             571..572 'S': S<i8, i16>
-            582..583 '_': &dyn Foo<i8, usize>
+            582..583 '_': &dyn Foo<{unknown}, {unknown}>
             602..606 'obj2': &dyn Baz<i8, i16>
-        "]],
+            496..499: expected &dyn Bar<{unknown}, {unknown}, {unknown}>, got &dyn Baz<i8, i16>
+            531..534: expected &dyn Foo<{unknown}, {unknown}>, got &dyn Bar<{unknown}, {unknown}, {unknown}>
+            602..606: expected &dyn Foo<{unknown}, {unknown}>, got &dyn Baz<i8, i16>
+        "#]],
     );
 }
 
index 6bcede4c46a5e383983ce586ff75f28fe5d6823e..c830e576efeb8db1d86de51a4d166240427b9913 100644 (file)
@@ -1475,7 +1475,6 @@ fn test(
 }
 
 #[test]
-#[ignore]
 fn error_bound_chalk() {
     check_types(
         r#"
index 57ae9455b478f97456873009a02b06ad9c0eaf70..7ac0118fe205c2999a7ca13981f97586cae61f8e 100644 (file)
@@ -241,6 +241,10 @@ fn get_doc_link(db: &RootDatabase, definition: Definition) -> Option<String> {
         Definition::ModuleDef(ModuleDef::Module(module)) => module.krate(),
         _ => definition.module(db)?.krate(),
     };
+    // FIXME: using import map doesn't make sense here. What we want here is
+    // canonical path. What import map returns is the shortest path suitable for
+    // import. See this test:
+    cov_mark::hit!(test_reexport_order);
     let import_map = db.import_map(krate.into());
 
     let mut base = krate.display_name(db)?.to_string();
@@ -642,13 +646,15 @@ pub mod ba$0r {}
         )
     }
 
-    // FIXME: ImportMap will return re-export paths instead of public module
-    // paths. The correct path to documentation will never be a re-export.
-    // This problem stops us from resolving stdlib items included in the prelude
-    // such as `Option::Some` correctly.
-    #[ignore = "ImportMap may return re-exports"]
     #[test]
     fn test_reexport_order() {
+        cov_mark::check!(test_reexport_order);
+        // FIXME: This should return
+        //
+        //    https://docs.rs/test/*/test/wrapper/modulestruct.Item.html
+        //
+        // That is, we should point inside the module, rather than at the
+        // re-export.
         check(
             r#"
 pub mod wrapper {
@@ -663,7 +669,7 @@ fn foo() {
     let bar: wrapper::It$0em;
 }
         "#,
-            expect![[r#"https://docs.rs/test/*/test/wrapper/module/struct.Item.html"#]],
+            expect![[r#"https://docs.rs/test/*/test/wrapper/struct.Item.html"#]],
         )
     }
 }
index c08516805e151a525508f21797ecd6e410836784..afeded315734e35444c38a6a09fce2329060498f 100644 (file)
@@ -1821,9 +1821,10 @@ pub struct Bar
         );
     }
 
-    #[ignore = "path based links currently only support documentation on ModuleDef items"]
     #[test]
     fn test_hover_path_link_field() {
+        // FIXME: Should be
+        //  [Foo](https://docs.rs/test/*/test/struct.Foo.html)
         check(
             r#"
 pub struct Foo;
@@ -1845,7 +1846,7 @@ pub struct Bar {
 
                 ---
 
-                [Foo](https://docs.rs/test/*/test/struct.Foo.html)
+                [Foo](struct.Foo.html)
             "#]],
         );
     }
index 9b432e92ffc95bef98ab20775c349ac1838014c8..2f2b605fcbc44fad30c87d5de08792d7a01a4999 100644 (file)
@@ -583,25 +583,25 @@ pub struct Foo { pub(crate) bar: () }
     }
 
     #[test]
-    #[ignore]
-    // FIXME handle reexports properly
     fn fix_visibility_of_reexport() {
+        // FIXME: broken test, this should fix visibility of the re-export
+        // rather than the struct.
         check_assist(
             fix_visibility,
-            r"
-            mod foo {
-                use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz$0
-            ",
-            r"
-            mod foo {
-                $0pub(crate) use bar::Baz;
-                mod bar { pub(super) struct Baz; }
-            }
-            foo::Baz
-            ",
+            r#"
+mod foo {
+    use bar::Baz;
+    mod bar { pub(super) struct Baz; }
+}
+foo::Baz$0
+"#,
+            r#"
+mod foo {
+    use bar::Baz;
+    mod bar { $0pub(crate) struct Baz; }
+}
+foo::Baz
+"#,
         )
     }
 }
index 72f251961f6c82ad5181a9fba54a6bc8b0933997..68f25f284251d46f8e77ba7dafa9d2ebed788618 100644 (file)
@@ -341,43 +341,27 @@ fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {}
     }
 
     #[test]
-    #[ignore]
-    fn bug_trait_inside_fn() {
-        // FIXME:
-        // This is broken, and in fact, should not even be looked at by this
-        // lint in the first place. There's weird stuff going on in the
-        // collection phase.
-        // It's currently being brought in by:
-        // * validate_func on `a` recursing into modules
-        // * then it finds the trait and then the function while iterating
-        //   through modules
-        // * then validate_func is called on Dirty
-        // * ... which then proceeds to look at some unknown module taking no
-        //   attrs from either the impl or the fn a, and then finally to the root
-        //   module
-        //
-        // It should find the attribute on the trait, but it *doesn't even see
-        // the trait* as far as I can tell.
-
+    fn complex_ignore() {
+        // FIXME: this should trigger errors for the second case.
         check_diagnostics(
             r#"
 trait T { fn a(); }
 struct U {}
 impl T for U {
     fn a() {
-        // this comes out of bitflags, mostly
         #[allow(non_snake_case)]
-        trait __BitFlags {
+        trait __BitFlagsOk {
             const HiImAlsoBad: u8 = 2;
-            #[inline]
-            fn Dirty(&self) -> bool {
-                false
-            }
+            fn Dirty(&self) -> bool { false }
         }
 
+        trait __BitFlagsBad {
+            const HiImAlsoBad: u8 = 2;
+            fn Dirty(&self) -> bool { false }
+        }
     }
 }
-    "#,
+"#,
         );
     }
 
@@ -414,18 +398,14 @@ fn ignores_extern_items() {
     }
 
     #[test]
-    #[ignore]
     fn bug_traits_arent_checked() {
         // FIXME: Traits and functions in traits aren't currently checked by
         // r-a, even though rustc will complain about them.
         check_diagnostics(
             r#"
 trait BAD_TRAIT {
-    // ^^^^^^^^^ ðŸ’¡ weak: Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait`
     fn BAD_FUNCTION();
-    // ^^^^^^^^^^^^ ðŸ’¡ weak: Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function`
     fn BadFunction();
-    // ^^^^^^^^^^^^ ðŸ’¡ weak: Function `BadFunction` should have snake_case name, e.g. `bad_function`
 }
     "#,
         );
index 96dd684b32cca595784f5ec01ff667ab9ab92364..84485ea284503c3ba4771c5615f7f900db77ea8c 100644 (file)
@@ -174,6 +174,13 @@ Instead, explicitly check for `None`, `Err`, etc.
 `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.
 
+## `#[ignore]`
+
+Do not `#[ignore]` tests.
+If the test currently does not work, assert the wrong behavior and add a fixme explaining why it is wrong.
+
+**Rationale:** noticing when the behavior is fixed, making sure that even the wrong behavior is acceptable (ie, not a panic).
+
 ## Function Preconditions
 
 Express function preconditions in types and force the caller to provide them (rather than checking in callee):