From 4584868a7a23ea640694f34ed9e8c907473c66b4 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 15 Jun 2021 16:37:58 +0300 Subject: internal: don't #[ignore] tests See the style.md for motivation --- crates/ide/src/goto_definition.rs | 10 ++--- crates/ide_assists/src/handlers/fix_visibility.rs | 2 - .../ide_assists/src/handlers/generate_function.rs | 49 +++++++--------------- .../src/completions/attribute/derive.rs | 49 +++------------------- 4 files changed, 27 insertions(+), 83 deletions(-) diff --git a/crates/ide/src/goto_definition.rs b/crates/ide/src/goto_definition.rs index 8dd643a0f..d8e0dc4d5 100644 --- a/crates/ide/src/goto_definition.rs +++ b/crates/ide/src/goto_definition.rs @@ -1130,15 +1130,15 @@ fn foo<'foobar>(_: &'foobar ()) { } #[test] - #[ignore] // requires the HIR to somehow track these hrtb lifetimes fn goto_lifetime_hrtb() { - check( + // FIXME: requires the HIR to somehow track these hrtb lifetimes + check_unresolved( r#"trait Foo {} fn foo() where for<'a> T: Foo<&'a$0 (u8, u16)>, {} //^^ "#, ); - check( + check_unresolved( r#"trait Foo {} fn foo() where for<'a$0> T: Foo<&'a (u8, u16)>, {} //^^ @@ -1147,9 +1147,9 @@ fn foo() where for<'a$0> T: Foo<&'a (u8, u16)>, {} } #[test] - #[ignore] // requires ForTypes to be implemented fn goto_lifetime_hrtb_for_type() { - check( + // FIXME: requires ForTypes to be implemented + check_unresolved( r#"trait Foo {} fn foo() where T: for<'a> Foo<&'a$0 (u8, u16)>, {} //^^ diff --git a/crates/ide_assists/src/handlers/fix_visibility.rs b/crates/ide_assists/src/handlers/fix_visibility.rs index 2f2b605fc..f834bf16a 100644 --- a/crates/ide_assists/src/handlers/fix_visibility.rs +++ b/crates/ide_assists/src/handlers/fix_visibility.rs @@ -361,8 +361,6 @@ pub struct Foo { pub bar: () } } #[test] - #[ignore] - // FIXME reenable this test when `Semantics::resolve_record_field` works with union fields fn fix_visibility_of_union_field() { check_assist( fix_visibility, diff --git a/crates/ide_assists/src/handlers/generate_function.rs b/crates/ide_assists/src/handlers/generate_function.rs index 706c995ac..6a658d4cf 100644 --- a/crates/ide_assists/src/handlers/generate_function.rs +++ b/crates/ide_assists/src/handlers/generate_function.rs @@ -811,9 +811,8 @@ fn bar(baz: Baz::Bof) ${0:-> ()} { } #[test] - #[ignore] - // FIXME fix printing the generics of a `Ty` to make this test pass fn add_function_with_generic_arg() { + // FIXME: This is wrong, generated `bar` should include generic parameter. check_assist( generate_function, r" @@ -826,7 +825,7 @@ fn foo(t: T) { bar(t) } -fn bar(t: T) ${0:-> ()} { +fn bar(t: T) ${0:-> ()} { todo!() } ", @@ -834,9 +833,8 @@ fn bar(t: T) ${0:-> ()} { } #[test] - #[ignore] - // FIXME Fix function type printing to make this test pass fn add_function_with_fn_arg() { + // FIXME: The argument in `bar` is wrong. check_assist( generate_function, r" @@ -857,7 +855,7 @@ fn foo() { bar(Baz::new); } -fn bar(arg: fn() -> Baz) ${0:-> ()} { +fn bar(new: fn) ${0:-> ()} { todo!() } ", @@ -865,9 +863,8 @@ fn bar(arg: fn() -> Baz) ${0:-> ()} { } #[test] - #[ignore] - // FIXME Fix closure type printing to make this test pass fn add_function_with_closure_arg() { + // FIXME: The argument in `bar` is wrong. check_assist( generate_function, r" @@ -882,7 +879,7 @@ fn foo() { bar(closure) } -fn bar(closure: impl Fn(i64) -> i64) ${0:-> ()} { +fn bar(closure: ()) ${0:-> ()} { todo!() } ", @@ -986,13 +983,10 @@ fn foo() { } #[test] - #[ignore] - // Ignored until local imports are supported. - // See https://github.com/rust-analyzer/rust-analyzer/issues/1165 fn qualified_path_uses_correct_scope() { check_assist( generate_function, - " + r#" mod foo { pub struct Foo; } @@ -1001,8 +995,8 @@ fn bar() { let foo = Foo; baz$0(foo) } -", - " +"#, + r#" mod foo { pub struct Foo; } @@ -1015,7 +1009,7 @@ fn bar() { fn baz(foo: foo::Foo) ${0:-> ()} { todo!() } -", +"#, ) } @@ -1141,40 +1135,29 @@ fn bar() {} // The assist is only active if the cursor is on an unresolved path, // but the assist should only be offered if the path is a function call. generate_function, - r" + r#" fn foo() { bar(b$0az); } fn bar(baz: ()) {} -", +"#, ) } #[test] - #[ignore] fn create_method_with_no_args() { - check_assist( + // FIXME: This is wrong, this should just work. + check_assist_not_applicable( generate_function, - r" + r#" struct Foo; impl Foo { fn foo(&self) { self.bar()$0; } } - ", - r" -struct Foo; -impl Foo { - fn foo(&self) { - self.bar(); - } - fn bar(&self) { - todo!(); - } -} - ", + "#, ) } } diff --git a/crates/ide_completion/src/completions/attribute/derive.rs b/crates/ide_completion/src/completions/attribute/derive.rs index d526824fb..7b3133e53 100644 --- a/crates/ide_completion/src/completions/attribute/derive.rs +++ b/crates/ide_completion/src/completions/attribute/derive.rs @@ -93,57 +93,20 @@ mod tests { } #[test] - #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures fn empty_derive() { - check( - r#"#[derive($0)] struct Test;"#, - expect![[r#" - at Clone - at Clone, Copy - at Debug - at Default - at Hash - at PartialEq - at PartialEq, Eq - at PartialEq, PartialOrd - at PartialEq, Eq, PartialOrd, Ord - "#]], - ); + // FIXME: Add build-in derives to fixture. + check(r#"#[derive($0)] struct Test;"#, expect![[r#""#]]); } #[test] - #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures fn derive_with_input() { - check( - r#"#[derive(serde::Serialize, PartialEq, $0)] struct Test;"#, - expect![[r#" - at Clone - at Clone, Copy - at Debug - at Default - at Hash - at Eq - at PartialOrd - at Eq, PartialOrd, Ord - "#]], - ) + // FIXME: Add build-in derives to fixture. + check(r#"#[derive(serde::Serialize, PartialEq, $0)] struct Test;"#, expect![[r#""#]]) } #[test] - #[ignore] // FIXME: Fixtures cant test proc-macros/derives yet as we cant specify them in fixtures fn derive_with_input2() { - check( - r#"#[derive($0 serde::Serialize, PartialEq)] struct Test;"#, - expect![[r#" - at Clone - at Clone, Copy - at Debug - at Default - at Hash - at Eq - at PartialOrd - at Eq, PartialOrd, Ord - "#]], - ) + // FIXME: Add build-in derives to fixture. + check(r#"#[derive($0 serde::Serialize, PartialEq)] struct Test;"#, expect![[r#""#]]) } } -- cgit v1.2.3 From 067e97d149edc5eccdd0a30079f313325e87e449 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 15 Jun 2021 16:54:43 +0300 Subject: internal: enforce no #[ignore] and no #[should_panic] --- .../test_data/parser/ok/0011_outer_attribute.rast | 2 +- .../test_data/parser/ok/0011_outer_attribute.rs | 2 +- xtask/src/tidy.rs | 31 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast index ff5877a7b..31f76589d 100644 --- a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast +++ b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rast @@ -21,7 +21,7 @@ SOURCE_FILE@0..60 PATH@15..21 PATH_SEGMENT@15..21 NAME_REF@15..21 - IDENT@15..21 "ignore" + IDENT@15..21 "Ignore" R_BRACK@21..22 "]" WHITESPACE@22..23 "\n" FN_KW@23..25 "fn" diff --git a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs index 3d2e01d5c..6f04cb171 100644 --- a/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs +++ b/crates/syntax/test_data/parser/ok/0011_outer_attribute.rs @@ -1,5 +1,5 @@ #[cfg(test)] -#[ignore] +#[Ignore] fn foo() {} #[path = "a.rs"] diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs index f2ba8efef..25ddb43b2 100644 --- a/xtask/src/tidy.rs +++ b/xtask/src/tidy.rs @@ -89,6 +89,7 @@ fn rust_files_are_tidy() { let text = read_file(&path).unwrap(); check_todo(&path, &text); check_dbg(&path, &text); + check_test_attrs(&path, &text); check_trailing_ws(&path, &text); deny_clippy(&path, &text); tidy_docs.visit(&path, &text); @@ -334,6 +335,36 @@ fn check_dbg(path: &Path, text: &str) { } } +fn check_test_attrs(path: &Path, text: &str) { + let ignore_rule = + "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#ignore"; + let need_ignore: &[&str] = &[ + // Special case to run `#[ignore]` tests + "ide/src/runnables.rs", + // A legit test which needs to be ignored, as it takes too long to run + // :( + "hir_def/src/nameres/collector.rs", + // Obviously needs ignore. + "ide_assists/src/handlers/toggle_ignore.rs", + // See above. + "ide_assists/src/tests/generated.rs", + ]; + if text.contains("#[ignore") && !need_ignore.iter().any(|p| path.ends_with(p)) { + panic!("\ndon't `#[ignore]` tests, see:\n\n {}\n\n {}\n", ignore_rule, path.display(),) + } + + let panic_rule = + "https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#panic"; + let need_panic: &[&str] = &["test_utils/src/fixture.rs"]; + if text.contains("#[should_panic") && !need_panic.iter().any(|p| path.ends_with(p)) { + panic!( + "\ndon't add `#[should_panic]` tests, see:\n\n {}\n\n {}\n", + panic_rule, + path.display(), + ) + } +} + fn check_trailing_ws(path: &Path, text: &str) { if is_exclude_dir(path, &["test_data"]) { return; -- cgit v1.2.3