From a598309cb66bcd8313b8e2a084e49b56a5ed16aa Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Thu, 2 May 2019 14:58:52 -0700 Subject: [PATCH] Don't rely on a thread local to uniquely create test roots --- .gitignore | 1 + Cargo.toml | 1 + tests/testsuite/main.rs | 3 + .../support/cargo-test-macro/Cargo.toml | 12 ++ .../support/cargo-test-macro/src/lib.rs | 24 ++++ tests/testsuite/support/paths.rs | 107 ++++++++++++------ 6 files changed, 112 insertions(+), 36 deletions(-) create mode 100644 tests/testsuite/support/cargo-test-macro/Cargo.toml create mode 100644 tests/testsuite/support/cargo-test-macro/src/lib.rs diff --git a/.gitignore b/.gitignore index 85e363a3758..add90d768b9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /target +/tests/testsuite/support/cargo-test-macro/target Cargo.lock .cargo /config.stamp diff --git a/Cargo.toml b/Cargo.toml index dc7eedc711c..d667bb682ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -104,6 +104,7 @@ features = [ bufstream = "0.1" proptest = "0.9.1" varisat = "0.2.1" +cargo-test-macro = { "path" = "tests/testsuite/support/cargo-test-macro" } [[bin]] name = "cargo" diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 682e4f00d0f..0c08fa49b7b 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -6,6 +6,9 @@ #![warn(clippy::needless_borrow)] #![warn(clippy::redundant_clone)] +#[macro_use] +extern crate cargo_test_macro; + #[macro_use] mod support; diff --git a/tests/testsuite/support/cargo-test-macro/Cargo.toml b/tests/testsuite/support/cargo-test-macro/Cargo.toml new file mode 100644 index 00000000000..9c5bb7f2c50 --- /dev/null +++ b/tests/testsuite/support/cargo-test-macro/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "cargo-test-macro" +version = "0.1.0" +authors = ["Jethro Beekman "] +edition = "2018" + +[lib] +proc-macro = true + +[dependencies] +quote = "0.6" +syn = { version = "0.15", features = ["full"] } diff --git a/tests/testsuite/support/cargo-test-macro/src/lib.rs b/tests/testsuite/support/cargo-test-macro/src/lib.rs new file mode 100644 index 00000000000..3a4b736bc23 --- /dev/null +++ b/tests/testsuite/support/cargo-test-macro/src/lib.rs @@ -0,0 +1,24 @@ +extern crate proc_macro; + +use quote::{quote, ToTokens}; +use syn::{*, parse::Parser}; + +#[proc_macro_attribute] +pub fn cargo_test( + _attr: proc_macro::TokenStream, + item: proc_macro::TokenStream, +) -> proc_macro::TokenStream { + let mut fn_def = parse_macro_input!(item as ItemFn); + + let attr = quote! { + #[test] + }; + fn_def.attrs.extend(Attribute::parse_outer.parse2(attr).unwrap()); + + let stmt = quote! { + let _test_guard = crate::support::paths::init_root(); + }; + fn_def.block.stmts.insert(0, parse2(stmt).unwrap()); + + fn_def.into_token_stream().into() +} diff --git a/tests/testsuite/support/paths.rs b/tests/testsuite/support/paths.rs index ac57e54b76a..66868d27b76 100644 --- a/tests/testsuite/support/paths.rs +++ b/tests/testsuite/support/paths.rs @@ -1,58 +1,93 @@ -use std::cell::Cell; +use std::cell::RefCell; +use std::collections::HashMap; use std::env; use std::fs; use std::io::{self, ErrorKind}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::{Once, ONCE_INIT}; +use std::sync::Mutex; use filetime::{self, FileTime}; +use lazy_static::lazy_static; static CARGO_INTEGRATION_TEST_DIR: &'static str = "cit"; -static NEXT_ID: AtomicUsize = AtomicUsize::new(0); - -thread_local!(static TASK_ID: usize = NEXT_ID.fetch_add(1, Ordering::SeqCst)); - -fn init() { - static GLOBAL_INIT: Once = ONCE_INIT; - thread_local!(static LOCAL_INIT: Cell = Cell::new(false)); - GLOBAL_INIT.call_once(|| { - global_root().mkdir_p(); - }); - LOCAL_INIT.with(|i| { - if i.get() { - return; + +lazy_static! { + static ref GLOBAL_ROOT: PathBuf = { + let mut path = t!(env::current_exe()); + path.pop(); // chop off exe name + path.pop(); // chop off 'debug' + + // If `cargo test` is run manually then our path looks like + // `target/debug/foo`, in which case our `path` is already pointing at + // `target`. If, however, `cargo test --target $target` is used then the + // output is `target/$target/debug/foo`, so our path is pointing at + // `target/$target`. Here we conditionally pop the `$target` name. + if path.file_name().and_then(|s| s.to_str()) != Some("target") { + path.pop(); } - i.set(true); - root().rm_rf(); - home().mkdir_p(); - }) + + path.push(CARGO_INTEGRATION_TEST_DIR); + + path.rm_rf(); + path.mkdir_p(); + + path + }; + + static ref TEST_ROOTS: Mutex> = Default::default(); } -fn global_root() -> PathBuf { - let mut path = t!(env::current_exe()); - path.pop(); // chop off exe name - path.pop(); // chop off 'debug' - - // If `cargo test` is run manually then our path looks like - // `target/debug/foo`, in which case our `path` is already pointing at - // `target`. If, however, `cargo test --target $target` is used then the - // output is `target/$target/debug/foo`, so our path is pointing at - // `target/$target`. Here we conditionally pop the `$target` name. - if path.file_name().and_then(|s| s.to_str()) != Some("target") { - path.pop(); - } +// We need to give each test a unique id. The test name could serve this +// purpose, but the `test` crate doesn't have a way to obtain the current test +// name.[*] Instead, we used the `cargo-test-macro` crate to automatically +// insert an init function for each test that sets the test name in a thread +// local variable. +// +// [*] It does set the thread name, but only when running concurrently. If not +// running concurrently, all tests are run on the main thread. +thread_local! { + static TEST_ID: RefCell> = RefCell::new(None); +} - path.join(CARGO_INTEGRATION_TEST_DIR) +pub struct TestIdGuard { + _private: () +} + +pub fn init_root() -> TestIdGuard { + static NEXT_ID: AtomicUsize = AtomicUsize::new(0); + + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + TEST_ID.with(|n| { *n.borrow_mut() = Some(id) } ); + + let guard = TestIdGuard { + _private: () + }; + + root().mkdir_p(); + + guard +} + +impl Drop for TestIdGuard { + fn drop(&mut self) { + TEST_ID.with(|n| { *n.borrow_mut() = None } ); + } } pub fn root() -> PathBuf { - init(); - global_root().join(&TASK_ID.with(|my_id| format!("t{}", my_id))) + let id = TEST_ID.with(|n| { + n.borrow().expect("Tests must use the `#[cargo_test]` attribute in \ + order to be able to use the crate root.") + } ); + GLOBAL_ROOT.join(&format!("t{}", id)) } pub fn home() -> PathBuf { - root().join("home") + let mut path = root(); + path.push("home"); + path.mkdir_p(); + path } pub trait CargoPathExt {