Revert "#11738" - Use test name for dir when running tests

This reverts commit 64b0e79, reversing changes made to 9580786.
rust-lang · Mar 8, 2023 · f3778f9 · f3778f9
1 parent c59c623
commit f3778f9
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 113 deletions.
diff --git a/crates/cargo-test-macro/src/lib.rs b/crates/cargo-test-macro/src/lib.rs
@@ -133,9 +133,6 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
         add_attr(&mut ret, "ignore", reason);
     }
 
-    let mut test_name = None;
-    let mut num = 0;
-
     // Find where the function body starts, and add the boilerplate at the start.
     for token in item {
         let group = match token {
@@ -147,35 +144,18 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
                     continue;
                 }
             }
-            TokenTree::Ident(i) => {
-                // The first time through it will be `fn` the second time is the
-                // name of the test.
-                if test_name.is_none() && num == 1 {
-                    test_name = Some(i.to_string())
-                } else {
-                    num += 1;
-                }
-                ret.extend(Some(TokenTree::Ident(i)));
-                continue;
-            }
             other => {
                 ret.extend(Some(other));
                 continue;
             }
         };
 
-        let name = &test_name
-            .clone()
-            .map(|n| n.split("::").next().unwrap().to_string())
-            .unwrap();
-
-        let mut new_body = to_token_stream(&format!(
-            r#"let _test_guard = {{
+        let mut new_body = to_token_stream(
+            r#"let _test_guard = {
                 let tmp_dir = option_env!("CARGO_TARGET_TMPDIR");
-                let test_dir = cargo_test_support::paths::test_dir(std::file!(), "{name}");
-                cargo_test_support::paths::init_root(tmp_dir, test_dir)
-            }};"#
-        ));
+                cargo_test_support::paths::init_root(tmp_dir)
+            };"#,
+        );
 
         new_body.extend(group.stream());
         ret.extend(Some(TokenTree::from(Group::new(

diff --git a/crates/cargo-test-support/src/paths.rs b/crates/cargo-test-support/src/paths.rs
@@ -7,6 +7,7 @@ use std::fs;
 use std::io::{self, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::process::Command;
+use std::sync::atomic::{AtomicUsize, Ordering};
 use std::sync::Mutex;
 
 static CARGO_INTEGRATION_TEST_DIR: &str = "cit";
@@ -50,20 +51,28 @@ pub fn global_root() -> PathBuf {
     }
 }
 
-// We need to give each test a unique id. The test name serve this
-// purpose. We are able to get the test name by having the `cargo-test-macro`
-// crate automatically insert an init function for each test that sets the
-// test name in a thread local variable.
+// 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_NAME: RefCell<Option<PathBuf>> = RefCell::new(None);
+    static TEST_ID: RefCell<Option<usize>> = RefCell::new(None);
 }
 
 pub struct TestIdGuard {
     _private: (),
 }
 
-pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGuard {
-    TEST_NAME.with(|n| *n.borrow_mut() = Some(test_name));
+pub fn init_root(tmp_dir: Option<&'static str>) -> TestIdGuard {
+    static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
+
+    let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
+    TEST_ID.with(|n| *n.borrow_mut() = Some(id));
+
     let guard = TestIdGuard { _private: () };
 
     set_global_root(tmp_dir);
@@ -76,20 +85,20 @@ pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGua
 
 impl Drop for TestIdGuard {
     fn drop(&mut self) {
-        TEST_NAME.with(|n| *n.borrow_mut() = None);
+        TEST_ID.with(|n| *n.borrow_mut() = None);
     }
 }
 
 pub fn root() -> PathBuf {
-    let test_name = TEST_NAME.with(|n| {
-        n.borrow().clone().expect(
+    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.",
         )
     });
 
     let mut root = global_root();
-    root.push(&test_name);
+    root.push(&format!("t{}", id));
     root
 }
 
@@ -336,26 +345,3 @@ pub fn windows_reserved_names_are_allowed() -> bool {
         true
     }
 }
-
-/// This takes the test location (std::file!() should be passed) and the test name
-/// and outputs the location the test should be places in, inside of `target/tmp/cit`
-///
-/// `path: tests/testsuite/workspaces.rs`
-/// `name: `workspace_in_git
-/// `output: "testsuite/workspaces/workspace_in_git`
-pub fn test_dir(path: &str, name: &str) -> std::path::PathBuf {
-    let test_dir: std::path::PathBuf = std::path::PathBuf::from(path)
-        .components()
-        // Trim .rs from any files
-        .map(|c| c.as_os_str().to_str().unwrap().trim_end_matches(".rs"))
-        // We only want to take once we have reached `tests` or `src`. This helps when in a
-        // workspace: `workspace/more/src/...` would result in `src/...`
-        .skip_while(|c| c != &"tests" && c != &"src")
-        // We want to skip "tests" since it is taken in `skip_while`.
-        // "src" is fine since you could have test in "src" named the same as one in "tests"
-        // Skip "mod" since `snapbox` tests have a folder per test not a file and the files
-        // are named "mod.rs"
-        .filter(|c| c != &"tests" && c != &"mod")
-        .collect();
-    test_dir.join(name)
-}
diff --git a/src/doc/contrib/src/tests/writing.md b/src/doc/contrib/src/tests/writing.md
@@ -65,9 +65,8 @@ The [`#[cargo_test]` attribute](#cargo_test-attribute) is used in place of `#[te
 #### `#[cargo_test]` attribute
 
 The `#[cargo_test]` attribute injects code which does some setup before starting the test.
-It will create a filesystem "sandbox" under the "cargo integration test" directory for each test, such as
-`/path/to/cargo/target/tmp/cit/testsuite/bad_config/bad1`. The sandbox will contain a `home` directory that
-will be used instead of your normal home directory.
+It will create a filesystem "sandbox" under the "cargo integration test" directory for each test, such as `/path/to/cargo/target/tmp/cit/t123/`.
+The sandbox will contain a `home` directory that will be used instead of your normal home directory.
 
 The `#[cargo_test]` attribute takes several options that will affect how the test is generated.
 They are listed in parentheses separated with commas, such as:
@@ -201,7 +200,7 @@ Then populate
 - This attribute injects code which does some setup before starting the
   test, creating a filesystem "sandbox" under the "cargo integration test"
   directory for each test such as
-  `/path/to/cargo/target/tmp/cit/testsuite/cargo_add/add_basic`
+  `/path/to/cargo/target/cit/t123/`
 - The sandbox will contain a `home` directory that will be used instead of your normal home directory
 
 `Project`:
@@ -268,9 +267,9 @@ environment. The general process is:
 
    `cargo test --test testsuite -- features2::inactivate_targets`.
 2. In another terminal, head into the sandbox directory to inspect the files and run `cargo` directly.
-    1. The sandbox directories match the format of `tests/testsuite`, just replace `tests` with `target/tmp/cit` 
+    1. The sandbox directories start with `t0` for the first test.
 
-       `cd target/tmp/cit/testsuite/features2/inactivate_target`
+       `cd target/tmp/cit/t0`
     2. Set up the environment so that the sandbox configuration takes effect:
 
        `export CARGO_HOME=$(pwd)/home/.cargo`

diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs
@@ -144,51 +144,3 @@ fn aaa_trigger_cross_compile_disabled_check() {
     // This triggers the cross compile disabled check to run ASAP, see #5141
     cargo_test_support::cross_compile::disabled();
 }
-
-// This is placed here as running tests in `cargo-test-support` would rebuild it
-#[cargo_test]
-fn check_test_dir() {
-    let tests = vec![
-        (
-            "tests/testsuite/workspaces.rs",
-            "workspace_in_git",
-            "testsuite/workspaces/workspace_in_git",
-        ),
-        (
-            "tests/testsuite/cargo_remove/invalid_arg/mod.rs",
-            "case",
-            "testsuite/cargo_remove/invalid_arg/case",
-        ),
-        (
-            "tests/build-std/main.rs",
-            "cross_custom",
-            "build-std/main/cross_custom",
-        ),
-        (
-            "src/tools/cargo/tests/testsuite/build.rs",
-            "cargo_compile_simple",
-            "src/tools/cargo/testsuite/build/cargo_compile_simple",
-        ),
-        (
-            "src/tools/cargo/tests/testsuite/cargo_add/add_basic/mod.rs",
-            "case",
-            "src/tools/cargo/testsuite/cargo_add/add_basic/case",
-        ),
-        (
-            "src/tools/cargo/tests/build-std/main.rs",
-            "cross_custom",
-            "src/tools/cargo/build-std/main/cross_custom",
-        ),
-        (
-            "workspace/more/src/tools/cargo/tests/testsuite/build.rs",
-            "cargo_compile_simple",
-            "src/tools/cargo/testsuite/build/cargo_compile_simple",
-        ),
-    ];
-    for (path, name, expected) in tests {
-        assert_eq!(
-            cargo_test_support::paths::test_dir(path, name),
-            std::path::PathBuf::from(expected)
-        );
-    }
-}