Stricter test modules

[JonathanWoollett-Light]

What it does

It is easier to illustrate this as a lint formed of multiple lints.

• test_outside_tests_module: Warns when a unit test is outside a tests module.

• unflagged_tests_module: Warns when a module named tests is not flagged with #[cfg(test)].

• non_eof_tests_module: Warns when a module named tests is not at the end of a file.

Lint Name

strict_tests_module

Category

style, pedantic

Advantage

• Enforces the idiomatic style of unit tests.
• Allows tools like grcov to correctly exclude unit tests from coverage with --excl-start "mod tests".

Drawbacks

• Tools like bindgen often include tests outside test modules. This would warn on this case.

Example

#[test]
fn my_test() { /* ... */ }
#[cfg(test)]
mod tests { /* ... */ }

Could be written as:

#[cfg(test)]
mod tests {
    #[test]
    fn my_test() { /* ... */ }
    /* ... */
}

---

mod tests { /* ... */ }

Could be written as:

#[cfg(test)]
mod tests { /* ... */ }

---

#[cfg(test)]
mod tests { /* ... */ }
fn an_item_after() { /* ... */ }

Could be written as:

fn an_item_after() { /* ... */ }
#[cfg(test)]
mod tests { /* ... */ }

[blyxyas]

I think this issue would be better formatted as a task list instead of a numbered list, just for clarity.

@rustbot claim

[JonathanWoollett-Light]

@blyxyas Changed to task list.

[Manishearth]

See #10515 (comment): unflagged_test_modules is probably not worth it.

The other two sound useful though.

[blyxyas]

@JonathanWoollett-Light Could you use Markdown's ~~strikethrough~~ on the second task to indicate it won't be completed?

[Manishearth]

done

[Alexendoo]

I'd say tests outside a test module ought to be a restriction lint, it's a pretty common pattern in the wild

As an approximation a GitHub search of lang:rs /^#\[test/ NOT path:test finds 94.7k files

[smoelius]

I interpreted the lint to mean: a test and a test module within the same enclosing file/module (which I think would be unusual).

In other words, I interpreted the mod tests in the example to be necessary.

[Alexendoo]

Definitely not as popular but still 1.9k results

Looking a few of them it seems common to put a top level test fn right next to the thing it's testing even if there's a test module later in the file, e.g. https://github.com/tokio-rs/axum/blob/b314cd9bc23f6eeb62a1f11a80b95a1720c7816d/axum/src/extract/request_parts.rs#L174

[blyxyas]

non_eof_test_modules isn't really a clear name (It assumes that the user knows what EOF means), maybe items_after_test_module?

[Alexendoo]

For things after the test module I found a couple cases that seem like they'd be worth ignoring

Another test or benchmark module following the initial test module, e.g. https://github.com/xi-editor/xi-editor/blob/d027769cec36c3b479ead6a5e963fb5a664149a9/rust/rope/src/delta.rs#L887

And test-ish things like static asserts, e.g. https://github.com/rome/tools/blob/763fd98ad9072a17ca2ef7ed29c6a164eedc8a6b/crates/rome_formatter/src/format_element.rs#L379

The second being less clear cut, I could see someone wanting them to go above the tests module