Skip to content

libtest: use binary search for --exact test filtering #154865

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 1 commit into from
Apr 19, 2026
Merged

libtest: use binary search for --exact test filtering #154865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_builtin_macros/src/test_harness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ fn mk_tests_slice(cx: &TestCtxt<'_>, sp: Span) -> Box<ast::Expr> {
let ecx = &cx.ext_cx;

let mut tests = cx.test_cases.clone();
// Note that this sort is load-bearing: the libtest harness uses binary search to find tests by
// name.
tests.sort_by(|a, b| a.name.as_str().cmp(b.name.as_str()));

ecx.expr_array_ref(
Expand Down
9 changes: 5 additions & 4 deletions library/test/src/console.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use super::helpers::metrics::MetricMap;
use super::options::{Options, OutputFormat};
use super::test_result::TestResult;
use super::time::{TestExecTime, TestSuiteExecTime};
use super::types::{NamePadding, TestDesc, TestDescAndFn};
use super::types::{NamePadding, TestDesc, TestDescAndFn, TestList};
use super::{filter_tests, run_tests, term};

/// Generic wrapper over stdout.
Expand Down Expand Up @@ -170,7 +170,7 @@ impl ConsoleTestState {
}

// List the tests to console, and optionally to logfile. Filters are honored.
pub(crate) fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Result<()> {
pub(crate) fn list_tests_console(opts: &TestOpts, tests: TestList) -> io::Result<()> {
let output = match term::stdout() {
None => OutputLocation::Raw(io::stdout().lock()),
Some(t) => OutputLocation::Pretty(t),
Expand All @@ -186,7 +186,7 @@ pub(crate) fn list_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) ->
let mut st = ConsoleTestDiscoveryState::new(opts)?;

out.write_discovery_start()?;
for test in filter_tests(opts, tests).into_iter() {
for test in filter_tests(opts, tests) {
use crate::TestFn::*;

let TestDescAndFn { desc, testfn } = test;
Expand Down Expand Up @@ -307,8 +307,9 @@ pub(crate) fn get_formatter(opts: &TestOpts, max_name_len: usize) -> Box<dyn Out

/// A simple console test runner.
/// Runs provided tests reporting process and results to the stdout.
pub fn run_tests_console(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> io::Result<bool> {
pub fn run_tests_console(opts: &TestOpts, tests: TestList) -> io::Result<bool> {
let max_name_len = tests
.tests
.iter()
.max_by_key(|t| len_if_padded(t))
.map(|t| t.desc.name.as_slice().len())
Expand Down
95 changes: 78 additions & 17 deletions library/test/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub mod test {
pub use crate::time::{TestExecTime, TestTimeOptions};
pub use crate::types::{
DynTestFn, DynTestName, StaticBenchFn, StaticTestFn, StaticTestName, TestDesc,
TestDescAndFn, TestId, TestName, TestType,
TestDescAndFn, TestId, TestList, TestListOrder, TestName, TestType,
};
pub use crate::{assert_test_result, filter_tests, run_test, test_main, test_main_static};
}
Expand Down Expand Up @@ -106,6 +106,16 @@ pub fn test_main_with_exit_callback<F: FnOnce()>(
tests: Vec<TestDescAndFn>,
options: Option<Options>,
exit_callback: F,
) {
let tests = TestList::new(tests, TestListOrder::Unsorted);
test_main_inner(args, tests, options, exit_callback)
}

fn test_main_inner<F: FnOnce()>(
args: &[String],
tests: TestList,
options: Option<Options>,
exit_callback: F,
) {
let mut opts = match cli::parse_opts(args) {
Some(Ok(o)) => o,
Expand Down Expand Up @@ -180,7 +190,9 @@ pub fn test_main_with_exit_callback<F: FnOnce()>(
pub fn test_main_static(tests: &[&TestDescAndFn]) {
let args = env::args().collect::<Vec<_>>();
let owned_tests: Vec<_> = tests.iter().map(make_owned_test).collect();
test_main(&args, owned_tests, None)
// Tests are sorted by name at compile time by mk_tests_slice.
let tests = TestList::new(owned_tests, TestListOrder::Sorted);
Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the test_main(&args, owned_tests, None) invocation here, and just construct the TestList inside test_main(), where test_main() would call test_main_inner(). This would reduce code duplication a bit since both test_main_static() and test_main_static_abort() seem to be the only functions that invoke test_main() and they both have a TestListOrder::Sorted. I might be incorrect though since I notice that test_main() is calling on test_main_with_exit_callback(), which denotes the test list order as unsorted, but I'm not sure why it needs to be marked as unsorted for test_main() when the test_main_static_* functions are seemingly only one using it.

Ideally, I would have prefer test_main() to keep with calling on test_main_with_exit_callback() and not need to introduce a test_main_inner() through, making it take a TestList instead of Vec<TestDescAndFn>. However, I can see that librustdoc/doctest.rs does utilize test_main_with_exit_callback(), though I don't see why a TestList object can not be constructed there.

Copy link
Copy Markdown
Contributor Author

@sunshowers sunshowers Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I definitely thought of that, as well as the related approach of making TestList be a newtype wrapper which always ensures sortedness. Happy to hear opinions before I go about making these bigger changes.

At the moment, I believe that within this repo, merged doctests pass in a potentially-unsorted list. But I wasn't sure how much these methods are callable outside the repo.

Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, I believe that within this repo, merged doctests pass in a potentially-unsorted list. But I wasn't sure how much these methods are callable outside the repo.

Looking at librustdoc/doctest.rs, which is the only file (as far as my Go to references are telling me and a quick text search on codebase) outside this file that calls test_main_with_exit_callback, it looks like it sorts the test list as well?

    // We need to call `test_main` even if there is no doctest to run to get the output
    // `running 0 tests...`.
    if ran_edition_tests == 0 || !standalone_tests.is_empty() {
        standalone_tests.sort_by(|a, b| a.desc.name.as_slice().cmp(b.desc.name.as_slice()));
        test::test_main_with_exit_callback(&test_args, standalone_tests, None, || {
            let times = times.times_in_secs();
            // We ensure temp dir destructor is called.
            std::mem::drop(temp_dir.take());
            if let Some((total_time, compilation_time)) = times {
                test::print_merged_doctests_times(&test_args, total_time, compilation_time);
            }
        });
    }

But also since these are public methods accessible in the test crate, I'm not sure if people would need to use these methods and expect the passed in test array to handle both sorted/unsorted (which I think making TestList be a newtype wrapper that ensures sortedness is a good idea). The test module docs:

//! Support code for rustc's built in unit-test and micro-benchmarking
//! framework.
//!
//! Almost all user code will only be interested in `Bencher` and
//! `black_box`. All other interactions (such as writing tests and
//! benchmarks themselves) should be done via the `#[test]` and
//! `#[bench]` attributes.
//!
//! See the [Testing Chapter](../book/ch11-00-testing.html) of the book for more
//! details.

feels like it implies that people won't really use the test_main_* methods. Are these methods used in nextest?

Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I do see it in librust/doctest/runner.rs here

Copy link
Copy Markdown
Contributor Author

@sunshowers sunshowers Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these methods used in nextest?

No, nextest doesn't insert itself into the build process in any way.

How do you think I should proceed with this?

Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, I believe that within this repo, merged doctests pass in a potentially-unsorted list.

I was reading through, librustdoc/lib.rs, librustdoc/doctest/markdown , and librustdoc/doctest.rs, librustdoc/doctest/runner, and I think it does sort the merged doctests already.

In librustdoc/doctest.rs, that's what I'm assuming the iterative loop for run_tests(...) (lines 352-371) does for us (correct me if I'm wrong though). If that's true, then it seems like all usage of test_main* functions pass in a sorted test list.

But I wasn't sure how much these methods are callable outside the repo.
At least within librustdoc, the test list seems to be in sorted order, but because test_main* are publicly accessible functions via the test crate, it's possible for people to pass in tests that are unsorted.

How do you think I should proceed with this?

To answer this question, I prefer what you mentioned earlier here:

as well as the related approach of making TestList be a newtype wrapper which always ensures sortedness

We wouldn't need the TestListOrder enum with this approach, and can construct a TestList type in each of the test_main* functions. test_main_static* would invoke test_main underneath the hood like before. The test list being sorted internally would have to be documented for these functions as well.

Moreover, I feel like the TestList newtype should not be a publicly accessible struct, and should remain as an internal helper struct within the rust repo (pub(crate) visibility?); I can't see in what situations people would create a TestList struct and use it when all the test crate function uses TestDescAndFn (and we can't change the signature of test_main* since it would cause a breaking change -- though it is marked as experimental so not sure what policy is on that -- + means that more librustdoc code needs to be modified).

The consequence to this change is that some changes in librustdoc might need to be made (probably separately) to prevent redundant sorting, or there should be a note somewhere that there's some redundant sorting occurring in librustdoc.

Those are my thoughts and how I would probably proceed with this.

test_main_inner(&args, tests, None, || {})
}

/// A variant optimized for invocation with a static test vector.
Expand Down Expand Up @@ -229,7 +241,9 @@ pub fn test_main_static_abort(tests: &[&TestDescAndFn]) {

let args = env::args().collect::<Vec<_>>();
let owned_tests: Vec<_> = tests.iter().map(make_owned_test).collect();
test_main(&args, owned_tests, Some(Options::new().panic_abort(true)))
// Tests are sorted by name at compile time by mk_tests_slice.
let tests = TestList::new(owned_tests, TestListOrder::Sorted);
test_main_inner(&args, tests, Some(Options::new().panic_abort(true)), || {})
}

/// Clones static values for putting into a dynamic vector, which test_main()
Expand Down Expand Up @@ -298,7 +312,7 @@ impl FilteredTests {

pub fn run_tests<F>(
opts: &TestOpts,
tests: Vec<TestDescAndFn>,
tests: TestList,
mut notify_about_test_event: F,
) -> io::Result<()>
where
Expand Down Expand Up @@ -334,7 +348,7 @@ where
timeout: Instant,
}

let tests_len = tests.len();
let tests_len = tests.tests.len();

let mut filtered = FilteredTests { tests: Vec::new(), benches: Vec::new(), next_id: 0 };

Expand Down Expand Up @@ -512,25 +526,48 @@ where
Ok(())
}

pub fn filter_tests(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> Vec<TestDescAndFn> {
pub fn filter_tests(opts: &TestOpts, tests: TestList) -> Vec<TestDescAndFn> {
let TestList { tests, order } = tests;
let mut filtered = tests;
let matches_filter = |test: &TestDescAndFn, filter: &str| {
let test_name = test.desc.name.as_slice();

match opts.filter_exact {
true => test_name == filter,
false => test_name.contains(filter),
}
};

// Remove tests that don't match the test filter
// Remove tests that don't match the test filter.
if !opts.filters.is_empty() {
filtered.retain(|test| opts.filters.iter().any(|filter| matches_filter(test, filter)));
if opts.filter_exact && order == TestListOrder::Sorted {
// Let's say that `f` is the number of filters and `n` is the number
// of tests.
//
// The test array is sorted by name (guaranteed by the caller via
// TestListOrder::Sorted), so use binary search for O(f log n)
// exact-match lookups instead of an O(n) linear scan.
//
// This is important for Miri, where the interpreted execution makes
// the linear scan very expensive.
filtered = filter_exact_match(filtered, &opts.filters);
} else {
filtered.retain(|test| {
let test_name = test.desc.name.as_slice();
opts.filters.iter().any(|filter| {
if opts.filter_exact {
test_name == filter.as_str()
} else {
test_name.contains(filter.as_str())
}
})
});
}
}

// Skip tests that match any of the skip filters
//
// After exact positive filtering above, the filtered set is small, so a
// linear scan is acceptable even under Miri.
if !opts.skip.is_empty() {
filtered.retain(|test| !opts.skip.iter().any(|sf| matches_filter(test, sf)));
filtered.retain(|test| {
let name = test.desc.name.as_slice();
!opts.skip.iter().any(|sf| {
if opts.filter_exact { name == sf.as_str() } else { name.contains(sf.as_str()) }
})
});
}

// Excludes #[should_panic] tests
Expand All @@ -553,6 +590,30 @@ pub fn filter_tests(opts: &TestOpts, tests: Vec<TestDescAndFn>) -> Vec<TestDescA
filtered
}

/// Extract tests whose names exactly match one of the given `filters`, using
/// binary search on the (assumed sorted) test list.
fn filter_exact_match(mut tests: Vec<TestDescAndFn>, filters: &[String]) -> Vec<TestDescAndFn> {
// Binary search for each filter in the sorted test list.
let mut indexes: Vec<usize> = filters
.iter()
.filter_map(|f| tests.binary_search_by(|t| t.desc.name.as_slice().cmp(f.as_str())).ok())
.collect();
indexes.sort_unstable();
indexes.dedup();

// Extract matching tests. Process indexes in descending order so that
// swap_remove (which replaces the removed element with the last) does not
// invalidate indexes we haven't visited yet.
let mut result = Vec::with_capacity(indexes.len());
for &idx in indexes.iter().rev() {
result.push(tests.swap_remove(idx));
Copy link
Copy Markdown
Contributor

@asder8215 asder8215 Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no feedback here, but I just wanted to say this is the first time I've seen swap_remove being used 👀

And it's really cool how you avoid an allocation/making a copy of the TestDescAndFn (not that you could since I don't see TestDescAndFn implement the Clone trait) through this method since the index order is sorted and deduplicated, so moving the TestDescAndFn items in backward index order would always give you your filtered TestDescAndFn in reverse order. You don't have to worry about the wrong item being taken since it gets replaced by the last item in the Vec, which is not something you are taking anyways.

I'll keep a note of this in mind if I need to do something like this in the future.

}
// Reverse to restore the original sorted order, since we extracted the
// matching tests in descending index order.
result.reverse();
result
}

pub fn convert_benchmarks_to_tests(tests: Vec<TestDescAndFn>) -> Vec<TestDescAndFn> {
// convert benchmarks to tests, if we're not benchmarking them
tests
Expand Down
16 changes: 9 additions & 7 deletions library/test/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ fn filter_for_ignored_option() {
opts.run_tests = true;
opts.run_ignored = RunIgnored::Only;

let tests = one_ignored_one_unignored_test();
let tests = TestList::new(one_ignored_one_unignored_test(), TestListOrder::Unsorted);
let filtered = filter_tests(&opts, tests);

assert_eq!(filtered.len(), 1);
Expand All @@ -494,7 +494,7 @@ fn run_include_ignored_option() {
opts.run_tests = true;
opts.run_ignored = RunIgnored::Yes;

let tests = one_ignored_one_unignored_test();
let tests = TestList::new(one_ignored_one_unignored_test(), TestListOrder::Unsorted);
let filtered = filter_tests(&opts, tests);

assert_eq!(filtered.len(), 2);
Expand Down Expand Up @@ -527,16 +527,16 @@ fn exclude_should_panic_option() {
testfn: DynTestFn(Box::new(move || Ok(()))),
});

let filtered = filter_tests(&opts, tests);
let filtered = filter_tests(&opts, TestList::new(tests, TestListOrder::Unsorted));

assert_eq!(filtered.len(), 2);
assert!(filtered.iter().all(|test| test.desc.should_panic == ShouldPanic::No));
}

#[test]
fn exact_filter_match() {
fn tests() -> Vec<TestDescAndFn> {
["base", "base::test", "base::test1", "base::test2"]
fn tests() -> TestList {
let tests = ["base", "base::test", "base::test1", "base::test2"]
.into_iter()
.map(|name| TestDescAndFn {
desc: TestDesc {
Expand All @@ -555,7 +555,8 @@ fn exact_filter_match() {
},
testfn: DynTestFn(Box::new(move || Ok(()))),
})
.collect()
.collect();
TestList::new(tests, TestListOrder::Sorted)
}

let substr =
Expand Down Expand Up @@ -908,7 +909,8 @@ fn test_dyn_bench_returning_err_fails_when_run_as_test() {
}
Ok(())
};
run_tests(&TestOpts { run_tests: true, ..TestOpts::new() }, vec![desc], notify).unwrap();
let tests = TestList::new(vec![desc], TestListOrder::Unsorted);
run_tests(&TestOpts { run_tests: true, ..TestOpts::new() }, tests, notify).unwrap();
let result = rx.recv().unwrap().result;
assert_eq!(result, TrFailed);
}
27 changes: 27 additions & 0 deletions library/test/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,30 @@ impl TestDescAndFn {
}
}
}

/// Whether a [`TestList`]'s tests are known to be sorted by name.
///
/// When tests are sorted, `filter_tests` can use binary search for `--exact`
/// matches instead of a linear scan.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum TestListOrder {
/// Tests are sorted by name. This is guaranteed for tests generated by
/// `rustc --test` (see `mk_tests_slice` in
/// `compiler/rustc_builtin_macros/src/test_harness.rs`).
Sorted,
/// Test order is unknown; binary search must not be used.
Unsorted,
}

/// A list of tests, tagged with whether they are sorted by name.
#[derive(Debug)]
pub struct TestList {
pub tests: Vec<TestDescAndFn>,
pub order: TestListOrder,
}

impl TestList {
pub fn new(tests: Vec<TestDescAndFn>, order: TestListOrder) -> Self {
Self { tests, order }
}
}
Loading