Skip to content

Commit

Permalink
Avoid iterating files when --exact is passed in
Browse files Browse the repository at this point in the history
This fixes a quadratic performance issue in which the test directories are iterated for every test
case when run under nextest.
  • Loading branch information
zaneduffield committed Apr 23, 2024
1 parent 2c6057a commit 9c4ee86
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 9 deletions.
67 changes: 58 additions & 9 deletions src/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ use libtest_mimic::{Arguments, Trial};
pub fn runner(requirements: &[Requirements]) -> ExitCode {
let args = Arguments::from_args();

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

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

Expand All @@ -25,6 +24,43 @@ pub fn runner(requirements: &[Requirements]) -> ExitCode {
conclusion.exit_code()
}

fn find_tests(args: &Arguments, requirements: &[Requirements]) -> Vec<Trial> {
let tests: Vec<_> = if let Some(exact_filter) = exact_filter(args) {
let exact_tests: Vec<_> = requirements
.iter()
.flat_map(|req| req.exact(exact_filter))
.collect();

match exact_tests.len() {
0 if is_nextest() => {
panic!("Failed to find exact match for filter {exact_filter}");

Check warning on line 36 in src/runner.rs

View check run for this annotation

Codecov / codecov/patch

src/runner.rs#L35-L36

Added lines #L35 - L36 were not covered by tests
}
len @ (2..) if is_nextest() => {
panic!("Only expected one but found {len} exact matches for filter {exact_filter}");

Check warning on line 39 in src/runner.rs

View check run for this annotation

Codecov / codecov/patch

src/runner.rs#L39

Added line #L39 was not covered by tests
}
_ => {}
}
exact_tests
} else {
let mut tests: Vec<_> = requirements.iter().flat_map(|req| req.expand()).collect();
tests.sort_unstable_by(|a, b| a.name().cmp(b.name()));
tests
};
tests
}

fn is_nextest() -> bool {
std::env::var_os("NEXTEST").is_some_and(|val| val.eq("1"))
}

Check warning on line 54 in src/runner.rs

View check run for this annotation

Codecov / codecov/patch

src/runner.rs#L52-L54

Added lines #L52 - L54 were not covered by tests

fn exact_filter(args: &Arguments) -> Option<&str> {
if args.exact && args.skip.is_empty() {
args.filter.as_deref()
} else {
None
}
}

#[doc(hidden)]
pub struct Requirements {
test: TestFn,
Expand All @@ -49,6 +85,25 @@ impl Requirements {
}
}

fn trial(&self, path: Utf8PathBuf) -> Trial {
let testfn = self.test;
let name = utils::derive_test_name(&self.root, &path, &self.test_name);
Trial::test(name, move || {
testfn
.call(&path)
.map_err(|err| format!("{:?}", err).into())
})
}

fn exact(&self, filter: &str) -> Option<Trial> {
let path = utils::derive_test_path(&self.root, filter, &self.test_name)?;
if path.exists() {
Some(self.trial(path))
} else {
None

Check warning on line 103 in src/runner.rs

View check run for this annotation

Codecov / codecov/patch

src/runner.rs#L103

Added line #L103 was not covered by tests
}
}

/// Scans all files in a given directory, finds matching ones and generates a test descriptor
/// for each of them.
fn expand(&self) -> Vec<Trial> {
Expand All @@ -66,13 +121,7 @@ impl Requirements {
error
)
}) {
let testfn = self.test;
let name = utils::derive_test_name(&self.root, &path, &self.test_name);
Some(Trial::test(name, move || {
testfn
.call(&path)
.map_err(|err| format!("{:?}", err).into())
}))
Some(self.trial(path))
} else {
None
}
Expand Down
5 changes: 5 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ pub fn derive_test_name(root: &Utf8Path, path: &Utf8Path, test_name: &str) -> St

format!("{}::{}", test_name, relative)
}

pub fn derive_test_path(root: &Utf8Path, filter: &str, test_name: &str) -> Option<Utf8PathBuf> {
let relative = filter.strip_prefix(test_name)?.strip_prefix("::")?;
Some(root.join(relative))
}
1 change: 1 addition & 0 deletions tests/files/::colon::dir/::.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
floop
1 change: 1 addition & 0 deletions tests/files/::colon::dir/a.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
flarp

0 comments on commit 9c4ee86

Please sign in to comment.