From 2a3defd922db87adaffeb2a57db8796ff9ff22d2 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Tue, 26 Jan 2021 18:29:01 +0100 Subject: [PATCH 1/3] Add basic `bless` support --- src/common.rs | 23 ++++++++++ src/runtest.rs | 115 +++++++++++++++++++++++++++++++++++-------------- src/util.rs | 22 ++++++++++ 3 files changed, 127 insertions(+), 33 deletions(-) diff --git a/src/common.rs b/src/common.rs index 7e3bc73..ed0a19d 100644 --- a/src/common.rs +++ b/src/common.rs @@ -102,6 +102,9 @@ impl fmt::Display for Mode { #[derive(Clone)] pub struct Config { + /// `true` to overwrite stderr/stdout/fixed files instead of complaining about changes in output. + pub bless: bool, + /// The library paths required for running the compiler pub compile_lib_path: PathBuf, @@ -239,6 +242,25 @@ pub struct TestPaths { pub relative_dir: PathBuf, // e.g., foo/bar } +/// Used by `ui` tests to generate things like `foo.stderr` from `foo.rs`. +pub fn expected_output_path( + testpaths: &TestPaths, + revision: Option<&str>, + kind: &str, +) -> PathBuf { + assert!(UI_EXTENSIONS.contains(&kind)); + let mut parts = Vec::new(); + + if let Some(x) = revision { + parts.push(x); + } + parts.push(kind); + + let extension = parts.join("."); + testpaths.file.with_extension(extension) +} + +pub const UI_EXTENSIONS: &[&str] = &[UI_STDERR, UI_STDOUT, UI_FIXED]; pub const UI_STDERR: &str = "stderr"; pub const UI_STDOUT: &str = "stdout"; pub const UI_FIXED: &str = "fixed"; @@ -335,6 +357,7 @@ impl Default for Config { let platform = rustc_session::config::host_triple().to_string(); Config { + bless: false, compile_lib_path: PathBuf::from(""), run_lib_path: PathBuf::from(""), rustc_path: PathBuf::from("rustc"), diff --git a/src/runtest.rs b/src/runtest.rs index 8ed7cb6..6ceb833 100644 --- a/src/runtest.rs +++ b/src/runtest.rs @@ -9,7 +9,7 @@ // except according to those terms. use common::{Config, TestPaths}; -use common::{UI_FIXED, UI_STDERR, UI_STDOUT}; +use common::{expected_output_path, UI_FIXED, UI_STDERR, UI_STDOUT}; use common::{CompileFail, ParseFail, Pretty, RunFail, RunPass, RunPassValgrind}; use common::{Codegen, DebugInfoLldb, DebugInfoGdb, Rustdoc, CodegenUnits}; use common::{Incremental, RunMake, Ui, MirOpt}; @@ -20,7 +20,7 @@ use json; use regex::Regex; use rustfix::{apply_suggestions, get_suggestions_from_json, Filter}; use header::TestProps; -use util::logv; +use crate::util::{logv, PathBufExt}; use std::collections::HashMap; use std::collections::HashSet; @@ -2167,6 +2167,18 @@ actual:\n\ // compiler flags set in the test cases: cmd.env_remove("RUSTFLAGS"); + if self.config.bless { + cmd.env("RUSTC_BLESS_TEST", "--bless"); + // Assume this option is active if the environment variable is "defined", with _any_ value. + // As an example, a `Makefile` can use this option by: + // + // ifdef RUSTC_BLESS_TEST + // cp "$(TMPDIR)"/actual_something.ext expected_something.ext + // else + // $(DIFF) expected_something.ext "$(TMPDIR)"/actual_something.ext + // endif + } + if self.config.target.contains("msvc") { // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe` // and that `lib.exe` lives next to it. @@ -2323,16 +2335,17 @@ actual:\n\ } if errors > 0 { - println!("To update references, run this command from build directory:"); + println!("To update references, rerun the tests and pass the `--bless` flag"); let relative_path_to_file = - self.testpaths.relative_dir - .join(self.testpaths.file.file_name().unwrap()); - println!("{}/update-references.sh '{}' '{}'", - self.config.src_base.display(), - self.config.build_base.display(), - relative_path_to_file.display()); - self.fatal_proc_rec(&format!("{} errors occurred comparing output.", errors), - &proc_res); + self.testpaths.relative_dir.join(self.testpaths.file.file_name().unwrap()); + println!( + "To only update this specific test, also pass `--test-args {}`", + relative_path_to_file.display(), + ); + self.fatal_proc_rec( + &format!("{} errors occurred comparing output.", errors), + &proc_res, + ); } if self.props.run_pass { @@ -2566,11 +2579,14 @@ actual:\n\ } fn expected_output_path(&self, kind: &str) -> PathBuf { - let extension = match self.revision { - Some(r) => format!("{}.{}", r, kind), - None => kind.to_string(), - }; - self.testpaths.file.with_extension(extension) + let mut path = + expected_output_path(&self.testpaths, self.revision, kind); + + if !path.exists() { + path = expected_output_path(&self.testpaths, self.revision, kind); + } + + path } fn load_expected_output(&self, path: &Path) -> String { @@ -2594,35 +2610,68 @@ actual:\n\ }) } + fn delete_file(&self, file: &PathBuf) { + if !file.exists() { + // Deleting a nonexistant file would error. + return; + } + if let Err(e) = fs::remove_file(file) { + self.fatal(&format!("failed to delete `{}`: {}", file.display(), e,)); + } + } + fn compare_output(&self, kind: &str, actual: &str, expected: &str) -> usize { if actual == expected { return 0; } - println!("normalized {}:\n{}\n", kind, actual); - println!("expected {}:\n{}\n", kind, expected); - println!("diff of {}:\n", kind); - - for diff in diff::lines(expected, actual) { - match diff { - diff::Result::Left(l) => println!("-{}", l), - diff::Result::Both(l, _) => println!(" {}", l), - diff::Result::Right(r) => println!("+{}", r), + if !self.config.bless { + if expected.is_empty() { + println!("normalized {}:\n{}\n", kind, actual); + } else { + println!("diff of {}:\n", kind); + for diff in diff::lines(expected, actual) { + match diff { + diff::Result::Left(l) => println!("-{}", l), + diff::Result::Both(l, _) => println!(" {}", l), + diff::Result::Right(r) => println!("+{}", r), + } + } } } - let output_file = self.output_base_name().with_extension(kind); - match File::create(&output_file).and_then(|mut f| f.write_all(actual.as_bytes())) { - Ok(()) => { } - Err(e) => { - self.fatal(&format!("failed to write {} to `{}`: {}", - kind, output_file.display(), e)) + let output_file = self + .output_base_name() + .with_extra_extension(self.revision.unwrap_or("")) + .with_extra_extension(kind); + + let mut files = vec![output_file]; + if self.config.bless { + files.push(expected_output_path( + self.testpaths, + self.revision, + kind, + )); + } + + for output_file in &files { + if actual.is_empty() { + self.delete_file(output_file); + } else if let Err(err) = fs::write(&output_file, &actual) { + self.fatal(&format!( + "failed to write {} to `{}`: {}", + kind, + output_file.display(), + err, + )); } } println!("\nThe actual {0} differed from the expected {0}.", kind); - println!("Actual {} saved to {}", kind, output_file.display()); - 1 + for output_file in files { + println!("Actual {} saved to {}", kind, output_file.display()); + } + if self.config.bless { 0 } else { 1 } } } diff --git a/src/util.rs b/src/util.rs index c00f28e..0eb9d42 100644 --- a/src/util.rs +++ b/src/util.rs @@ -10,6 +10,8 @@ use std::env; use common::Config; +use std::ffi::OsStr; +use std::path::PathBuf; /// Conversion table from triple OS name to Rust SYSNAME const OS_TABLE: &'static [(&'static str, &'static str)] = &[ @@ -108,3 +110,23 @@ pub fn logv(config: &Config, s: String) { println!("{}", s); } } + +pub trait PathBufExt { + /// Append an extension to the path, even if it already has one. + fn with_extra_extension>(&self, extension: S) -> PathBuf; +} + +impl PathBufExt for PathBuf { + fn with_extra_extension>(&self, extension: S) -> PathBuf { + if extension.as_ref().is_empty() { + self.clone() + } else { + let mut fname = self.file_name().unwrap().to_os_string(); + if !extension.as_ref().to_str().unwrap().starts_with('.') { + fname.push("."); + } + fname.push(extension); + self.with_file_name(fname) + } + } +} From 32c51259be9be405ba2aa176fca22a84c31793ea Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Sat, 30 Jan 2021 11:13:11 +0100 Subject: [PATCH 2/3] Add a cargo-inspired testsuite --- Cargo.toml | 1 + tests/bless.rs | 85 ++++++++++++++++++++++++++++++ tests/test_support/mod.rs | 105 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+) create mode 100644 tests/bless.rs create mode 100644 tests/test_support/mod.rs diff --git a/Cargo.toml b/Cargo.toml index cf84500..cab3437 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,6 +26,7 @@ serde_json = "1.0" serde_derive = "1.0" rustfix = "0.5" tester = "0.8" +lazy_static = "1.4" [target."cfg(unix)".dependencies] libc = "0.2" diff --git a/tests/bless.rs b/tests/bless.rs new file mode 100644 index 0000000..9ee26f4 --- /dev/null +++ b/tests/bless.rs @@ -0,0 +1,85 @@ +//! Tests for the `bless` option + +extern crate compiletest_rs as compiletest; + +mod test_support; +use test_support::{testsuite, TestsuiteBuilder, GLOBAL_ROOT}; +use compiletest::Config; + +fn setup(mode: &str) -> (Config, TestsuiteBuilder) { + let builder = testsuite(mode); + let mut config = Config::default(); + let cfg_mode = mode.parse().expect("Invalid mode"); + config.mode = cfg_mode; + config.src_base = builder.root.clone(); + config.build_base = GLOBAL_ROOT.join("build_base"); + + (config, builder) +} + +#[test] +fn test_bless_new_file() { + let (mut config, builder) = setup("ui"); + config.bless = true; + + builder.mk_file( + "foobar.rs", + r#" + #[warn(unused_variables)] + fn main() { + let abc = "foobar"; + } + "#, + ); + compiletest::run_tests(&config); + + // Blessing should cause the stderr to be created directly + assert!(builder.file_contents("foobar.stderr").contains("unused variable")); + + // And a second run of the tests, with blessing disabled should work just fine + config.bless = false; + compiletest::run_tests(&config); +} + +#[test] +fn test_bless_update_file() { + let (mut config, builder) = setup("ui"); + config.bless = true; + + builder.mk_file( + "foobar2.rs", + r#" + #[warn(unused_variables)] + fn main() { + let abc = "foobar_update"; + } + "#, + ); + builder.mk_file( + "foobar2.stderr", + r#" + warning: unused variable: `abc` + --> $DIR/foobar2.rs:4:27 + | + 4 | let abc = "foobar"; + | ^^^ help: if this is intentional, prefix it with an underscore: `_abc` + | + note: the lint level is defined here + --> $DIR/foobar2.rs:2:26 + | + 2 | #[warn(unused_variables)] + | ^^^^^^^^^^^^^^^^ + + warning: 1 warning emitted + "#, + ); + compiletest::run_tests(&config); + + // Blessing should cause the stderr to be created directly + assert!(builder.file_contents("foobar2.stderr").contains("unused variable")); + assert!(builder.file_contents("foobar2.stderr").contains("foobar_update")); + + // And a second run of the tests, with blessing disabled should work just fine + config.bless = false; + compiletest::run_tests(&config); +} diff --git a/tests/test_support/mod.rs b/tests/test_support/mod.rs new file mode 100644 index 0000000..c6962af --- /dev/null +++ b/tests/test_support/mod.rs @@ -0,0 +1,105 @@ +//! Provides a simple way to set up compiletest sample testsuites used in testing. +//! +//! Inspired by cargo's `cargo-test-support` crate: +//! https://github.com/rust-lang/cargo/tree/master/crates/cargo-test-support +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::cell::RefCell; +use std::sync::atomic::{AtomicUsize, Ordering}; + + +static COMPILETEST_INTEGRATION_TEST_DIR: &str = "cit"; + +thread_local! { + static TEST_ID: RefCell> = RefCell::new(None); +} + +lazy_static::lazy_static! { + pub static ref GLOBAL_ROOT: PathBuf = { + let mut path = env::current_exe().unwrap(); + path.pop(); // chop off exe name + path.pop(); // chop off 'deps' part + path.pop(); // chop off 'debug' + + path.push(COMPILETEST_INTEGRATION_TEST_DIR); + path.mkdir_p(); + path + }; +} + +pub fn testsuite(mode: &str) -> TestsuiteBuilder { + let builder = TestsuiteBuilder::new(mode); + builder.build(); + builder +} + +pub struct TestsuiteBuilder { + pub root: PathBuf, +} + +impl TestsuiteBuilder { + pub fn new(mode: &str) -> Self { + static NEXT_ID: AtomicUsize = AtomicUsize::new(0); + + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + TEST_ID.with(|n| *n.borrow_mut() = Some(id)); + let root = GLOBAL_ROOT.join(format!("id{}", TEST_ID.with(|n|n.borrow().unwrap()))).join(mode); + root.mkdir_p(); + + Self { + root, + } + } + + + /// Creates a new file to be used for the integration test + pub fn mk_file(&self, path: &str, body: &str) { + self.root.mkdir_p(); + fs::write(self.root.join(&path), &body) + .unwrap_or_else(|e| panic!("could not create file {}: {}", path, e)); + } + + /// Returns the contents of the file + pub fn file_contents(&self, name: &str) -> String { + fs::read_to_string(self.root.join(name)).expect("Unable to read file") + } + + // Sets up a new testsuite root directory + fn build(&self) { + // Cleanup before we run the next test + self.rm_root(); + + // Create the new directory + self.root.mkdir_p(); + } + + /// Deletes the root directory and all its contents + fn rm_root(&self) { + self.root.rm_rf(); + } +} + +pub trait PathExt { + fn rm_rf(&self); + fn mkdir_p(&self); +} + +impl PathExt for Path { + fn rm_rf(&self) { + if self.is_dir() { + if let Err(e) = fs::remove_dir_all(self) { + panic!("failed to remove {:?}: {:?}", self, e) + } + } else { + if let Err(e) = fs::remove_file(self) { + panic!("failed to remove {:?}: {:?}", self, e) + } + } + } + + fn mkdir_p(&self) { + fs::create_dir_all(self) + .unwrap_or_else(|e| panic!("failed to mkdir_p {}: {}", self.display(), e)) + } +} From c3096be3c7feb989897c1a0ef8bdf08e6f2f7174 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 10 Feb 2021 15:00:41 +0100 Subject: [PATCH 3/3] Update to tester 0.9 This is a breaking change and allows for setting multiple filters at once. --- Cargo.toml | 2 +- src/common.rs | 6 +++--- src/lib.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cab3437..c23d87e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ serde = "1.0" serde_json = "1.0" serde_derive = "1.0" rustfix = "0.5" -tester = "0.8" +tester = "0.9" lazy_static = "1.4" [target."cfg(unix)".dependencies] diff --git a/src/common.rs b/src/common.rs index ed0a19d..98d4655 100644 --- a/src/common.rs +++ b/src/common.rs @@ -148,8 +148,8 @@ pub struct Config { /// Run ignored tests pub run_ignored: bool, - /// Only run tests that match this filter - pub filter: Option, + /// Only run tests that match these filters + pub filters: Vec, /// Exactly match the filter, rather than a substring pub filter_exact: bool, @@ -372,7 +372,7 @@ impl Default for Config { stage_id: "stage-id".to_owned(), mode: Mode::RunPass, run_ignored: false, - filter: None, + filters: vec![], filter_exact: false, logfile: None, runtool: None, diff --git a/src/lib.rs b/src/lib.rs index 0099706..d0f4875 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,7 +111,7 @@ pub fn run_tests(config: &Config) { pub fn test_opts(config: &Config) -> test::TestOpts { test::TestOpts { - filter: config.filter.clone(), + filters: config.filters.clone(), filter_exact: config.filter_exact, exclude_should_panic: false, force_run_in_process: false,