Skip to content

Commit 7d3033d

Browse files
committed
Auto merge of #11812 - weihanglo:revert-11738, r=epage
Revert "#11738" - Use test name for dir when running tests
2 parents c59c623 + f3778f9 commit 7d3033d

File tree

4 files changed

+30
-113
lines changed

4 files changed

+30
-113
lines changed

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

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,6 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
133133
add_attr(&mut ret, "ignore", reason);
134134
}
135135

136-
let mut test_name = None;
137-
let mut num = 0;
138-
139136
// Find where the function body starts, and add the boilerplate at the start.
140137
for token in item {
141138
let group = match token {
@@ -147,35 +144,18 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
147144
continue;
148145
}
149146
}
150-
TokenTree::Ident(i) => {
151-
// The first time through it will be `fn` the second time is the
152-
// name of the test.
153-
if test_name.is_none() && num == 1 {
154-
test_name = Some(i.to_string())
155-
} else {
156-
num += 1;
157-
}
158-
ret.extend(Some(TokenTree::Ident(i)));
159-
continue;
160-
}
161147
other => {
162148
ret.extend(Some(other));
163149
continue;
164150
}
165151
};
166152

167-
let name = &test_name
168-
.clone()
169-
.map(|n| n.split("::").next().unwrap().to_string())
170-
.unwrap();
171-
172-
let mut new_body = to_token_stream(&format!(
173-
r#"let _test_guard = {{
153+
let mut new_body = to_token_stream(
154+
r#"let _test_guard = {
174155
let tmp_dir = option_env!("CARGO_TARGET_TMPDIR");
175-
let test_dir = cargo_test_support::paths::test_dir(std::file!(), "{name}");
176-
cargo_test_support::paths::init_root(tmp_dir, test_dir)
177-
}};"#
178-
));
156+
cargo_test_support::paths::init_root(tmp_dir)
157+
};"#,
158+
);
179159

