fix: Do not warn tests outside a module for integration tests

[berkus]

Fixes a false positive that triggers tests_outside_test_module in integration tests. As per The Rust Book the integration tests do not need neither #[cfg(test)] nor a separate module because they are separate binary crates.

changelog: [tests_outside_test_module] Do not lint for integration tests
fixes #11024

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Alexendoo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[berkus]

I am asking for some guidance on how to test the lint on the unit tests though - the test suite I see in clippy is actually integration tests.

[Alexendoo]

Being in tests does not mean that it's an integration test, e.g. mod tests; is not uncommon - https://github.com/rust-lang/rust/tree/9ffe52e05b063f5fea020d501216a35559ca8620/compiler/rustc_errors/src/markdown/tests

Similarly an integration test can live outside the tests directory - https://github.com/DioxusLabs/dioxus/blob/e1ea213d056d22dcc801a68b0f24300d32b0c90d/packages/desktop/Cargo.toml#L101-L104

[berkus]

Being in tests does not mean that it's an integration test, e.g. mod tests; is not uncommon - https://github.com/rust-lang/rust/tree/9ffe52e05b063f5fea020d501216a35559ca8620/compiler/rustc_errors/src/markdown/tests

in this case I believe it should work, as the test file path will not start with tests. I can try and add a test for it, again if I understand how to do unit testing in clippy source.

Similarly an integration test can live outside the tests directory - https://github.com/DioxusLabs/dioxus/blob/e1ea213d056d22dcc801a68b0f24300d32b0c90d/packages/desktop/Cargo.toml#L101-L104

For such custom paths there should be some Cargo manifest parsing, I guess. Again, any guidelines to an accepted approach are appreciated.

[Alexendoo]

in this case I believe it should work, as the test file path will not start with tests. I can try and add a test for it, again if I understand how to do unit testing in clippy source.

Ah I missed that it checked the first path component, that can be a problem in workspaces as the working directory would be the workspace root rather than the package root. Tests inside an integration test can also live in submodules

For such custom paths there should be some Cargo manifest parsing, I guess. Again, any guidelines to an accepted approach are appreciated.

Sorry to say I don't have a good answer here, we generally try to avoid parsing Cargo things

[Alexendoo]

Looks like libtest does store this information, it's approximated by checking the last path component of the directory the crate entry point is located in

https://github.com/rust-lang/rust/blob/a91f7d72f12efcc00ecf71591f066c534d45ddf7/compiler/rustc_expand/src/expand.rs#L402-L409

https://github.com/rust-lang/rust/blob/a91f7d72f12efcc00ecf71591f066c534d45ddf7/compiler/rustc_builtin_macros/src/test.rs#L526

We could copy that or update https://doc.rust-lang.org/nightly/nightly-rustc/clippy_utils/fn.with_test_item_names.html to check the description for the TestType to ensure consistency

[xFrednet]

Hey @berkus , this is a ping from triage, since there hasn't been any activity in some time. Are you still planning to continue this implementation?

If you have any questions, you're always welcome to ask them in this PR or on Zulip.

@rustbot author

[berkus]

Hi, yes, i will get back on it next week.

[berkus]

it's approximated by checking the last path component of the directory the crate entry point is located in

I've applied this, but need to update tests; short on time recently, will get back to it soon.

[oxalica]

Got here from #11119 which is also about false positive in integration tests. Seems your is_integration_test function in this PR would be useful for that too. Could we move it to clippy_utils for use in is_in_test? So we can also fix #11119 by the way.

[berkus]

Yeah that should be possible, I will find some time to fix the remaining tests and move the fn - this week!

[bors]

☔ The latest upstream changes (presumably #13440) made this pull request unmergeable. Please resolve the merge conflicts.

[berkus]

Ok @bors will do. Still finding time to mess with the tests setup.