Skip to content

Test function name starts with test_ #8931

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

Closed
laralove143 opened this issue Jun 2, 2022 · 10 comments · Fixed by #13710
Closed

Test function name starts with test_ #8931

laralove143 opened this issue Jun 2, 2022 · 10 comments · Fixed by #13710
Assignees
Labels
A-lint Area: New lints

Comments

@laralove143
Copy link

laralove143 commented Jun 2, 2022

What it does

None of the examples in Rust guidelines on writing tests start functions with test_, which makes for better code quality since tests::foo is more readable than tests::test_foo

Somewhat related to: clippy::module_name_repetitions

Lint Name

test_name_repetitions

Category

pedantic

Advantage

Make the code match Rust guidelines thus making it more idiomatic

Drawbacks

Not sure

Example

#[cfg(test)]
mod tests {
    #[test]
    fn test_foo() {
        assert!(true);
    }
}

Could be written as:

rust
#[cfg(test)]
mod tests {
    #[test]
    fn foo() {
        assert!(true);
    }
}
@laralove143 laralove143 added the A-lint Area: New lints label Jun 2, 2022
@ghost
Copy link

ghost commented Jun 3, 2022

I've seen that sometimes the test functions are directly in the module that gets tested and not a nested module. I guess the lint isn't applicable in that case.

@laralove143
Copy link
Author

Could you elaborate? Maybe showing a module tree

@laralove143
Copy link
Author

Is anyone looking into this?

@kraktus
Copy link
Contributor

kraktus commented Sep 5, 2022

Could you elaborate? Maybe showing a module tree

Something like that

struct Foo;

// test `Foo` in the middle of the file 
#[cfg(test)]
#[test]
fn test_foo() { todo!() }

struct Bar;

// test `Bar` in the middle of the file 
#[cfg(test)]
#[test]
fn test_bar() { todo!() }

// ...

@laralove143
Copy link
Author

laralove143 commented Sep 7, 2022

Oh I didn't know you could do that, we could just limit this lint for items in a module containing test in its name, kind of like https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions

@andrewbanchich
Copy link
Contributor

i personally don't see a point to the test_ prefix even if it isn't in a module containing test in the name.

if the function is a test, we can see

#[cfg(test)]
#[test] // it says test right here. it's the same thing, just moved up a line.
fn foo_works() { todo!() }

so it's obvious in the code. and when we run tests, we see

test foo_works ... ok

so cargo test output already adds a test at the beginning. plus, you just ran cargo test so it should be obvious that the functions you're looking at are tests.

@Caylub
Copy link

Caylub commented Apr 27, 2024

@rustbot claim

@farazdagi
Copy link
Contributor

@Caylub are you still working on this? If not, I'd like to give it a try (starting with the review comments in your PR).

@Caylub
Copy link

Caylub commented Nov 6, 2024

@farazdagi Yeah, this is my bad, I opened this with good intentions, and never followed through.

Feel free to pick this up!

@farazdagi
Copy link
Contributor

Thank you!
@rustbot claim

@rustbot rustbot assigned farazdagi and unassigned Caylub Nov 6, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 16, 2025
This PR has started as an effort to proceed from the feedback in
#12861.

- Checks test functions (functions marked with `#[test]` annotation) for
redundant "test_" prefix.
- Auto-fix is supported (and handles collisions gracefully, see below).
- If removing "test_" prefix from, say, `test_foo()` results in a name
collision (either because function `foo()` is already defined within the
current scope, or because the `foo()` call exists within function --
thus creating an unwanted recursion), lint suggests function rename,
warning the user that a simple trimming of `test_` prefix will result in
a name collision.
- If removing "test_" prefix results in invalid identifier (consider
`test_const`, `test_`, `test_42`), then again no auto-fix is suggested,
user is asked to rename function, with a note that a simple prefix
trimming will result in an invalid function name.
(`Applicability::HasPlaceholders` is used and user is suggested to: drop
`test_` prefix + add `_works` suffix, i.e. `test_foo` becomes
`foo_works` -- but again, user has to apply those changes manually).
- If trimmed version of the function name is a valid identifier, doesn't
result in name collision or unwanted recursion, then user is able to run
auto-fix.

fixes #8931

changelog: new lint: [`redundant_test_prefix`]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
5 participants