Skip to content

Commit a4a395a

Browse files
committed
Avoid iterating files when --exact is passed in
This fixes a quadratic performance issue in which the test directories are iterated for every test case when run under nextest.
1 parent 2c6057a commit a4a395a

File tree

4 files changed

+136
-9
lines changed

4 files changed

+136
-9
lines changed

src/runner.rs

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ use libtest_mimic::{Arguments, Trial};
1111
pub fn runner(requirements: &[Requirements]) -> ExitCode {
1212
let args = Arguments::from_args();
1313

14-
let mut tests: Vec<_> = requirements.iter().flat_map(|req| req.expand()).collect();
15-
tests.sort_unstable_by(|a, b| a.name().cmp(b.name()));
14+
let tests = find_tests(&args, requirements);
1615

1716
let conclusion = libtest_mimic::run(&args, tests);
1817

@@ -25,6 +24,50 @@ pub fn runner(requirements: &[Requirements]) -> ExitCode {
2524
conclusion.exit_code()
2625
}
2726

27+
fn find_tests(args: &Arguments, requirements: &[Requirements]) -> Vec<Trial> {
28+
let tests: Vec<_> = if let Some(exact_filter) = exact_filter(args) {
29+
let exact_tests: Vec<_> = requirements
30+
.iter()
31+
.flat_map(|req| req.exact(exact_filter))
32+
.collect();
33+
34+
if is_nextest() {
35+
if exact_tests.is_empty() {
36+
panic!("Failed to find exact match for filter {exact_filter}");
37+
} else if exact_tests.len() > 1 {
38+
panic!(
39+
"Only expected one but found {} exact matches for filter {exact_filter}",
40+
exact_tests.len()
41+
);
42+
}
43+
}
44+
exact_tests
45+
} else if is_full_scan_forbidden(args) {
46+
panic!("Exact filter was expected to be used");
47+
} else {
48+
let mut tests: Vec<_> = requirements.iter().flat_map(|req| req.expand()).collect();
49+
tests.sort_unstable_by(|a, b| a.name().cmp(b.name()));
50+
tests
51+
};
52+
tests
53+
}
54+
55+
fn is_nextest() -> bool {
56+
std::env::var("NEXTEST").as_deref() == Ok("1")
57+
}
58+
59+
fn is_full_scan_forbidden(args: &Arguments) -> bool {
60+
!args.list && std::env::var("__DATATEST_FULL_SCAN_FORBIDDEN").as_deref() == Ok("1")
61+
}
62+
63+
fn exact_filter(args: &Arguments) -> Option<&str> {
64+
if args.exact && args.skip.is_empty() {
65+
args.filter.as_deref()
66+
} else {
67+
None
68+
}
69+
}
70+
2871
#[doc(hidden)]
2972
pub struct Requirements {
3073
test: TestFn,
@@ -49,6 +92,21 @@ impl Requirements {
4992
}
5093
}
5194

95+
fn trial(&self, path: Utf8PathBuf) -> Trial {
96+
let testfn = self.test;
97+
let name = utils::derive_test_name(&self.root, &path, &self.test_name);
98+
Trial::test(name, move || {
99+
testfn
100+
.call(&path)
101+
.map_err(|err| format!("{:?}", err).into())
102+
})
103+
}
104+
105+
fn exact(&self, filter: &str) -> Option<Trial> {
106+
let path = utils::derive_test_path(&self.root, filter, &self.test_name)?;
107+
path.exists().then(|| self.trial(path))
108+
}
109+
52110
/// Scans all files in a given directory, finds matching ones and generates a test descriptor
53111
/// for each of them.
54112
fn expand(&self) -> Vec<Trial> {
@@ -66,13 +124,7 @@ impl Requirements {
66124
error
67125
)
68126
}) {
69-
let testfn = self.test;
70-
let name = utils::derive_test_name(&self.root, &path, &self.test_name);
71-
Some(Trial::test(name, move || {
72-
testfn
73-
.call(&path)
74-
.map_err(|err| format!("{:?}", err).into())
75-
}))
127+
Some(self.trial(path))
76128
} else {
77129
None
78130
}

src/utils.rs

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,76 @@ pub fn derive_test_name(root: &Utf8Path, path: &Utf8Path, test_name: &str) -> St
3333

3434
format!("{}::{}", test_name, relative)
3535
}
36+
37+
pub fn derive_test_path(root: &Utf8Path, filter: &str, test_name: &str) -> Option<Utf8PathBuf> {
38+
let relative = filter.strip_prefix(test_name)?.strip_prefix("::")?;
39+
Some(root.join(relative))
40+
}
41+
42+
#[cfg(test)]
43+
mod tests {
44+
use super::*;
45+
46+
mod derive_path {
47+
use super::*;
48+
49+
#[test]
50+
fn missing_test_name() {
51+
assert_eq!(derive_test_path("root".into(), "file", "test_name"), None);
52+
}
53+
54+
#[test]
55+
fn missing_colons() {
56+
assert_eq!(
57+
derive_test_path("root".into(), "test_name", "test_name"),
58+
None
59+
);
60+
}
61+
62+
#[test]
63+
fn is_relative_to_root() {
64+
assert_eq!(
65+
derive_test_path("root".into(), "test_name::file", "test_name"),
66+
Some("root/file".into())
67+
);
68+
assert_eq!(
69+
derive_test_path("root2".into(), "test_name::file", "test_name"),
70+
Some("root2/file".into())
71+
);
72+
}
73+
74+
#[test]
75+
fn nested_dirs() {
76+
assert_eq!(
77+
derive_test_path("root".into(), "test_name::dir/dir2/file", "test_name"),
78+
Some("root/dir/dir2/file".into())
79+
);
80+
}
81+
82+
#[test]
83+
fn subsequent_module_separators_remain() {
84+
assert_eq!(
85+
derive_test_path("root".into(), "test_name::mod::file", "test_name"),
86+
Some("root/mod::file".into())
87+
);
88+
}
89+
90+
#[test]
91+
fn inverse_of_derive_test_name() {
92+
let root: Utf8PathBuf = "root".into();
93+
for (path, test_name) in [
94+
(root.join("foo/bar.txt"), "test_name"),
95+
(root.join("foo::bar.txt"), "test_name"),
96+
(root.join("foo/bar/baz"), "test_name"),
97+
(root.join("foo"), "test_name::mod"),
98+
(root.join("🦀"), "🚀::🚀"),
99+
] {
100+
let derived_test_name = derive_test_name(&root, &path, test_name);
101+
assert_eq!(
102+
derive_test_path(&root, &derived_test_name, test_name),
103+
Some(path)
104+
);
105+
}
106+
}
107+
}
108+
}

tests/files/::colon::dir/::.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
floop

tests/files/::colon::dir/a.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
flarp

0 commit comments

Comments
 (0)