Skip to content

Commit

Permalink
Add a check to ensure tests with overlapping prefixes aren't added.
Browse files Browse the repository at this point in the history
Some tests will delete their output directory before starting.
The output directory is based on the test names.
If one test is the prefix of another test, then when that test
starts, it could try to delete the output directory of the other
test with the longer path.
  • Loading branch information
ehuss committed Mar 23, 2023
1 parent 2da2ade commit 1692d0c
Showing 1 changed file with 35 additions and 2 deletions.
37 changes: 35 additions & 2 deletions src/tools/compiletest/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use build_helper::git::{get_git_modified_files, get_git_untracked_files};
use core::panic;
use getopts::Options;
use lazycell::LazyCell;
use std::collections::BTreeSet;
use std::ffi::OsString;
use std::fs;
use std::io::{self, ErrorKind};
Expand Down Expand Up @@ -406,7 +407,9 @@ pub fn run_tests(config: Config) {

let mut tests = Vec::new();
for c in &configs {
make_tests(c, &mut tests);
let mut found_paths = BTreeSet::new();
make_tests(c, &mut tests, &mut found_paths);
check_overlapping_tests(&found_paths);
}

tests.sort_by(|a, b| a.desc.name.as_slice().cmp(&b.desc.name.as_slice()));
Expand Down Expand Up @@ -528,7 +531,11 @@ pub fn test_opts(config: &Config) -> test::TestOpts {
}
}

pub fn make_tests(config: &Config, tests: &mut Vec<test::TestDescAndFn>) {
pub fn make_tests(
config: &Config,
tests: &mut Vec<test::TestDescAndFn>,
found_paths: &mut BTreeSet<PathBuf>,
) {
debug!("making tests from {:?}", config.src_base.display());
let inputs = common_inputs_stamp(config);
let modified_tests = modified_tests(config, &config.src_base).unwrap_or_else(|err| {
Expand All @@ -540,6 +547,7 @@ pub fn make_tests(config: &Config, tests: &mut Vec<test::TestDescAndFn>) {
&PathBuf::new(),
&inputs,
tests,
found_paths,
&modified_tests,
)
.unwrap_or_else(|_| panic!("Could not read tests from {}", config.src_base.display()));
Expand Down Expand Up @@ -610,6 +618,7 @@ fn collect_tests_from_dir(
relative_dir_path: &Path,
inputs: &Stamp,
tests: &mut Vec<test::TestDescAndFn>,
found_paths: &mut BTreeSet<PathBuf>,
modified_tests: &Vec<PathBuf>,
) -> io::Result<()> {
// Ignore directories that contain a file named `compiletest-ignore-dir`.
Expand Down Expand Up @@ -643,6 +652,8 @@ fn collect_tests_from_dir(
let file_name = file.file_name();
if is_test(&file_name) && (!config.only_modified || modified_tests.contains(&file_path)) {
debug!("found test file: {:?}", file_path.display());
let rel_test_path = relative_dir_path.join(file_path.file_stem().unwrap());
found_paths.insert(rel_test_path);
let paths =
TestPaths { file: file_path, relative_dir: relative_dir_path.to_path_buf() };

Expand All @@ -657,6 +668,7 @@ fn collect_tests_from_dir(
&relative_file_path,
inputs,
tests,
found_paths,
modified_tests,
)?;
}
Expand Down Expand Up @@ -1072,3 +1084,24 @@ fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> {
fn not_a_digit(c: char) -> bool {
!c.is_digit(10)
}

fn check_overlapping_tests(found_paths: &BTreeSet<PathBuf>) {
let mut collisions = Vec::new();
for path in found_paths {
for ancestor in path.ancestors().skip(1) {
if found_paths.contains(ancestor) {
collisions.push((path, ancestor.clone()));
}
}
}
if !collisions.is_empty() {
let collisions: String = collisions
.into_iter()
.map(|(path, check_parent)| format!("test {path:?} clashes with {check_parent:?}\n"))
.collect();
panic!(
"{collisions}\n\
Tests cannot have overlapping names. Make sure they use unique prefixes."
);
}
}

0 comments on commit 1692d0c

Please sign in to comment.