Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

items_after_test_module interacts badly with rstest and submodules #11050

Closed
asomers opened this issue Jun 30, 2023 · 0 comments · Fixed by #11611
Closed

items_after_test_module interacts badly with rstest and submodules #11050

asomers opened this issue Jun 30, 2023 · 0 comments · Fixed by #11611
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@asomers
Copy link

asomers commented Jun 30, 2023

Summary

I like to organize my tests into multiple modules just to provide namespacing. For example, I might use one module for each function under test, and each of those modules will contain one function for each test case, like this:

mod func1 {
    use super::*;
    #[test]
    fn test_case1() {...}
    #[test]
    fn test_case2() {...}
}
mod func2 {
    use super::*;
    #[test]
    fn test_case1() {...}
    #[test]
    fn test_case2() {...}
}

The problem is that if anything above the final module includes a #[cfg(test)], then the items_after_test_module lint will fire. That's even true in integration tests, which aren't built with and without #[cfg(test)]. The problem is exacerbated by macro crates like rstest, which sometimes emit a #[cfg(test)] in the generated code. And it's frustrating to debug, because the user can't even see the offending #[cfg(test)].
I don't know what a perfect solution would be. But skipping this lint in integration tests would be a good start. Or, skipping this lint within an outer module which is itself #[cfg(test)].

Lint Name

items_after_test_module

Reproducer

I tried this code:

use rstest::rstest;

#[rstest]
#[case(42)]
fn t(#[case] _x: u32) {}

mod func1 {
    struct SomethingUsedByTheTest{}

    #[test]
    fn t() {}
}

I saw this happen:

warning: items were found after the testing module
  --> tests/t.rs:3:1
   |
3  |   #[rstest]
   |   ^--------
   |   |
   |  _in this procedural macro expansion
   | |
4  | | #[case(42)]
5  | | fn t(#[case] _x: u32) {}
6  | |
...  |
10 | |     #[test]
11 | |     fn t() {}
   | |_____________^
   |
   = help: move the items to before the testing module was defined
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module
   = note: `#[warn(clippy::items_after_test_module)]` on by default
   = note: this warning originates in the attribute macro `rstest` (in Nightly builds, run with -Z macro-backtrace for more info)

I expected to see this happen:

No errors (except for SomethingUsedByTheTest is never constructed, of course).

Version

rustc 1.72.0-nightly (5bd28f5ea 2023-06-28)
binary: rustc
commit-hash: 5bd28f5eac1ba3569bfa8d49ec3f5acbdfdff7a0
commit-date: 2023-06-28
host: x86_64-unknown-freebsd
release: 1.72.0-nightly
LLVM version: 16.0.5

Additional Labels

No response

@asomers asomers added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jun 30, 2023
@bors bors closed this as completed in 279127c Oct 6, 2023
asomers added a commit to asomers/fsx-rs that referenced this issue Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant