Skip to content

Commit 6ec18d5

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 6ec18d5

File tree

4 files changed

+149
-9
lines changed

4 files changed

+149
-9
lines changed

src/runner.rs

Lines changed: 74 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,59 @@ 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() {
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_os("NEXTEST")
57+
.filter(|var| var == "1")
58+
.is_some()
59+
}
60+
61+
fn is_full_scan_forbidden() -> bool {
62+
#[cfg(test)]
63+
{
64+
std::env::var_os("DATATEST_FULL_SCAN_FORBIDDEN")
65+
.filter(|var| var == "1")
66+
.is_some()
67+
}
68+
#[cfg(not(test))]
69+
false
70+
}
71+
72+
fn exact_filter(args: &Arguments) -> Option<&str> {
73+
if args.exact && args.skip.is_empty() {
74+
args.filter.as_deref()
75+
} else {
76+
None
77+
}
78+
}
79+
2880
#[doc(hidden)]
2981
pub struct Requirements {
3082
test: TestFn,
@@ -49,6 +101,25 @@ impl Requirements {
49101
}
50102
}
51103

104+
fn trial(&self, path: Utf8PathBuf) -> Trial {
105+
let testfn = self.test;
106+
let name = utils::derive_test_name(&self.root, &path, &self.test_name);
107+
Trial::test(name, move || {
108+
testfn
109+
.call(&path)
110+
.map_err(|err| format!("{:?}", err).into())
111+
})
112+
}
113+
114+
fn exact(&self, filter: &str) -> Option<Trial> {
115+
let path = utils::derive_test_path(&self.root, filter, &self.test_name)?;
116+
if path.exists() {
117+
Some(self.trial(path))
118+
} else {
119+
None
120+
}
121+
}
122+
52123
/// Scans all files in a given directory, finds matching ones and generates a test descriptor
53124
/// for each of them.
54125
fn expand(&self) -> Vec<Trial> {
@@ -66,13 +137,7 @@ impl Requirements {
66137
error
67138
)
68139
}) {
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-
}))
140+
Some(self.trial(path))
76141
} else {
77142
None
78143
}

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)