180160
new_body.extend(group.stream());
181161
ret.extend(Some(TokenTree::from(Group::new(

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

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::fs;
77
use std::io::{self, ErrorKind};
88
use std::path::{Path, PathBuf};
99
use std::process::Command;
10+
use std::sync::atomic::{AtomicUsize, Ordering};
1011
use std::sync::Mutex;
1112

1213
static CARGO_INTEGRATION_TEST_DIR: &str = "cit";
@@ -50,20 +51,28 @@ pub fn global_root() -> PathBuf {
5051
}
5152
}
5253

53-
// We need to give each test a unique id. The test name serve this
54-
// purpose. We are able to get the test name by having the `cargo-test-macro`
55-
// crate automatically insert an init function for each test that sets the
56-
// test name in a thread local variable.
54+
// We need to give each test a unique id. The test name could serve this
55+
// purpose, but the `test` crate doesn't have a way to obtain the current test
56+
// name.[*] Instead, we used the `cargo-test-macro` crate to automatically
57+
// insert an init function for each test that sets the test name in a thread
58+
// local variable.
59+
//
60+
// [*] It does set the thread name, but only when running concurrently. If not
61+
// running concurrently, all tests are run on the main thread.
5762
thread_local! {
58-
static TEST_NAME: RefCell<Option<PathBuf>> = RefCell::new(None);
63+
static TEST_ID: RefCell<Option<usize>> = RefCell::new(None);
5964
}
6065

6166
pub struct TestIdGuard {
6267
_private: (),
6368
}
6469

65-
pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGuard {
66-
TEST_NAME.with(|n| *n.borrow_mut() = Some(test_name));
70+
pub fn init_root(tmp_dir: Option<&'static str>) -> TestIdGuard {
71+
static NEXT_ID: AtomicUsize = AtomicUsize::new(0);
72+
73+
let id = NEXT_ID.fetch_add(1, Ordering::SeqCst);
74+
TEST_ID.with(|n| *n.borrow_mut() = Some(id));
75+
6776
let guard = TestIdGuard { _private: () };
6877

6978
set_global_root(tmp_dir);
@@ -76,20 +85,20 @@ pub fn init_root(tmp_dir: Option<&'static str>, test_name: PathBuf) -> TestIdGua
7685

7786
impl Drop for TestIdGuard {
7887
fn drop(&mut self) {
79-
TEST_NAME.with(|n| *n.borrow_mut() = None);
88+
TEST_ID.with(|n| *n.borrow_mut() = None);
8089
}
8190
}
8291

8392
pub fn root() -> PathBuf {
84-
let test_name = TEST_NAME.with(|n| {
85-
n.borrow().clone().expect(
93+
let id = TEST_ID.with(|n| {
94+
n.borrow().expect(
8695
"Tests must use the `#[cargo_test]` attribute in \
8796
order to be able to use the crate root.",
8897
)
8998
});
9099

91100
let mut root = global_root();
92-
root.push(&test_name);
101+
root.push(&format!("t{}", id));
93102
root
94103
}
95104

@@ -336,26 +345,3 @@ pub fn windows_reserved_names_are_allowed() -> bool {
336345
true
337346
}
338347
}
339-
340-
/// This takes the test location (std::file!() should be passed) and the test name
341-
/// and outputs the location the test should be places in, inside of `target/tmp/cit`
342-
///
343-
/// `path: tests/testsuite/workspaces.rs`
344-
/// `name: `workspace_in_git
345-
/// `output: "testsuite/workspaces/workspace_in_git`
346-
pub fn test_dir(path: &str, name: &str) -> std::path::PathBuf {
347-
let test_dir: std::path::PathBuf = std::path::PathBuf::from(path)
348-
.components()
349-
// Trim .rs from any files
350-
.map(|c| c.as_os_str().to_str().unwrap().trim_end_matches(".rs"))
351-
// We only want to take once we have reached `tests` or `src`. This helps when in a
352-
// workspace: `workspace/more/src/...` would result in `src/...`
353-
.skip_while(|c| c != &"tests" && c != &"src")
354-
// We want to skip "tests" since it is taken in `skip_while`.
355-
// "src" is fine since you could have test in "src" named the same as one in "tests"
356-
// Skip "mod" since `snapbox` tests have a folder per test not a file and the files
357-
// are named "mod.rs"
358-
.filter(|c| c != &"tests" && c != &"mod")
359-
.collect();
360-
test_dir.join(name)
361-
}

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,8 @@ The [`#[cargo_test]` attribute](#cargo_test-attribute) is used in place of `#[te
6565
#### `#[cargo_test]` attribute
6666

6767
The `#[cargo_test]` attribute injects code which does some setup before starting the test.
68-
It will create a filesystem "sandbox" under the "cargo integration test" directory for each test, such as
69-
`/path/to/cargo/target/tmp/cit/testsuite/bad_config/bad1`. The sandbox will contain a `home` directory that
70-
will be used instead of your normal home directory.
68+
It will create a filesystem "sandbox" under the "cargo integration test" directory for each test, such as `/path/to/cargo/target/tmp/cit/t123/`.
69+
The sandbox will contain a `home` directory that will be used instead of your normal home directory.
7170

7271
The `#[cargo_test]` attribute takes several options that will affect how the test is generated.
7372
They are listed in parentheses separated with commas, such as:
@@ -201,7 +200,7 @@ Then populate
201200
- This attribute injects code which does some setup before starting the
202201
test, creating a filesystem "sandbox" under the "cargo integration test"
203202
directory for each test such as
204-
`/path/to/cargo/target/tmp/cit/testsuite/cargo_add/add_basic`
203+
`/path/to/cargo/target/cit/t123/`
205204
- The sandbox will contain a `home` directory that will be used instead of your normal home directory
206205

207206
`Project`:
@@ -268,9 +267,9 @@ environment. The general process is:
268267

269268
`cargo test --test testsuite -- features2::inactivate_targets`.
270269
2. In another terminal, head into the sandbox directory to inspect the files and run `cargo` directly.
271-
1. The sandbox directories match the format of `tests/testsuite`, just replace `tests` with `target/tmp/cit`
270+
1. The sandbox directories start with `t0` for the first test.
272271

273-
`cd target/tmp/cit/testsuite/features2/inactivate_target`
272+
`cd target/tmp/cit/t0`
274273
2. Set up the environment so that the sandbox configuration takes effect:
275274

276275
`export CARGO_HOME=$(pwd)/home/.cargo`

tests/testsuite/main.rs

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -144,51 +144,3 @@ fn aaa_trigger_cross_compile_disabled_check() {
144144
// This triggers the cross compile disabled check to run ASAP, see #5141
145145
cargo_test_support::cross_compile::disabled();
146146
}
147-
148-
// This is placed here as running tests in `cargo-test-support` would rebuild it
149-
#[cargo_test]
150-
fn check_test_dir() {
151-
let tests = vec![
152-
(
153-
"tests/testsuite/workspaces.rs",
154-
"workspace_in_git",
155-
"testsuite/workspaces/workspace_in_git",
156-
),
157-
(
158-
"tests/testsuite/cargo_remove/invalid_arg/mod.rs",
159-
"case",
160-
"testsuite/cargo_remove/invalid_arg/case",
161-
),
162-
(
163-
"tests/build-std/main.rs",
164-
"cross_custom",
165-
"build-std/main/cross_custom",
166-
),
167-
(
168-
"src/tools/cargo/tests/testsuite/build.rs",
169-
"cargo_compile_simple",
170-
"src/tools/cargo/testsuite/build/cargo_compile_simple",
171-
),
172-
(
173-
"src/tools/cargo/tests/testsuite/cargo_add/add_basic/mod.rs",
174-
"case",
175-
"src/tools/cargo/testsuite/cargo_add/add_basic/case",
176-
),
177-
(
178-
"src/tools/cargo/tests/build-std/main.rs",
179-
"cross_custom",
180-
"src/tools/cargo/build-std/main/cross_custom",
181-
),
182-
(
183-
"workspace/more/src/tools/cargo/tests/testsuite/build.rs",
184-
"cargo_compile_simple",
185-
"src/tools/cargo/testsuite/build/cargo_compile_simple",
186-
),
187-
];
188-
for (path, name, expected) in tests {
189-
assert_eq!(
190-
cargo_test_support::paths::test_dir(path, name),
191-
std::path::PathBuf::from(expected)
192-
);
193-
}
194-
}

0 commit comments

Comments
 (0)