From cec59864393bfd7efab2834b895fc73fbb59f78e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 13 Oct 2024 23:22:57 +1100 Subject: [PATCH] Document various parts of compiletest's `lib.rs` --- src/tools/compiletest/src/lib.rs | 127 +++++++++++++++++++++++++++---- 1 file changed, 114 insertions(+), 13 deletions(-) diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 18000e5602c..2e66c084dd7 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -43,6 +43,11 @@ use crate::common::{ use crate::header::HeadersCache; use crate::util::logv; +/// Creates the `Config` instance for this invocation of compiletest. +/// +/// The config mostly reflects command-line arguments, but there might also be +/// some code here that inspects environment variables or even runs executables +/// (e.g. when discovering debugger versions). pub fn parse_config(args: Vec) -> Config { let mut opts = Options::new(); opts.reqopt("", "compile-lib-path", "path to host shared libraries", "PATH") @@ -413,6 +418,7 @@ pub fn opt_str2(maybestr: Option) -> String { } } +/// Called by `main` after the config has been parsed. pub fn run_tests(config: Arc) { // If we want to collect rustfix coverage information, // we first make sure that the coverage file does not exist. @@ -454,6 +460,8 @@ pub fn run_tests(config: Arc) { configs.push(config.clone()); }; + // Discover all of the tests in the test suite directory, and build a libtest + // structure for each test (or each revision of a multi-revision test). let mut tests = Vec::new(); for c in configs { let mut found_paths = HashSet::new(); @@ -463,7 +471,12 @@ pub fn run_tests(config: Arc) { tests.sort_by(|a, b| a.desc.name.as_slice().cmp(&b.desc.name.as_slice())); + // Delegate to libtest to filter and run the big list of structures created + // during test discovery. When libtest decides to run a test, it will invoke + // the corresponding closure created by `make_test_closure`. let res = test::run_tests_console(&opts, tests); + + // Check the outcome reported by libtest. match res { Ok(true) => {} Ok(false) => { @@ -532,6 +545,11 @@ pub fn test_opts(config: &Config) -> test::TestOpts { } } +/// Creates libtest structures for every test/revision in the test suite directory. +/// +/// This always inspects _all_ test files in the suite (e.g. all 17k+ ui tests), +/// regardless of whether any filters/tests were specified on the command-line, +/// because filtering is handled later by libtest. pub fn make_tests( config: Arc, tests: &mut Vec, @@ -610,10 +628,17 @@ fn common_inputs_stamp(config: &Config) -> Stamp { stamp } +/// Returns a list of modified/untracked test files that should be run when +/// the `--only-modified` flag is in use. +/// +/// (Might be inaccurate in some cases.) fn modified_tests(config: &Config, dir: &Path) -> Result, String> { + // If `--only-modified` wasn't passed, the list of modified tests won't be + // used for anything, so avoid some work and just return an empty list. if !config.only_modified { return Ok(vec![]); } + let files = get_git_modified_files(&config.git_config(), Some(dir), &vec!["rs", "stderr", "fixed"])? .unwrap_or(vec![]); @@ -634,6 +659,8 @@ fn modified_tests(config: &Config, dir: &Path) -> Result, String> { Ok(full_paths) } +/// Recursively scans a directory to find test files and create test structures +/// that will be handed over to libtest. fn collect_tests_from_dir( config: Arc, cache: &HeadersCache, @@ -650,6 +677,8 @@ fn collect_tests_from_dir( return Ok(()); } + // For run-make tests, a "test file" is actually a directory that contains + // an `rmake.rs` or `Makefile`" if config.mode == Mode::RunMake { if dir.join("Makefile").exists() && dir.join("rmake.rs").exists() { return Err(io::Error::other( @@ -663,6 +692,7 @@ fn collect_tests_from_dir( relative_dir: relative_dir_path.parent().unwrap().to_path_buf(), }; tests.extend(make_test(config, cache, &paths, inputs, poisoned)); + // This directory is a test, so don't try to find other tests inside it. return Ok(()); } } @@ -677,22 +707,27 @@ fn collect_tests_from_dir( fs::create_dir_all(&build_dir).unwrap(); // Add each `.rs` file as a test, and recurse further on any - // subdirectories we find, except for `aux` directories. + // subdirectories we find, except for `auxiliary` directories. // FIXME: this walks full tests tree, even if we have something to ignore // use walkdir/ignore like in tidy? for file in fs::read_dir(dir)? { let file = file?; let file_path = file.path(); let file_name = file.file_name(); + if is_test(&file_name) && (!config.only_modified || modified_tests.contains(&file_path)) { + // We found a test file, so create the corresponding libtest structures. debug!("found test file: {:?}", file_path.display()); + + // Record the stem of the test file, to check for overlaps later. 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() }; - tests.extend(make_test(config.clone(), cache, &paths, inputs, poisoned)) } else if file_path.is_dir() { + // Recurse to find more tests in a subdirectory. let relative_file_path = relative_dir_path.join(file.file_name()); if &file_name != "auxiliary" { debug!("found directory: {:?}", file_path.display()); @@ -728,6 +763,8 @@ pub fn is_test(file_name: &OsString) -> bool { !invalid_prefixes.iter().any(|p| file_name.starts_with(p)) } +/// For a single test file, creates one or more test structures (one per revision) +/// that can be handed over to libtest to run, possibly in parallel. fn make_test( config: Arc, cache: &HeadersCache, @@ -735,6 +772,9 @@ fn make_test( inputs: &Stamp, poisoned: &mut bool, ) -> Vec { + // For run-make tests, each "test file" is actually a _directory_ containing + // an `rmake.rs` or `Makefile`. But for the purposes of directive parsing, + // we want to look at that recipe file, not the directory itself. let test_path = if config.mode == Mode::RunMake { if testpaths.file.join("rmake.rs").exists() && testpaths.file.join("Makefile").exists() { panic!("run-make tests cannot have both `rmake.rs` and `Makefile`"); @@ -750,26 +790,40 @@ fn make_test( } else { PathBuf::from(&testpaths.file) }; + + // Scan the test file to discover its revisions, if any. let early_props = EarlyProps::from_file(&config, &test_path); - // Incremental tests are special, they inherently cannot be run in parallel. - // `runtest::run` will be responsible for iterating over revisions. + // Normally we create one libtest structure per revision, with two exceptions: + // - If a test doesn't use revisions, create a dummy revision (None) so that + // the test can still run. + // - Incremental tests inherently can't run their revisions in parallel, so + // we treat them like non-revisioned tests here. Incremental revisions are + // handled internally by `runtest::run` instead. let revisions = if early_props.revisions.is_empty() || config.mode == Mode::Incremental { vec![None] } else { early_props.revisions.iter().map(|r| Some(r.as_str())).collect() }; + // For each revision (or the sole dummy revision), create and return a + // `test::TestDescAndFn` that can be handed over to libtest. revisions .into_iter() .map(|revision| { + // Create a test name and description to hand over to libtest. let src_file = std::fs::File::open(&test_path).expect("open test file to parse ignores"); let test_name = crate::make_test_name(&config, testpaths, revision); + // Create a libtest description for the test/revision. + // This is where `ignore-*`/`only-*`/`needs-*` directives are handled, + // because they need to set the libtest ignored flag. let mut desc = make_test_description( &config, cache, test_name, &test_path, src_file, revision, poisoned, ); - // Ignore tests that already run and are up to date with respect to inputs. + + // If a test's inputs haven't changed since the last time it ran, + // mark it as ignored so that libtest will skip it. if !config.force_rerun && is_up_to_date(&config, testpaths, &early_props, revision, inputs) { @@ -777,18 +831,25 @@ fn make_test( // Keep this in sync with the "up-to-date" message detected by bootstrap. desc.ignore_message = Some("up-to-date"); } - test::TestDescAndFn { - desc, - testfn: make_test_closure(config.clone(), testpaths, revision), - } + + // Create the callback that will run this test/revision when libtest calls it. + let testfn = make_test_closure(config.clone(), testpaths, revision); + + test::TestDescAndFn { desc, testfn } }) .collect() } +/// The path of the `stamp` file that gets created or updated whenever a +/// particular test completes successfully. fn stamp(config: &Config, testpaths: &TestPaths, revision: Option<&str>) -> PathBuf { output_base_dir(config, testpaths, revision).join("stamp") } +/// Returns a list of files that, if modified, would cause this test to no +/// longer be up-to-date. +/// +/// (Might be inaccurate in some cases.) fn files_related_to_test( config: &Config, testpaths: &TestPaths, @@ -824,46 +885,61 @@ fn files_related_to_test( related } +/// Checks whether a particular test/revision is "up-to-date", meaning that no +/// relevant files/settings have changed since the last time the test succeeded. +/// +/// (This is not very reliable in some circumstances, so the `--force-rerun` +/// flag can be used to ignore up-to-date checking and always re-run tests.) fn is_up_to_date( config: &Config, testpaths: &TestPaths, props: &EarlyProps, revision: Option<&str>, - inputs: &Stamp, + inputs: &Stamp, // Last-modified timestamp of the compiler, compiletest etc ) -> bool { let stamp_name = stamp(config, testpaths, revision); - // Check hash. + // Check the config hash inside the stamp file. let contents = match fs::read_to_string(&stamp_name) { Ok(f) => f, Err(ref e) if e.kind() == ErrorKind::InvalidData => panic!("Can't read stamp contents"), + // The test hasn't succeeded yet, so it is not up-to-date. Err(_) => return false, }; let expected_hash = runtest::compute_stamp_hash(config); if contents != expected_hash { + // Some part of compiletest configuration has changed since the test + // last succeeded, so it is not up-to-date. return false; } - // Check timestamps. + // Check the timestamp of the stamp file against the last modified time + // of all files known to be relevant to the test. let mut inputs = inputs.clone(); for path in files_related_to_test(config, testpaths, props, revision) { inputs.add_path(&path); } + // If no relevant files have been modified since the stamp file was last + // written, the test is up-to-date. inputs < Stamp::from_path(&stamp_name) } +/// The maximum of a set of file-modified timestamps. #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] struct Stamp { time: SystemTime, } impl Stamp { + /// Creates a timestamp holding the last-modified time of the specified file. fn from_path(path: &Path) -> Self { let mut stamp = Stamp { time: SystemTime::UNIX_EPOCH }; stamp.add_path(path); stamp } + /// Updates this timestamp to the last-modified time of the specified file, + /// if it is later than the currently-stored timestamp. fn add_path(&mut self, path: &Path) { let modified = fs::metadata(path) .and_then(|metadata| metadata.modified()) @@ -871,6 +947,9 @@ impl Stamp { self.time = self.time.max(modified); } + /// Updates this timestamp to the most recent last-modified time of all files + /// recursively contained in the given directory, if it is later than the + /// currently-stored timestamp. fn add_dir(&mut self, path: &Path) { for entry in WalkDir::new(path) { let entry = entry.unwrap(); @@ -886,6 +965,7 @@ impl Stamp { } } +/// Creates a name for this test/revision that can be handed over to libtest. fn make_test_name( config: &Config, testpaths: &TestPaths, @@ -914,20 +994,41 @@ fn make_test_name( )) } +/// Creates a callback for this test/revision that libtest will call when it +/// decides to actually run the underlying test. fn make_test_closure( config: Arc, testpaths: &TestPaths, revision: Option<&str>, ) -> test::TestFn { - let config = config.clone(); let testpaths = testpaths.clone(); let revision = revision.map(str::to_owned); + + // This callback is the link between compiletest's test discovery code, + // and the parts of compiletest that know how to run an individual test. test::DynTestFn(Box::new(move || { runtest::run(config, &testpaths, revision.as_deref()); Ok(()) })) } +/// Checks that test discovery didn't find any tests whose name stem is a prefix +/// of some other tests's name. +/// +/// For example, suppose the test suite contains these two test files: +/// - `tests/rustdoc/primitive.rs` +/// - `tests/rustdoc/primitive/no_std.rs` +/// +/// The test runner might put the output from those tests in these directories: +/// - `$build/test/rustdoc/primitive/` +/// - `$build/test/rustdoc/primitive/no_std/` +/// +/// Because one output path is a subdirectory of the other, the two tests might +/// interfere with each other in unwanted ways, especially if the test runner +/// decides to delete test output directories to clean them between runs. +/// To avoid problems, we forbid test names from overlapping in this way. +/// +/// See for more context. fn check_overlapping_tests(found_paths: &HashSet) { let mut collisions = Vec::new(); for path in found_paths {