Skip to content

Commit

Permalink
test: added actual ui-toml tests
Browse files Browse the repository at this point in the history
  • Loading branch information
kayagokalp committed Oct 25, 2024
1 parent a4d8766 commit f6ce62a
Show file tree
Hide file tree
Showing 16 changed files with 271 additions and 32 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6216,7 +6216,9 @@ Released 2018-09-13
[`standard-macro-braces`]: https://doc.rust-lang.org/clippy/lint_configuration.html#standard-macro-braces
[`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
[`suppress-restriction-lint-in-const`]: https://doc.rust-lang.org/clippy/lint_configuration.html#suppress-restriction-lint-in-const
[`test-without-fail-case`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case
[`test-without-fail-case-fallible-paths`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-fallible-paths
[`test-without-fail-case-include-indexing-as-fallible`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-include-indexing-as-fallible
[`test-without-fail-case-non-fallible-paths`]: https://doc.rust-lang.org/clippy/lint_configuration.html#test-without-fail-case-non-fallible-paths
[`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack
[`too-many-arguments-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-arguments-threshold
[`too-many-lines-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-many-lines-threshold
Expand Down
46 changes: 43 additions & 3 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -831,10 +831,50 @@ if no suggestion can be made.
* [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing)


## `test-without-fail-case`
Lint tests to understand whether it can fail or not.
## `test-without-fail-case-fallible-paths`
List of full paths of macros and functions, that can fail. If a test, or a function
that the test calls contains a call to any one of these, lint will mark the test fallible.

**Default Value:** `{ include_indexing_as_fallible = false, fallible_paths = ["core::panic", "core::assert", "core::assert_eq", "core::assert_ne"], non_fallible_paths = ["std::print", "std::println", "std::dbg", "std::eprint", "std::eprintln"] }`
By default this macros are defined as `assert!`, `assert_eq!`, `panic!`.

**Default Value:** `["core::panic", "core::assert", "core::assert_eq", "core::assert_ne"]`

---
**Affected lints:**
* [`test_without_fail_case`](https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case)


## `test-without-fail-case-include-indexing-as-fallible`
Whether to consider indexing as a fallible operation while assesing if a test can fail.
Indexing is fallible, and thus the a test that is doing that can fail but it is likely
that tests that fail this way were not intended.

If set true, the lint will consider indexing into a slice a failable case
and won't lint tests that has some sort of indexing. This analysis still done
in a interprocedural manner. Meaning that any indexing opeartion done inside of
a function that the test calls will still result the test getting marked fallible.

By default this is set to `false`. That is because from a usability perspective,
indexing an array is not the intended way to fail a test. So setting this `true`
reduces false positives but makes the analysis more focused on possible byproducts
of a test. That is the set of operations to get the point we assert something rather
than the existance of asserting that thing.

**Default Value:** `false`

---
**Affected lints:**
* [`test_without_fail_case`](https://rust-lang.github.io/rust-clippy/master/index.html#test_without_fail_case)


## `test-without-fail-case-non-fallible-paths`
List of full paths of macros and functions, that we want to mark as "not going to fail".
This allows us to make the lint more focused on actual short comings of our test suite
by marking common routines non-fallible, even though they are fallible.

By default this list contains: `println!`, `print!`, `dbg!`.

**Default Value:** `["std::print", "std::println", "std::dbg", "std::eprint", "std::eprintln"]`

---
**Affected lints:**
Expand Down
18 changes: 10 additions & 8 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,14 +639,8 @@ define_Conf! {
///
/// By default this macros are defined as `assert!`, `assert_eq!`, `panic!`.
#[lints(test_without_fail_case)]
test_without_fail_case_fallible_paths: Vec<String> = Vec::new(),
/// List of full paths of macros and functions, that we want to mark as "not going to fail".
/// This allows us to make the lint more focused on actual short comings of our test suite
/// by marking common routines non-fallible, even though they are fallible.
///
/// By default this list contains: `println!`, `print!`, `dbg!`.
#[lints(test_without_fail_case)]
test_without_fail_case_non_fallible_paths: Vec<String> = Vec::new(),
test_without_fail_case_fallible_paths: Vec<String> =
DEFAULT_FALLIBLE_PATHS.iter().map(ToString::to_string).collect(),
/// Whether to consider indexing as a fallible operation while assesing if a test can fail.
/// Indexing is fallible, and thus the a test that is doing that can fail but it is likely
/// that tests that fail this way were not intended.
Expand All @@ -663,6 +657,14 @@ define_Conf! {
/// than the existance of asserting that thing.
#[lints(test_without_fail_case)]
test_without_fail_case_include_indexing_as_fallible: bool = false,
/// List of full paths of macros and functions, that we want to mark as "not going to fail".
/// This allows us to make the lint more focused on actual short comings of our test suite
/// by marking common routines non-fallible, even though they are fallible.
///
/// By default this list contains: `println!`, `print!`, `dbg!`.
#[lints(test_without_fail_case)]
test_without_fail_case_non_fallible_paths: Vec<String> =
DEFAULT_NONFALLIBLE_PATHS.iter().map(ToString::to_string).collect(),
/// The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap
#[lints(boxed_local, useless_vec)]
too_large_for_stack: u64 = 200,
Expand Down
14 changes: 14 additions & 0 deletions tests/ui-toml/test_without_fail_case/auxiliary/test_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#[macro_export]
macro_rules! fallible_macro {
( $x:expr ) => {{
let _ = $x;
panic!("a");
}};
}

#[macro_export]
macro_rules! non_fallible_macro {
( $x:expr ) => {{
let _ = $x;
}};
}
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-without-fail-case-fallible-paths = ["test_macro::non_fallible_macro"]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-without-fail-case-include-indexing-as-fallible = true

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test-without-fail-case-non-fallible-paths = ["test_macro::fallible_macro"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1
|
LL | / fn test_custom_macro_fallible() {
LL | | println!("a")
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1
|
LL | / fn test_indexing_fallible() {
LL | | let a = [1, 2, 3];
LL | | let _ = a[0];
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1
|
LL | / fn func_a() {
LL | | let _ = 1;
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1
|
LL | / fn should_not_lint_if_defined_as_fallible() {
LL | | non_fallible_macro!(1);
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: aborting due to 4 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1
|
LL | / fn test_custom_macro_fallible() {
LL | | println!("a")
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1
|
LL | / fn test_indexing_fallible() {
LL | | let a = [1, 2, 3];
LL | | let _ = a[0];
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1
|
LL | / fn func_a() {
LL | | let _ = 1;
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:15:1
|
LL | / fn test_custom_macro_fallible() {
LL | | println!("a")
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1
|
LL | / fn func_a() {
LL | | let _ = 1;
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1
|
LL | / fn should_not_lint_if_defined_as_fallible() {
LL | | non_fallible_macro!(1);
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:21:1
|
LL | / fn test_indexing_fallible() {
LL | | let a = [1, 2, 3];
LL | | let _ = a[0];
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:27:1
|
LL | / fn func_a() {
LL | | let _ = 1;
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:32:1
|
LL | / fn should_not_lint_if_defined_as_fallible() {
LL | | non_fallible_macro!(1);
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui-toml/test_without_fail_case/test_without_fail_case.rs:37:1
|
LL | / fn should_lint_if_defined_as_non_fallible() {
LL | | fallible_macro!(1);
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases

error: aborting due to 4 previous errors

39 changes: 24 additions & 15 deletions tests/ui-toml/test_without_fail_case/test_without_fail_case.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,41 @@
//@aux-build:test_macro.rs
//@compile-flags: --test
//@revisions: default fail_macro no_fail_macro index_fail
//@[default] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/default
//@[fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/macro_fallible
//@[no_fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/macro_non_fallible
//@[index_fail] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/indexing_fallible

//@[fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/fail_macro
//@[no_fail_macro] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/no_fail_macro
//@[index_fail] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/test_without_fail_case/index_fail
#![allow(unused)]
#![allow(clippy::tests_outside_test_module)]
#![allow(clippy::unnecessary_literal_unwrap)]
#![warn(clippy::test_without_fail_case)]

macro_rules! test {
( $( $x:expr ),* ) => {{
let _ = $x;
panic!("a");
}};
}
use test_macro::{fallible_macro, non_fallible_macro};

// Should not lint because of the clippy.toml file that adds `test` as fallible.
#[test]
fn test_custom_macro_fallible() {
test![1];
println!("a")
}

// Should lint because of the clippy.toml file makes indexing fallible.
// Should not lint unless the clippy.toml file makes indexing fallible.
#[test]
fn test_indexing_fallible() {
let a = vec![1, 2, 3];
let a = [1, 2, 3];
let _ = a[0];
}

#[test]
fn func_a() {
let _ = 1;
}

#[test]
fn should_not_lint_if_defined_as_fallible() {
non_fallible_macro!(1);
}

#[test]
fn should_lint_if_defined_as_non_fallible() {
fallible_macro!(1);
}

fn main() {}
18 changes: 15 additions & 3 deletions tests/ui/test_without_fail_case.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui/test_without_fail_case.rs:23:1
|
LL | / fn test_without_fail() {
LL | | let x = 5;
LL | | let y = x + 2;
LL | | println!("y: {}", y);
LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: this function marked with `#[test]` cannot fail and will always succeed
--> tests/ui/test_without_fail_case.rs:78:1
|
Expand All @@ -9,8 +23,6 @@ LL | | }
| |_^
|
= note: consider adding assertions or panics to test failure cases
= note: `-D clippy::test-without-fail-case` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::test_without_fail_case)]`

error: aborting due to 1 previous error
error: aborting due to 2 previous errors

0 comments on commit f6ce62a

Please sign in to comment.