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

Muscraft · 2025-10-15T20:02:59Z

In #11738, I made a test's "sandbox" folder based on its name instead of a generated number, which can change from run to run. #11812 reverted this change due to problems with path length limits on Windows. After getting frustrated with the generated folders while trying to debug a test recently, I decided that it would be a good idea to bring back name-based folders to platforms that support them.

rustbot · 2025-10-15T20:03:05Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

weihanglo · 2025-10-15T20:14:48Z

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

I'd like to call out @ehuss's comment #11738 (comment), though personally fine with this PR.

Not sure how I missed this when I was going back through that PR. Given that concern, it might make sense for me to bring this up in the next team meeting.

ehuss · 2025-10-28T14:10:37Z

There is some documentation in https://github.com/rust-lang/cargo/blob/HEAD/src/doc/contrib/src/tests/writing.md that would need to be updated.

ehuss · 2025-11-11T14:53:20Z

@rustbot author

I think @Muscraft indicated he could add some kind of symlink to the first test.
There's also the docs to be updated.

weihanglo · 2025-11-22T16:31:59Z

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

Ran this twice and and it failed

$ cargo t --test testsuite config_include::bad_format Finished `test` profile [unoptimized + debuginfo] target(s) in 0.80s Running tests/testsuite/main.rs (/Users/user/.cargo/build-cache/debug/deps/testsuite-da0240c1196e03cf) running 1 test test config_include::bad_format ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 3900 filtered out; finished in 0.11s $ cargo t --test testsuite config_include::bad_format Finished `test` profile [unoptimized + debuginfo] target(s) in 0.74s Running tests/testsuite/main.rs (/Users/user/.cargo/build-cache/debug/deps/testsuite-da0240c1196e03cf) running 1 test test config_include::bad_format ... FAILED failures: ---- config_include::bad_format stdout ---- thread 'config_include::bad_format' (91574537) panicked at crates/cargo-test-support/src/lib.rs:200:9: failed running os::unix::fs::symlink(&self.dst, &self.src) error: File exists (os error 17) note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace failures: config_include::bad_format test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 3900 filtered out; finished in 0.00s

I am on Mac OS 15.7.1 [64-bit]

I figured out the issue, cargo normally rm -rf's the test directory and I wasn't doing that before I tried to create the symlink.

rustbot · 2025-12-02T23:02:58Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

ehuss · 2025-12-03T23:15:53Z

@Muscraft Is this ready?

Muscraft · 2025-12-04T01:38:39Z

Yes it's ready, I just forgot to mark it as such

@rustbot ready

rustbot · 2025-12-04T17:07:02Z

☔ The latest upstream changes (possibly 2aa0773) made this pull request unmergeable. Please resolve the merge conflicts.

ehuss

Some suggestions about removing some of the duplicate code and reducing the number of lines of code to try to simplify.

View changes since this review

ehuss · 2025-12-04T16:45:48Z

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

+        let mut new_body = if cfg!(windows) {
+            to_token_stream(
+                r#"let _test_guard = {
+                    let tmp_dir = option_env!("CARGO_TARGET_TMPDIR");
+                    cargo_test_support::paths::init_root(tmp_dir)
+                };"#,
+            )
+        } else {
+            to_token_stream(&format!(
+                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)
+            }};"#
+            ))
+        };


With an eye towards reducing the amount of duplicated code, I'm wondering if we can try to combine things a little more. So, for example, this could be:

Suggested change

let mut new_body = if cfg!(windows) {

to_token_stream(

r#"let _test_guard = {

let tmp_dir = option_env!("CARGO_TARGET_TMPDIR");

cargo_test_support::paths::init_root(tmp_dir)

};"#,

)

} else {

to_token_stream(&format!(

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)

}};"#

))

};

let mut new_body = to_token_stream(&format!(

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)

}};"#

));

ehuss · 2025-12-04T16:46:44Z

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

+    #[cfg(windows)]
    static TEST_ID: RefCell<Option<usize>> = const { RefCell::new(None) };
+    #[cfg(not(windows))]
+    static TEST_NAME: RefCell<Option<PathBuf>> = const { RefCell::new(None) };


Part 2 of merging. Also, changing the name of TEST_NAME to TEST_DIR, since it doesn't include just the test name, but the full path. That was a little confusing to me.

Suggested change

#[cfg(windows)]

static TEST_ID: RefCell<Option<usize>> = const { RefCell::new(None) };

#[cfg(not(windows))]

static TEST_NAME: RefCell<Option<PathBuf>> = const { RefCell::new(None) };

static TEST_ID: RefCell<Option<usize>> = const { RefCell::new(None) };

static TEST_DIR: RefCell<Option<PathBuf>> = const { RefCell::new(None) };

ehuss · 2025-12-04T16:50:36Z

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

+#[cfg(windows)]
 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);
+    let r = root();
+    r.rm_rf();
+    r.mkdir_p();
+
+    guard
+}
+
+/// For test harnesses like [`crate::cargo_test`]
+#[cfg(not(windows))]
+pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGuard {
+    static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
+    let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);

+    TEST_NAME.with(|n| *n.borrow_mut() = Some(test_name));
    let guard = TestIdGuard { _private: () };

    set_global_root(tmp_dir);
    let r = root();
    r.rm_rf();
    r.mkdir_p();
+    if id == 0 {
+        use crate::SymlinkBuilder;

+        let mut root = global_root();
+        root.push(&format!("t{}", id));
+        root.rm_rf();
+        SymlinkBuilder::new_dir(r, root).mk();
+    }
    guard
 }


Part 3 of merge. Most of the platform-specific behavior would be here.

Part of the change here is that TEST_DIR now has the same behavior on both platforms where it represents the (relative) path of the test directory.

This also adds a comment that explains what the symlink is doing.

Suggested change

#[cfg(windows)]

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);

let r = root();

r.rm_rf();

r.mkdir_p();

guard

}

/// For test harnesses like [`crate::cargo_test`]

#[cfg(not(windows))]

pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGuard {

static NEXT_ID: AtomicUsize = AtomicUsize::new(0);

let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);

TEST_NAME.with(|n| *n.borrow_mut() = Some(test_name));

let guard = TestIdGuard { _private: () };

set_global_root(tmp_dir);

let r = root();

r.rm_rf();

r.mkdir_p();

if id == 0 {

use crate::SymlinkBuilder;

let mut root = global_root();

root.push(&format!("t{}", id));

root.rm_rf();

SymlinkBuilder::new_dir(r, root).mk();

}

guard

}

pub fn init_root(tmp_dir: Option<&'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

}

ehuss · 2025-12-04T16:51:17Z

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

+        #[cfg(windows)]
        TEST_ID.with(|n| *n.borrow_mut() = None);
+        #[cfg(not(windows))]
+        TEST_NAME.with(|n| *n.borrow_mut() = None);


Part 4 of removing the platform-specific code.

Suggested change

#[cfg(windows)]

TEST_ID.with(|n| *n.borrow_mut() = None);

#[cfg(not(windows))]

TEST_NAME.with(|n| *n.borrow_mut() = None);

TEST_ID.with(|n| *n.borrow_mut() = None);

TEST_DIR.with(|n| *n.borrow_mut() = None);

ehuss · 2025-12-04T16:53:37Z

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

 /// Path to the test's filesystem scratchpad
 ///
 /// ex: `$CARGO_TARGET_TMPDIR/cit/t0`
+#[cfg(windows)]
 pub fn root() -> PathBuf {
    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(&format!("t{}", id));
    root
 }

+/// Path to the test's filesystem scratchpad
+///
+/// ex: `$CARGO_TARGET_TMPDIR/cit/t0`
+#[cfg(not(windows))]
+pub fn root() -> PathBuf {
+    let test_name = TEST_NAME.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(&test_name);
+    root
+}
+


Part 5 of removing the platform-specific code.

This also updates the doc-comment to mention both forms.

Suggested change

/// Path to the test's filesystem scratchpad

///

/// ex: `$CARGO_TARGET_TMPDIR/cit/<integration test>/<module>/<fn name>/`

/// or `$CARGO_TARGET_TMPDIR/cit/t0` on Windows

pub fn root() -> PathBuf {

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(&test_dir);

root

}

ehuss · 2025-12-04T16:54:47Z

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


   `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.


A very minor nit. The previous text was written with an eye towards the test directory names being sequentially numbered.

Suggested change

1. The first test's sandbox directory is called `t0`.

rustbot assigned ehuss Oct 15, 2025

rustbot added A-documenting-cargo-itself Area: Cargo's documentation A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2025

weihanglo reviewed Oct 15, 2025

View reviewed changes

rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2025

Muscraft force-pushed the named-test-dirs branch from 8f90edd to cbdfb3e Compare November 18, 2025 14:20

This comment has been minimized.

Sign in to view

Muscraft force-pushed the named-test-dirs branch from cbdfb3e to 860241e Compare November 18, 2025 14:46

epage changed the title ~~feat: Use test name for dir when running tests~~ feat(test-support): Use test name for dir when running tests Nov 18, 2025

weihanglo reviewed Nov 22, 2025

View reviewed changes

feat: Use test name for dir when running tests

3f47330

Muscraft force-pushed the named-test-dirs branch from 860241e to 3f47330 Compare December 2, 2025 23:02

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Dec 4, 2025

ehuss reviewed Dec 4, 2025

View reviewed changes

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

Are you sure you want to change the base?

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

Conversation

Muscraft commented Oct 15, 2025

Uh oh!

rustbot commented Oct 15, 2025

Uh oh!

weihanglo Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ehuss commented Oct 28, 2025

Uh oh!

ehuss commented Nov 11, 2025

Uh oh!

This comment has been minimized.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Dec 2, 2025

Uh oh!

ehuss commented Dec 3, 2025

Uh oh!

Muscraft commented Dec 4, 2025

Uh oh!

rustbot commented Dec 4, 2025

Uh oh!

ehuss left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

weihanglo Oct 15, 2025 •

edited

Loading

ehuss left a comment •

edited by rustbot

Loading