From 652ee63fffe1e825c6c2eb6d5d1de800d2241a74 Mon Sep 17 00:00:00 2001 From: Dylan McKay Date: Sun, 3 May 2020 19:59:33 +1200 Subject: [PATCH] Implement saving of test artifacts to a directory --- src/config.rs | 10 +- src/config/clap.rs | 2 +- src/event_handler/default.rs | 12 +-- src/model.rs | 2 +- src/parse.rs | 5 +- src/run/find_files.rs | 176 ++++++++++++++++++++++++++++++++++- src/run/mod.rs | 10 +- 7 files changed, 198 insertions(+), 19 deletions(-) diff --git a/src/config.rs b/src/config.rs index b7adfcb..81a834d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,7 +4,7 @@ #[cfg(feature = "clap")] pub mod clap; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::collections::HashMap; use std::fmt; use tempfile::NamedTempFile; @@ -82,6 +82,14 @@ impl Config self.test_paths.push(PathBuf::from(path.into())); } + /// Gets an iterator over all test search directories. + pub fn test_search_directories(&self) -> impl Iterator { + self.test_paths.iter().filter(|p| { + println!("test path file name: {:?}", p.file_name()); + p.is_dir() + }).map(PathBuf::as_ref) + } + /// Checks if a given extension will have tests run on it pub fn is_extension_supported(&self, extension: &str) -> bool { self.supported_file_extensions.iter(). diff --git a/src/config/clap.rs b/src/config/clap.rs index f497593..2220cbb 100644 --- a/src/config/clap.rs +++ b/src/config/clap.rs @@ -17,7 +17,7 @@ const SHOW_OPTION_VALUES: &'static [(&'static str, fn(&Config, &mut dyn Write) - ("test-file-paths", |config, writer| { let test_file_paths = crate::run::find_files::with_config(config).unwrap(); for test_file_path in test_file_paths { - writeln!(writer, "{}", test_file_path)?; + writeln!(writer, "{}", test_file_path.absolute.display())?; } Ok(()) diff --git a/src/event_handler/default.rs b/src/event_handler/default.rs index 7ead708..59f822d 100644 --- a/src/event_handler/default.rs +++ b/src/event_handler/default.rs @@ -80,22 +80,22 @@ impl super::EventHandler for EventHandler { pub fn result(result: &TestResult, verbose: bool, config: &Config) { match result.overall_result { TestResultKind::Pass => { - print::success(format!("PASS :: {}", result.path.display())); + print::success(format!("PASS :: {}", result.path.relative.display())); }, TestResultKind::UnexpectedPass => { - print::failure(format!("UNEXPECTED PASS :: {}", result.path.display())); + print::failure(format!("UNEXPECTED PASS :: {}", result.path.relative.display())); }, TestResultKind::Skip => { print::line(); print::warning(format!( "SKIP :: {} (test does not contain any test commands, perhaps you meant to add a 'CHECK'?)", - result.path.display())); + result.path.relative.display())); print::line(); }, TestResultKind::Error { ref message } => { if verbose { print::line(); } - print::error(format!("ERROR :: {}", result.path.display())); + print::error(format!("ERROR :: {}", result.path.relative.display())); if verbose { print::textln(message); @@ -106,7 +106,7 @@ pub fn result(result: &TestResult, verbose: bool, config: &Config) { TestResultKind::Fail { ref reason, ref hint } => { if verbose { print::line(); } - print::failure(format!("FAIL :: {}", result.path.display())); + print::failure(format!("FAIL :: {}", result.path.relative.display())); // FIXME: improve formatting @@ -125,7 +125,7 @@ pub fn result(result: &TestResult, verbose: bool, config: &Config) { } }, TestResultKind::ExpectedFailure => { - print::warning(format!("XFAIL :: {}", result.path.display())); + print::warning(format!("XFAIL :: {}", result.path.relative.display())); }, } diff --git a/src/model.rs b/src/model.rs index fdbc42e..ba6a852 100644 --- a/src/model.rs +++ b/src/model.rs @@ -159,7 +159,7 @@ pub struct CheckFailureInfo { pub struct TestResult { /// A path to the test. - pub path: PathBuf, + pub path: TestFilePath, /// The kind of result. pub overall_result: TestResultKind, pub individual_run_results: Vec<(TestResultKind, Invocation, run::CommandLine, ProgramOutput)>, diff --git a/src/parse.rs b/src/parse.rs index a95b968..7772f6b 100644 --- a/src/parse.rs +++ b/src/parse.rs @@ -2,7 +2,6 @@ use crate::model::*; use regex::Regex; use std::mem; -use std::path::Path; lazy_static! { static ref DIRECTIVE_REGEX: Regex = Regex::new("([A-Z-]+):(.*)").unwrap(); @@ -10,8 +9,8 @@ lazy_static! { } /// Parses a test file -pub fn test_file(path: TestFilePath, chars: I) -> Result - where P: AsRef, I: Iterator { +pub fn test_file(path: TestFilePath, chars: I) -> Result + where I: Iterator { let mut commands = Vec::new(); let test_body: String = chars.collect(); diff --git a/src/run/find_files.rs b/src/run/find_files.rs index 42546d9..202a698 100644 --- a/src/run/find_files.rs +++ b/src/run/find_files.rs @@ -3,7 +3,7 @@ use crate::{Config, model::TestFilePath}; use std; -use std::path::Path; +use std::path::{Path, PathBuf}; use walkdir::WalkDir; /// Recursively finds tests for the given paths. @@ -18,7 +18,8 @@ pub fn with_config(config: &Config) -> Result, String> { } let test_paths = absolute_paths.into_iter().map(|absolute_path| { - let relative_path = // TODO: find most specific path in search tree. failing that, find most common path from all test paths and use that. otherwise use a random one like 'test'. maybe rename 'relative_path' to 'relative_path_for_display'. + let relative_path = relative_path::compute(&absolute_path, config).expect("could not compute relative path"); + TestFilePath { absolute: absolute_path, relative: relative_path } }).collect(); @@ -41,6 +42,177 @@ pub fn in_path(path: &str, } } +mod relative_path { + use crate::Config; + use std::path::{Path, PathBuf}; + + pub fn compute(test_absolute_path: &Path, config: &Config) + -> Option { + let mut take_path_relative_to_dir = None; + + if take_path_relative_to_dir.is_none() { + if let Some(least_specific_parent_test_search_directory_path) = + least_specific_parent_test_search_directory_path(test_absolute_path, config) { + take_path_relative_to_dir = Some(least_specific_parent_test_search_directory_path); + } + } + + if take_path_relative_to_dir.is_none() { + if let Some(most_common_test_path_ancestor) = + most_common_test_path_ancestor(test_absolute_path, config) { + take_path_relative_to_dir = Some(most_common_test_path_ancestor); + } + } + + take_path_relative_to_dir.map(|relative_to| { + test_absolute_path.strip_prefix(relative_to).expect("relative path computation failed: not a prefix").to_owned() + }) + } + + /// Attempt to find the most specific prefix directory from the test search paths in the config. + fn least_specific_parent_test_search_directory_path(test_absolute_path: &Path, config: &Config) + -> Option { + // N.B. we iterate over the test paths here. We don't check for the directory's actual + // existence on the filesystem. This makes testing easier, but also: test paths can only + // be strict prefixes/supersets of other test paths if they ARE directories. + let matching_parent_test_search_directories = config.test_paths.iter() + .filter(|possible_dir_path| test_absolute_path.starts_with(possible_dir_path)); + + let least_specific_matching_test_search_directory = matching_parent_test_search_directories.min_by_key(|p| p.components().count()); + + if let Some(least_specific_matching_test_search_directory) = least_specific_matching_test_search_directory { + Some(least_specific_matching_test_search_directory.to_owned()) + } else { + None + } + } + + /// Otherwise, find the most common path from all the test file paths. + /// + /// NOTE: this will return `None` in several cases, such as if there is only one test path, + /// or On windows in the case where there are tests located on several different device drives. + fn most_common_test_path_ancestor(test_absolute_path: &Path, config: &Config) + -> Option { + // different disk drives at the same time. + { + let initial_current_path_containing_everything_so_far = test_absolute_path.parent().unwrap(); + let mut current_path_containing_everything_so_far = initial_current_path_containing_everything_so_far; + + for test_path in config.test_paths.iter() { + if !test_path.starts_with(current_path_containing_everything_so_far) { + let common_ancestor = test_path.ancestors().find(|p| current_path_containing_everything_so_far.starts_with(p)); + + if let Some(common_ancestor) = common_ancestor { + // The common ancestor path may be empty if the files are on different + // devices. + if common_ancestor.file_name().is_some() { + println!("common ancestor: {:?}", common_ancestor.file_name()); + current_path_containing_everything_so_far = common_ancestor; + } + } else { + // N.B. we only ever expect no common ancestor on Windows + // where paths may be on different devices. This should be uncommon. + // We cannot use this logic to compute the relative path in this scenario. + } + } + } + + if current_path_containing_everything_so_far != initial_current_path_containing_everything_so_far { + Some(current_path_containing_everything_so_far.to_owned()) + } else { + None // no common prefix path could be calculated from the test paths + } + + } + } + + #[cfg(test)] + mod test { + use crate::Config; + use std::path::Path; + + #[test] + fn test_compute() { + let config = Config { + test_paths: [ + "/home/foo/projects/cool-project/tests/", + "/home/foo/projects/cool-project/tests/run-pass/", + "/home/foo/projects/cool-project/tests/run-fail/", + ].iter().map(|p| Path::new(p).to_owned()).collect(), + ..Config::default() + }; + + assert_eq!(super::compute( + &Path::new("/home/foo/projects/cool-project/tests/run-pass/test1.txt"), &config), + Some(Path::new("run-pass/test1.txt").to_owned())); + } + + #[test] + fn test_least_specific_parent_test_search_directory_path_when_all_test_paths_are_directories() { + let config = Config { + test_paths: [ + "/home/foo/projects/cool-project/tests/", + "/home/foo/projects/cool-project/tests/run-pass/", + "/home/foo/projects/cool-project/tests/run-fail/", + ].iter().map(|p| Path::new(p).to_owned()).collect(), + ..Config::default() + }; + + assert_eq!(super::least_specific_parent_test_search_directory_path( + &Path::new("/home/foo/projects/cool-project/tests/run-pass/test1.txt"), &config), + Some(Path::new("/home/foo/projects/cool-project/tests/").to_owned())); + } + + #[test] + fn test_least_specific_parent_test_search_directory_path_when_one_test_path_directory() { + let config = Config { + test_paths: [ + "/home/foo/projects/cool-project/tests/", + ].iter().map(|p| Path::new(p).to_owned()).collect(), + ..Config::default() + }; + + assert_eq!(super::least_specific_parent_test_search_directory_path( + &Path::new("/home/foo/projects/cool-project/tests/run-pass/test1.txt"), &config), + Some(Path::new("/home/foo/projects/cool-project/tests/").to_owned())); + } + + #[test] + fn test_most_common_test_path_ancestor_when_all_paths_are_absolute() { + let config = Config { + test_paths: [ + "/home/foo/projects/cool-project/tests/run-pass/test1.txt", + "/home/foo/projects/cool-project/tests/run-pass/test2.txt", + "/home/foo/projects/cool-project/tests/run-fail/test3.txt", + ].iter().map(|p| Path::new(p).to_owned()).collect(), + ..Config::default() + }; + + assert_eq!(super::most_common_test_path_ancestor( + &Path::new("/home/foo/projects/cool-project/tests/run-pass/test1.txt"), &config), + Some(Path::new("/home/foo/projects/cool-project/tests").to_owned())); + } + + + #[test] + fn test_most_common_test_path_ancestor_when_all_paths_absolute_on_different_drives() { + let config = Config { + test_paths: [ + "C:/tests/run-pass/test1.txt", + "C:/tests/run-pass/test2.txt", + "Z:/tests/run-fail/test3.txt", + "Z:/tests/run-fail/test4.txt", + ].iter().map(|p| Path::new(p).to_owned()).collect(), + ..Config::default() + }; + + assert_eq!(super::most_common_test_path_ancestor( + &Path::new("C:/tests/run-pass/test2.txt"), &config), + None); + } + } +} + fn tests_in_dir(path: &str, config: &Config) -> Result,String> { let tests = files_in_dir(path)?.into_iter() diff --git a/src/run/mod.rs b/src/run/mod.rs index 5fdaa1b..491c93e 100644 --- a/src/run/mod.rs +++ b/src/run/mod.rs @@ -52,7 +52,7 @@ pub fn tests( let mut has_failure = false; for test_file_path in test_paths { - let test_file = util::parse_test(&test_file_path).unwrap(); + let test_file = util::parse_test(test_file_path).unwrap(); let is_successful = self::single_file(&test_file, &mut event_handler, &config, &artifact_config); if !is_successful { has_failure = true; } @@ -160,14 +160,14 @@ mod save_artifacts { pub fn individual_run_result(run_number: usize, result_kind: &TestResultKind, command_line: &CommandLine, output: &ProgramOutput, test_file: &TestFile, config: &Config) { let dir_test_file = format!("run-command-{}", run_number); - let dir_run_result = test_file.relative_path.join(format!("run-command-{}", run_number)); + let dir_run_result = test_file.path.relative.join(format!("run-command-{}", run_number)); save(&dir_run_result.join("result.txt"), config, || { - format!("{:?}\n", result_kind) + format!("{:#?}\n", result_kind) }); - save(&dir_run_result.join("stdout.txt"), config, || output.stdout); - save(&dir_run_result.join("stderr.txt"), config, || output.stderr); + save(&dir_run_result.join("stdout.txt"), config, || &output.stdout[..]); + save(&dir_run_result.join("stderr.txt"), config, || &output.stderr[..]); } fn save(relative_path: &Path, config: &Config, render: impl FnOnce() -> C )