feat(test-support): Use test name for dir when running tests

Cargo.toml

@@ -32,7 +32,7 @@ cargo-credential-macos-keychain = { version = "0.4.20", path = "credential/cargo
 cargo-credential-wincred = { version = "0.4.20", path = "credential/cargo-credential-wincred" }
 cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }
 cargo-test-macro = { version = "0.4.9", path = "crates/cargo-test-macro" }
-cargo-test-support = { version = "0.9.2", path = "crates/cargo-test-support" }
+cargo-test-support = { version = "0.10.0", path = "crates/cargo-test-support" }
 cargo-util = { version = "0.2.27", path = "crates/cargo-util" }
 cargo-util-schemas = { version = "0.12.0", path = "crates/cargo-util-schemas" }
 cargo_metadata = "0.23.1"

crates/cargo-test-macro/src/lib.rs

@@ -200,6 +200,9 @@ 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 {
@@ -211,18 +214,35 @@ 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 mut new_body = to_token_stream(
-            r#"let _test_guard = {
+        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 tmp_dir = env!("CARGO_TARGET_TMPDIR");
-                cargo_test_support::paths::init_root(tmp_dir)
-            };"#,
-        );
+                let test_dir = cargo_test_support::paths::test_dir(std::file!(), "{name}");
+                cargo_test_support::paths::init_root(tmp_dir, test_dir)
+            }};"#
+        ));
 
         new_body.extend(group.stream());
         ret.extend(Some(TokenTree::from(Group::new(

crates/cargo-test-support/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cargo-test-support"
-version = "0.9.2"
+version = "0.10.0"
 edition.workspace = true
 rust-version = "1.92"  # MSRV:1
 license.workspace = true

crates/cargo-test-support/src/lib.rs

@@ -1423,7 +1423,13 @@ pub trait TestEnvCommandExt: Sized {
             .env("__CARGO_TEST_DISABLE_GLOBAL_KNOWN_HOST", "1")
             // Set retry sleep to 1 millisecond.
             .env("__CARGO_TEST_FIXED_RETRY_SLEEP_MS", "1")
-            .env("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS", "200")
+            // Setting this to a large number helps avoid problems with long
+            // paths getting trimmed in snapshot tests.
+            //
+            // When updating this value, keep in mind that the `CARGO_TARGET_DIR`
+            // that gets set when Cargo's tests get run in `rust-lang/rust` can
+            // easily cause path lengths to exceed 200 characters.
+            .env("__CARGO_TEST_TTY_WIDTH_DO_NOT_USE_THIS", "400")
             // Incremental generates a huge amount of data per test, which we
             // don't particularly need. Tests that specifically need to check
             // the incremental behavior should turn this back on.

crates/cargo-test-support/src/paths.rs

@@ -46,16 +46,13 @@ pub fn global_root() -> PathBuf {
     }
 }
 
-// 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.
+// 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.
 thread_local! {
     static TEST_ID: RefCell<Option<usize>> = const { RefCell::new(None) };
+    static TEST_DIR: RefCell<Option<PathBuf>> = const { RefCell::new(None) };
 }
 
 /// See [`init_root`]
@@ -64,44 +61,56 @@ pub struct TestIdGuard {
 }
 
 /// For test harnesses like [`crate::cargo_test`]
-pub fn init_root(tmp_dir: &'static str) -> TestIdGuard {
+pub fn init_root(tmp_dir: &'static str, test_dir: PathBuf) -> 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));
-
+    if cfg!(windows) {
+        // Due to path-length limits, Windows doesn't use the full test name.
+        TEST_DIR.with(|n| *n.borrow_mut() = Some(PathBuf::from(format!("t{id}"))));
+    } else {
+        TEST_DIR.with(|n| *n.borrow_mut() = Some(test_dir));
+    }
     let guard = TestIdGuard { _private: () };
-
     set_global_root(tmp_dir);
     let r = root();
     r.rm_rf();
     r.mkdir_p();
-
+    #[cfg(not(windows))]
+    if id == 0 {
+        // Create a symlink from `t0` to the first test to make it easier to
+        // find and reuse when running a single test.
+        use crate::SymlinkBuilder;
+        let mut alias = global_root();
+        alias.push("t0");
+        alias.rm_rf();
+        SymlinkBuilder::new_dir(r, alias).mk();
+    }
     guard
 }
 
 impl Drop for TestIdGuard {
     fn drop(&mut self) {
         TEST_ID.with(|n| *n.borrow_mut() = None);
+        TEST_DIR.with(|n| *n.borrow_mut() = None);
     }
 }
 
 /// Path to the test's filesystem scratchpad
 ///
-/// ex: `$CARGO_TARGET_TMPDIR/cit/t0`
+/// ex: `$CARGO_TARGET_TMPDIR/cit/<integration test>/<module>/<fn name>/`
+/// or `$CARGO_TARGET_TMPDIR/cit/t0` on Windows
 pub fn root() -> PathBuf {
-    let id = TEST_ID.with(|n| {
-        n.borrow().expect(
+    let test_dir = TEST_DIR.with(|n| {
+        n.borrow().clone().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(&format!("t{}", id));
+    root.push(&test_dir);
     root
 }
-
 /// Path to the current test's `$HOME`
 ///
 /// ex: `$CARGO_TARGET_TMPDIR/cit/t0/home`
@@ -508,3 +517,26 @@ 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)
+}

src/doc/contrib/src/tests/writing.md

@@ -200,8 +200,10 @@ Then populate
 - This is used in place of `#[test]`
 - 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/cit/t123/`
+  directory for each test. The directory for each test is based on the
+  integration test name, module (if there is one), and function name[^1]:
+
+  `/path/to/cargo/target/tmp/cit/<integration test>/<module>/<fn name>/`
 - The sandbox will contain a `home` directory that will be used instead of your normal home directory
 
 `Project`:
@@ -258,6 +260,11 @@ or overwrite a binary immediately after running it. Under some conditions
 Windows will fail with errors like "directory not empty" or "failed to remove"
 or "access is denied".
 
+On Windows, to avoid path length limitations, the tests use the following
+directory structure instead:
+
+`/path/to/cargo/target/tmp/cit/t123/`
+
 ## Debugging tests
 
 In some cases, you may need to dig into a test that is not working as you
@@ -268,7 +275,7 @@ 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 start with `t0` for the first test.
+    1. The first test's sandbox directory is called `t0`.
 
        `cd target/tmp/cit/t0`
     2. Set up the environment so that the sandbox configuration takes effect:
@@ -299,3 +306,6 @@ environment. The general process is:
 [`Command`]: https://docs.rs/snapbox/latest/snapbox/cmd/struct.Command.html
 [`OutputAssert`]: https://docs.rs/snapbox/latest/snapbox/cmd/struct.OutputAssert.html
 [`Assert`]: https://docs.rs/snapbox/latest/snapbox/struct.Assert.html
+
+[^1]: Windows uses a separate directory layout, see [Platform-Specific Notes](#platform-specific-notes)
+    for more details.

tests/testsuite/main.rs

@@ -212,3 +212,52 @@ fn aaa_trigger_cross_compile_disabled_check() {
     // This triggers the cross compile disabled check to run ASAP, see #5141
     crate::utils::cross_compile::disabled();
 }
+
+// This is placed here as running tests in `cargo-test-support` would rebuild it
+#[cargo_test]
+#[cfg(not(windows))]
+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)
+        );
+    }
+}