Improve unit_tests tidy lint
Make it clearer where unit tests are allowed and restrict standard library unit tests inside the same package to std_detect, std and test.
This commit is contained in:
parent
3c30dbbe31
commit
5e1ffef03c
2 changed files with 57 additions and 33 deletions
|
|
@ -127,9 +127,9 @@ fn main() {
|
|||
check!(pal, &library_path);
|
||||
|
||||
// Checks that need to be done for both the compiler and std libraries.
|
||||
check!(unit_tests, &src_path);
|
||||
check!(unit_tests, &compiler_path);
|
||||
check!(unit_tests, &library_path);
|
||||
check!(unit_tests, &src_path, false);
|
||||
check!(unit_tests, &compiler_path, false);
|
||||
check!(unit_tests, &library_path, true);
|
||||
|
||||
if bins::check_filesystem_support(&[&root_path], &output_directory) {
|
||||
check!(bins, &root_path);
|
||||
|
|
|
|||
|
|
@ -1,44 +1,60 @@
|
|||
//! Tidy check to ensure `#[test]` and `#[bench]` are not used directly inside
|
||||
//! `core` or `alloc`.
|
||||
//! of the standard library.
|
||||
//!
|
||||
//! `core` and `alloc` cannot be tested directly due to duplicating lang items.
|
||||
//! All tests and benchmarks must be written externally in
|
||||
//! `{coretests,alloctests}/{tests,benches}`.
|
||||
//!
|
||||
//! Outside of `core` and `alloc`, tests and benchmarks should be outlined into
|
||||
//! separate files named `tests.rs` or `benches.rs`, or directories named
|
||||
//! Outside of the standard library, tests and benchmarks should be outlined
|
||||
//! into separate files named `tests.rs` or `benches.rs`, or directories named
|
||||
//! `tests` or `benches` unconfigured during normal build.
|
||||
|
||||
use std::path::Path;
|
||||
|
||||
use crate::walk::{filter_dirs, walk};
|
||||
|
||||
pub fn check(root_path: &Path, bad: &mut bool) {
|
||||
let core = root_path.join("core");
|
||||
let core_copy = core.clone();
|
||||
let is_core = move |path: &Path| path.starts_with(&core);
|
||||
let alloc = root_path.join("alloc");
|
||||
let alloc_copy = alloc.clone();
|
||||
let is_alloc = move |path: &Path| path.starts_with(&alloc);
|
||||
|
||||
pub fn check(root_path: &Path, stdlib: bool, bad: &mut bool) {
|
||||
let skip = move |path: &Path, is_dir| {
|
||||
let file_name = path.file_name().unwrap_or_default();
|
||||
|
||||
// Skip excluded directories and non-rust files
|
||||
if is_dir {
|
||||
filter_dirs(path)
|
||||
|| path.ends_with("src/doc")
|
||||
|| (file_name == "tests" || file_name == "benches")
|
||||
&& !is_core(path)
|
||||
&& !is_alloc(path)
|
||||
if filter_dirs(path) || path.ends_with("src/doc") {
|
||||
return true;
|
||||
}
|
||||
} else {
|
||||
let extension = path.extension().unwrap_or_default();
|
||||
extension != "rs"
|
||||
|| (file_name == "tests.rs" || file_name == "benches.rs")
|
||||
&& !is_core(path)
|
||||
&& !is_alloc(path)
|
||||
// Tests which use non-public internals and, as such, need to
|
||||
// have the types in the same crate as the tests themselves. See
|
||||
// the comment in alloctests/lib.rs.
|
||||
|| path.ends_with("library/alloc/src/collections/btree/borrow/tests.rs")
|
||||
if extension != "rs" {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
// Tests in a separate package are always allowed
|
||||
if is_dir && file_name != "tests" && file_name.as_encoded_bytes().ends_with(b"tests") {
|
||||
return true;
|
||||
}
|
||||
|
||||
if !stdlib {
|
||||
// Outside of the standard library tests may also be in separate files in the same crate
|
||||
if is_dir {
|
||||
if file_name == "tests" || file_name == "benches" {
|
||||
return true;
|
||||
}
|
||||
} else {
|
||||
if file_name == "tests.rs" || file_name == "benches.rs" {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if is_dir {
|
||||
// FIXME remove those exceptions once no longer necessary
|
||||
file_name == "std_detect" || file_name == "std" || file_name == "test"
|
||||
} else {
|
||||
// Tests which use non-public internals and, as such, need to
|
||||
// have the types in the same crate as the tests themselves. See
|
||||
// the comment in alloctests/lib.rs.
|
||||
path.ends_with("library/alloc/src/collections/btree/borrow/tests.rs")
|
||||
|| path.ends_with("library/alloc/src/collections/btree/map/tests.rs")
|
||||
|| path.ends_with("library/alloc/src/collections/btree/node/tests.rs")
|
||||
|| path.ends_with("library/alloc/src/collections/btree/set/tests.rs")
|
||||
|
|
@ -50,22 +66,30 @@ pub fn check(root_path: &Path, bad: &mut bool) {
|
|||
|
||||
walk(root_path, skip, &mut |entry, contents| {
|
||||
let path = entry.path();
|
||||
let is_core = path.starts_with(&core_copy);
|
||||
let is_alloc = path.starts_with(&alloc_copy);
|
||||
let package = path
|
||||
.strip_prefix(root_path)
|
||||
.unwrap()
|
||||
.components()
|
||||
.next()
|
||||
.unwrap()
|
||||
.as_os_str()
|
||||
.to_str()
|
||||
.unwrap();
|
||||
for (i, line) in contents.lines().enumerate() {
|
||||
let line = line.trim();
|
||||
let is_test = || line.contains("#[test]") && !line.contains("`#[test]");
|
||||
let is_bench = || line.contains("#[bench]") && !line.contains("`#[bench]");
|
||||
let manual_skip = line.contains("//tidy:skip");
|
||||
if !line.starts_with("//") && (is_test() || is_bench()) && !manual_skip {
|
||||
let explanation = if is_core {
|
||||
"`core` unit tests and benchmarks must be placed into `coretests`"
|
||||
} else if is_alloc {
|
||||
"`alloc` unit tests and benchmarks must be placed into `alloctests`"
|
||||
let explanation = if stdlib {
|
||||
format!(
|
||||
"`{package}` unit tests and benchmarks must be placed into `{package}tests`"
|
||||
)
|
||||
} else {
|
||||
"unit tests and benchmarks must be placed into \
|
||||
separate files or directories named \
|
||||
`tests.rs`, `benches.rs`, `tests` or `benches`"
|
||||
.to_owned()
|
||||
};
|
||||
let name = if is_test() { "test" } else { "bench" };
|
||||
tidy_error!(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue