diff --git a/Cargo.lock b/Cargo.lock index 7b7d9272e54..f7727eca4d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -476,7 +476,7 @@ version = "0.4.9" [[package]] name = "cargo-test-support" -version = "0.9.2" +version = "0.10.0" dependencies = [ "anstream", "anstyle", diff --git a/Cargo.toml b/Cargo.toml index d80b6228fb6..392758a17db 100644 --- a/Cargo.toml +++ b/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" diff --git a/crates/cargo-test-macro/src/lib.rs b/crates/cargo-test-macro/src/lib.rs index 8acf4f114fb..623b2c05e8f 100644 --- a/crates/cargo-test-macro/src/lib.rs +++ b/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( diff --git a/crates/cargo-test-support/Cargo.toml b/crates/cargo-test-support/Cargo.toml index 0c62f1bdb66..e0441080a7c 100644 --- a/crates/cargo-test-support/Cargo.toml +++ b/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 diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 1a749007b78..0fe13139f4f 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/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. diff --git a/crates/cargo-test-support/src/paths.rs b/crates/cargo-test-support/src/paths.rs index a7400380ee5..f9542c0aed8 100644 --- a/crates/cargo-test-support/src/paths.rs +++ b/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> = const { RefCell::new(None) }; + static TEST_DIR: RefCell> = 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////` +/// 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) +} diff --git a/src/doc/contrib/src/tests/writing.md b/src/doc/contrib/src/tests/writing.md index 99eef916d3f..a44893dd2d8 100644 --- a/src/doc/contrib/src/tests/writing.md +++ b/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////` - 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. diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 9817b179556..0fb0f3d5d22 100644 --- a/tests/testsuite/main.rs +++ b/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) + ); + } +}