From c0df742de8af58f06b6d6d4af3244ae470976f53 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 26 Jul 2019 01:18:35 +0300 Subject: [PATCH 1/3] tidy: libcoretest.rs -> unit_tests.rs --- src/tools/tidy/src/lib.rs | 2 +- src/tools/tidy/src/main.rs | 2 +- src/tools/tidy/src/{libcoretest.rs => unit_tests.rs} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename src/tools/tidy/src/{libcoretest.rs => unit_tests.rs} (100%) diff --git a/src/tools/tidy/src/lib.rs b/src/tools/tidy/src/lib.rs index 06c1a8c18bac7..3d40bdced63e0 100644 --- a/src/tools/tidy/src/lib.rs +++ b/src/tools/tidy/src/lib.rs @@ -38,8 +38,8 @@ pub mod pal; pub mod deps; pub mod extdeps; pub mod ui_tests; +pub mod unit_tests; pub mod unstable_book; -pub mod libcoretest; fn filter_dirs(path: &Path) -> bool { let skip = [ diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs index 918762ed6e69a..19f02f0a96e1d 100644 --- a/src/tools/tidy/src/main.rs +++ b/src/tools/tidy/src/main.rs @@ -27,7 +27,7 @@ fn main() { let collected = features::check(&path, &mut bad, verbose); pal::check(&path, &mut bad); unstable_book::check(&path, collected, &mut bad); - libcoretest::check(&path, &mut bad); + unit_tests::check(&path, &mut bad); if !args.iter().any(|s| *s == "--no-vendor") { deps::check(&path, &mut bad); } diff --git a/src/tools/tidy/src/libcoretest.rs b/src/tools/tidy/src/unit_tests.rs similarity index 100% rename from src/tools/tidy/src/libcoretest.rs rename to src/tools/tidy/src/unit_tests.rs From 6a4def0c9df85649336a52607347f613efcca918 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 26 Jul 2019 01:22:59 +0300 Subject: [PATCH 2/3] tidy: Fix a regression in `#[test]` detection in libcore `contents` is the whole file rather than a single line. --- src/tools/tidy/src/unit_tests.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/tools/tidy/src/unit_tests.rs b/src/tools/tidy/src/unit_tests.rs index 579a20e1142be..6c61144f27931 100644 --- a/src/tools/tidy/src/unit_tests.rs +++ b/src/tools/tidy/src/unit_tests.rs @@ -13,14 +13,16 @@ pub fn check(path: &Path, bad: &mut bool) { &mut |entry, contents| { let subpath = entry.path(); if let Some("rs") = subpath.extension().and_then(|e| e.to_str()) { - let contents = contents.trim(); - if !contents.starts_with("//") && contents.contains("#[test]") { - tidy_error!( - bad, - "`{}` contains `#[test]`; libcore tests must be placed inside \ - `src/libcore/tests/`", - subpath.display() - ); + for line in contents.lines() { + let line = line.trim(); + if !line.starts_with("//") && line.contains("#[test]") { + tidy_error!( + bad, + "`{}` contains `#[test]`; libcore tests must be placed inside \ + `src/libcore/tests/`", + subpath.display() + ); + } } } }, From aecaa03cf607d92d5ef5346400dd5302de8e5303 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 26 Jul 2019 03:01:18 +0300 Subject: [PATCH 3/3] tidy: Add a check for inline unit tests and benchmarks --- src/tools/tidy/src/unit_tests.rs | 103 +++++++++++++++++++++++++------ 1 file changed, 84 insertions(+), 19 deletions(-) diff --git a/src/tools/tidy/src/unit_tests.rs b/src/tools/tidy/src/unit_tests.rs index 6c61144f27931..b789e5cfd48d5 100644 --- a/src/tools/tidy/src/unit_tests.rs +++ b/src/tools/tidy/src/unit_tests.rs @@ -1,28 +1,93 @@ -//! Tidy check to ensure `#[test]` is not used directly inside `libcore`. +//! Tidy check to ensure `#[test]` and `#[bench]` are not used directly inside +//! `libcore` or `liballoc`. //! -//! `#![no_core]` libraries cannot be tested directly due to duplicating lang -//! item. All tests must be written externally in `libcore/tests`. +//! `#![no_std]` libraries cannot be tested directly due to duplicating lang +//! items. All tests and benchmarks must be written externally in `libcore/{tests,benches}` +//! or `liballoc/{tests,benches}`. +//! +//! Outside of libcore and liballoc 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; -pub fn check(path: &Path, bad: &mut bool) { - let libcore_path = path.join("libcore"); +pub fn check(root_path: &Path, bad: &mut bool) { + let libcore = &root_path.join("libcore"); + let liballoc = &root_path.join("liballoc"); + let libcore_tests = &root_path.join("libcore/tests"); + let liballoc_tests = &root_path.join("liballoc/tests"); + let libcore_benches = &root_path.join("libcore/benches"); + let liballoc_benches = &root_path.join("liballoc/benches"); + let is_core_or_alloc = |path: &Path| { + let is_core = path.starts_with(libcore) && + !(path.starts_with(libcore_tests) || path.starts_with(libcore_benches)); + let is_alloc = path.starts_with(liballoc) && + !(path.starts_with(liballoc_tests) || path.starts_with(liballoc_benches)); + is_core || is_alloc + }; + let fixme = [ + "liballoc", + "libpanic_unwind/dwarf", + "librustc", + "librustc_data_structures", + "librustc_incremental/persist", + "librustc_lexer/src", + "librustc_target/spec", + "librustdoc", + "libserialize", + "libstd", + "libsyntax", + "libsyntax_pos", + "libterm/terminfo", + "libtest", + "tools/compiletest/src", + "tools/tidy/src", + ]; + + let mut skip = |path: &Path| { + let file_name = path.file_name().unwrap_or_default(); + if path.is_dir() { + super::filter_dirs(path) || + path.ends_with("src/test") || + path.ends_with("src/doc") || + (file_name == "tests" || file_name == "benches") && !is_core_or_alloc(path) || + fixme.iter().any(|p| path.ends_with(p)) + } else { + let extension = path.extension().unwrap_or_default(); + extension != "rs" || + (file_name == "tests.rs" || file_name == "benches.rs") && !is_core_or_alloc(path) + } + }; + super::walk( - &libcore_path, - &mut |subpath| t!(subpath.strip_prefix(&libcore_path)).starts_with("tests"), + root_path, + &mut skip, &mut |entry, contents| { - let subpath = entry.path(); - if let Some("rs") = subpath.extension().and_then(|e| e.to_str()) { - for line in contents.lines() { - let line = line.trim(); - if !line.starts_with("//") && line.contains("#[test]") { - tidy_error!( - bad, - "`{}` contains `#[test]`; libcore tests must be placed inside \ - `src/libcore/tests/`", - subpath.display() - ); - } + let path = entry.path(); + let is_libcore = path.starts_with(libcore); + let is_liballoc = path.starts_with(liballoc); + 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]"); + if !line.starts_with("//") && (is_test() || is_bench()) { + let explanation = if is_libcore { + "libcore unit tests and benchmarks must be placed into \ + `libcore/tests` or `libcore/benches`" + } else if is_liballoc { + "liballoc unit tests and benchmarks must be placed into \ + `liballoc/tests` or `liballoc/benches`" + } else { + "unit tests and benchmarks must be placed into \ + separate files or directories named \ + `tests.rs`, `benches.rs`, `tests` or `benches`" + }; + let name = if is_test() { "test" } else { "bench" }; + tidy_error!( + bad, "`{}:{}` contains `#[{}]`; {}", + path.display(), i + 1, name, explanation, + ); + return; } } },