From 7bd1ec404ff28377f2bfd2652fa86edea997f7e0 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 15 May 2020 12:41:46 -0400 Subject: [PATCH 1/4] Revert "Include compiler crashes in ICE defintion" This reverts commit a3891cdd26d1c5d35257c351c7c86fa7e72604bb. --- src/main.rs | 79 +++++++++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/main.rs b/src/main.rs index a00932c..9fb0a72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -240,33 +240,39 @@ impl Config { status, stdout_utf8, stderr_utf8 ); - // for commit message: - // no text but 101 https://github.com/rust-lang/rust/issues/21599 - // no text, signal https://github.com/rust-lang/rust/issues/13368 + let saw_ice = || -> bool { stderr_utf8.contains("error: internal compiler error") }; - const SUCCESS: Option = Some(0); - const ICE: Option = Some(101); - - let input = (self.output_processing_mode(), status.code()); + let input = (self.output_processing_mode(), status.success()); let result = match input { - (OutputProcessingMode::RegressOnErrorStatus, SUCCESS) => TestOutcome::Baseline, - (OutputProcessingMode::RegressOnErrorStatus, _) => TestOutcome::Regressed, - - (OutputProcessingMode::RegressOnSuccessStatus, SUCCESS) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnSuccessStatus, _) => TestOutcome::Baseline, + (OutputProcessingMode::RegressOnErrorStatus, true) => TestOutcome::Baseline, + (OutputProcessingMode::RegressOnErrorStatus, false) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnIceAlone, ICE) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnIceAlone, None) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnIceAlone, _) => TestOutcome::Baseline, + (OutputProcessingMode::RegressOnSuccessStatus, true) => TestOutcome::Regressed, + (OutputProcessingMode::RegressOnSuccessStatus, false) => TestOutcome::Baseline, - (OutputProcessingMode::RegressOnNotIce, ICE) => TestOutcome::Baseline, - (OutputProcessingMode::RegressOnNotIce, None) => TestOutcome::Baseline, - (OutputProcessingMode::RegressOnNotIce, _) => TestOutcome::Regressed, + (OutputProcessingMode::RegressOnIceAlone, _) => { + if saw_ice() { + TestOutcome::Regressed + } else { + TestOutcome::Baseline + } + } + (OutputProcessingMode::RegressOnNotIce, _) => { + if saw_ice() { + TestOutcome::Baseline + } else { + TestOutcome::Regressed + } + } - (OutputProcessingMode::RegressOnNonCleanError, SUCCESS) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnNonCleanError, ICE) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnNonCleanError, None) => TestOutcome::Regressed, - (OutputProcessingMode::RegressOnNonCleanError, _) => TestOutcome::Baseline, + (OutputProcessingMode::RegressOnNonCleanError, true) => TestOutcome::Regressed, + (OutputProcessingMode::RegressOnNonCleanError, false) => { + if saw_ice() { + TestOutcome::Regressed + } else { + TestOutcome::Baseline + } + } }; debug!( "default_outcome_of_output: input: {:?} result: {:?}", @@ -311,27 +317,27 @@ enum OutputProcessingMode { RegressOnSuccessStatus, /// `RegressOnIceAlone`: Marks test outcome as `Regressed` if and only if - /// the `rustc` process crashes or reports an interal compiler error (ICE) - /// has occurred. This covers the use case for when you want to bisect to - /// see when an ICE was introduced pon a codebase that is meant to produce a - /// clean error. + /// the `rustc` process issues a diagnostic indicating that an internal + /// compiler error (ICE) occurred. This covers the use case for when you + /// want to bisect to see when an ICE was introduced pon a codebase that is + /// meant to produce a clean error. /// /// You explicitly opt into this seting via `--regress=ice`. RegressOnIceAlone, /// `RegressOnNotIce`: Marks test outcome as `Regressed` if and only if - /// the `rustc` process does not crash or report that an internal compiler - /// error (ICE) has occurred. This covers the use case for when you want to - /// bisect to see when an ICE was fixed. + /// the `rustc` process does not issue a diagnostic indicating that an + /// internal compiler error (ICE) occurred. This covers the use case for + /// when you want to bisect to see when an ICE was fixed. /// /// You explicitly opt into this setting via `--regress=non-ice` RegressOnNotIce, /// `RegressOnNonCleanError`: Marks test outcome as `Baseline` if and only - /// if the `rustc` process reports error status that is not an internal - /// compiler error (ICE). This is the use case if the regression is a case - /// where an ill-formed program has stopped being properly rejected by the - /// compiler. + /// if the `rustc` process reports error status and does not issue any + /// diagnostic indicating that an internal compiler error (ICE) occurred. + /// This is the use case if the regression is a case where an ill-formed + /// program has stopped being properly rejected by the compiler. /// /// (The main difference between this case and `RegressOnSuccessStatus` is /// the handling of ICE: `RegressOnSuccessStatus` assumes that ICE should be @@ -346,10 +352,11 @@ impl OutputProcessingMode { fn must_process_stderr(&self) -> bool { match self { OutputProcessingMode::RegressOnErrorStatus - | OutputProcessingMode::RegressOnSuccessStatus - | OutputProcessingMode::RegressOnNonCleanError + | OutputProcessingMode::RegressOnSuccessStatus => false, + + OutputProcessingMode::RegressOnNonCleanError | OutputProcessingMode::RegressOnIceAlone - | OutputProcessingMode::RegressOnNotIce => false, + | OutputProcessingMode::RegressOnNotIce => true, } } } From 82e0b7908e7849c143a946049ceecd9d587366ee Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 4 Mar 2020 16:31:57 -0500 Subject: [PATCH 2/4] Test infra needs tempfile crate. --- Cargo.lock | 1 + Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 1e8c50f..dea958c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,6 +148,7 @@ dependencies = [ "tar 0.4.26 (registry+https://github.com/rust-lang/crates.io-index)", "tee 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", + "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "xz2 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/Cargo.toml b/Cargo.toml index 0e0025f..e4d5024 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,3 +35,4 @@ colored="1.9" [dev-dependencies] quickcheck = "0.9.2" +tempfile = "3.1.0" From 11923382f6be9b6e2d72adeee523f80ef6263872 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 4 Mar 2020 16:31:45 -0500 Subject: [PATCH 3/4] Infrastructure for testing the command line tool. Note: generates tests into fresh temp dir, which is deleted after testing is done (regardless of success or failure). You can change the `which_temp::WhichTempDir` type to revise this behavior. This infrastructure includes two tests: `tests/cli.rs` and `tests/ice.rs`. Each uses very different strategies for testing cargo-bisect-rustc. 1. `tests/cli.rs` uses a so-called meta-build strategy: the test inspects the `rustc` version, then generates build files that will inject (or remove, e.g. when testing `--regress=success`) `#[rustc_error]` from the source code based on the `rustc` version. This way, we get the effect of an error that will come or go based solely on the `rustc` version, without any dependence on the actual behavior of `rustc` itself (beyond its version string format remaining parsable). * This strategy should remain usable for the foreseeable future, without any need for intervention from `cargo-bisect-rustc` developers. 2. `tests/ice.rs` uses a totally different strategy: It embeds an ICE that we know originated at a certain version of the compiler. The ICE is embedded in the file `src/ice/included_main.rs`. The injection point associated with the ICE is encoded in the constant `INJECTION_COMMIT`. * Over time, since we only keep a certain number of builds associated with PR merge commits available to download, the embedded ICE, the `INJECTION_COMMIT` definition, and the search bounds defined in `INJECTION_LOWER_BOUND` and `INJECTION_UPPER_BOUND` will all have to be updated as soon as the commit for `INJECTION_COMMIT` is no longer available for download. * Thus, this testing strategy requires regular maintenance from the `cargo-bisect-rustc` developers. (However, it is more flexible than the meta-build strategy, in that you can embed arbitrary failures from the recent past using this approach. The meta-build approach can only embed things that can be expressed via features like `#[rustc_error]`, which cannot currently express ICE's. ---- Includes suggestions from code review Co-authored-by: bjorn3 ---- Includes some coments explaining the `WhichTempDir` type. (That type maybe should just be an enum rather than a trait you implement... not sure why I made it so general...) ---- Includes workaround for rustfmt issue. Specifically, workaround https://github.com/rust-lang/rustfmt/issues/3794 which was causing CI's attempt to run `cargo fmt -- --check` to erroneously report: ``` % cargo fmt -- --check error[E0583]: file not found for module `meta_build` --> /private/tmp/cbr/tests/cli.rs:11:20 | 11 | pub(crate) mod meta_build; | ^^^^^^^^^^ | = help: name the file either meta_build.rs or meta_build/mod.rs inside the directory "/private/tmp/cbr/tests/cli/cli" error[E0583]: file not found for module `command_invocation` --> /private/tmp/cbr/tests/ice.rs:34:20 | 34 | pub(crate) mod command_invocation; | ^^^^^^^^^^^^^^^^^^ | = help: name the file either command_invocation.rs or command_invocation/mod.rs inside the directory "/private/tmp/cbr/tests/ice/common" ``` ---- Includes fix for oversight in my cli test system: it needed to lookup target binary, not our PATH. (This functionality is also available via other means, such as `$CARGO_BIN_EXE_` and https://crates.io/crates/assert_cmd. I opted not to use the builtin env variable because that is only available in very recent cargo versions, and I would prefer our test suite to work peven on older versions of cargo, if that is feasible...) ---- Includes applications of rustfmt suggestions, as well as an expansion of a comment in a manner compatible with rustfmt. (Namely, that replaced an inline comment which is erroneously deleted by rustfmt (see https://github.com/rust-lang/rustfmt/issues/2781 ) with an additional note in the comment above the definition.) --- Cargo.toml | 1 + tests/cli/meta_build.rs | 84 ++++++++++++++ tests/cli/meta_build/included_build_suffix.rs | 80 ++++++++++++++ tests/cli/meta_build/included_main.rs | 2 + tests/cli/mod.rs | 92 ++++++++++++++++ tests/common/command_invocation.rs | 57 ++++++++++ tests/common/make_a_crate.rs | 43 ++++++++ tests/common/which_temp.rs | 83 ++++++++++++++ tests/ice.rs | 103 ++++++++++++++++++ tests/icy_code/included_main.rs | 10 ++ 10 files changed, 555 insertions(+) create mode 100644 tests/cli/meta_build.rs create mode 100644 tests/cli/meta_build/included_build_suffix.rs create mode 100644 tests/cli/meta_build/included_main.rs create mode 100644 tests/cli/mod.rs create mode 100644 tests/common/command_invocation.rs create mode 100644 tests/common/make_a_crate.rs create mode 100644 tests/common/which_temp.rs create mode 100644 tests/ice.rs create mode 100644 tests/icy_code/included_main.rs diff --git a/Cargo.toml b/Cargo.toml index e4d5024..4522b7f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,3 +36,4 @@ colored="1.9" [dev-dependencies] quickcheck = "0.9.2" tempfile = "3.1.0" +test_bin= "0.3.0" diff --git a/tests/cli/meta_build.rs b/tests/cli/meta_build.rs new file mode 100644 index 0000000..b2c6479 --- /dev/null +++ b/tests/cli/meta_build.rs @@ -0,0 +1,84 @@ +use std::fs::{DirBuilder}; +use std::path::{Path}; + +pub struct InjectionPoint { + pub date: YearMonthDay, + pub associated_sha: &'static str, +} + +pub struct Test<'a> { + pub crate_name: &'a str, + pub cli_params: &'a [&'a str], + pub delta_date: InjectionPoint, + pub delta_kind: DeltaKind, +} + +impl<'a> Test<'a> { + pub fn expected_sha(&self) -> &str { + self.delta_date.associated_sha + } +} + +pub fn make_crate_files( + dir_builder: &DirBuilder, + dir: &Path, + test: &Test) + -> Result<(), failure::Error> +{ + (crate::make_a_crate::Crate { + dir, + name: test.crate_name, + build_rs: Some(meta_build(test).into()), + cargo_toml: format!(r##" +[package] +name = "{NAME}" +version = "0.1.0" +authors = ["Felix S. Klock II "] +"##, NAME=test.crate_name).into(), + main_rs: MAIN_RS.into(), + }).make_files(dir_builder)?; + + Ok(()) +} + +// A test crate to exercise `cargo-bisect-rustc` has three basic components: a +// Cargo.toml file, a build.rs script that inspects the current version of Rust +// and injects an error for the appropriate versions into a build-time generated +// version.rs file, and a main.rs file that include!'s the version.rs file +// +// We only inject errors based on YYYY-MM-DD date comparison (<, <=, >=, >), and +// having that conditonally add a `#[rustc_error]` to the (injected) `fn main()` +// function. + +const MAIN_RS: &'static str = std::include_str!("meta_build/included_main.rs"); + +#[derive(Copy, Clone)] +pub struct YearMonthDay(pub u32, pub u32, pub u32); + +#[derive(Copy, Clone)] +pub enum DeltaKind { Fix, Err } + +fn meta_build(test: &Test) -> String { + let YearMonthDay(year, month, day) = test.delta_date.date; + let delta_kind = test.delta_kind; + let date_item = format!(r##" +/// `DELTA_DATE` identfies nightly where simulated change was injected. +const DELTA_DATE: YearMonthDay = YearMonthDay({YEAR}, {MONTH}, {DAY}); +"##, + YEAR=year, MONTH=month, DAY=day); + + let kind_variant = match delta_kind { + DeltaKind::Fix => "Fix", + DeltaKind::Err => "Err", + }; + let kind_item = format!(r##" +/// `DELTA_KIND` identfies whether simulated change is new error, or a fix to ancient error. +const DELTA_KIND: DeltaKind = DeltaKind::{VARIANT}; +"##, + VARIANT=kind_variant); + + format!("{DATE_ITEM}{KIND_ITEM}{SUFFIX}", + DATE_ITEM=date_item, KIND_ITEM=kind_item, SUFFIX=BUILD_SUFFIX) +} + +const BUILD_SUFFIX: &'static str = std::include_str!("meta_build/included_build_suffix.rs"); diff --git a/tests/cli/meta_build/included_build_suffix.rs b/tests/cli/meta_build/included_build_suffix.rs new file mode 100644 index 0000000..57f1941 --- /dev/null +++ b/tests/cli/meta_build/included_build_suffix.rs @@ -0,0 +1,80 @@ +// Strategy inspired by dtolnay/rustversion: run `rustc --version` at build time +// to observe version info. +// +// (The dtolnay/rustversion is dual-licensed under APACHE/MIT as of January 2020.) + +use std::env; +use std::ffi::OsString; +use std::fs; +use std::path::Path; +use std::process::{self, Command}; + +#[derive(PartialOrd, Ord, PartialEq, Eq, Debug)] +struct YearMonthDay(u32, u32, u32); + +enum DeltaKind { Fix, Err } + +fn main() { + let mut context = Context::introspect(); + context.generate(); +} + +struct Context { + commit: Commit, + rustc_date: YearMonthDay, +} + +#[derive(PartialOrd, Ord, PartialEq, Eq, Debug)] +struct Commit(String); + +impl Context { + fn introspect() -> Context { + let rustc = env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc")); + let output = Command::new(&rustc).arg("--version").output().unwrap_or_else(|e| { + let rustc = rustc.to_string_lossy(); + eprintln!("Error: failed to run `{} --version`: {}", rustc, e); + process::exit(1); + }); + let output = String::from_utf8(output.stdout).unwrap(); + let mut tokens = output.split(' '); + + let _rustc = tokens.next().unwrap(); + let _version = tokens.next().unwrap(); + let open_paren_commit = tokens.next().unwrap(); + let date_close_paren = tokens.next().unwrap(); + + let commit = Commit(open_paren_commit[1..].to_string()); + + let date_str: String = + date_close_paren.matches(|c: char| c.is_numeric() || c == '-').collect(); + let mut date_parts = date_str.split('-'); + let year: u32 = date_parts.next().unwrap().parse().unwrap(); + let month: u32 = date_parts.next().unwrap().parse().unwrap(); + let day: u32 = date_parts.next().unwrap().parse().unwrap(); + + Context { commit, rustc_date: YearMonthDay(year, month, day) } + } + + fn generate(&mut self) { + let inject_with_error = match DELTA_KIND { + DeltaKind::Err => self.rustc_date >= DELTA_DATE, + DeltaKind::Fix => self.rustc_date < DELTA_DATE, + }; + let prefix = if inject_with_error { "#[rustc_error] " } else { "" }; + let maybe_static_error = format!("{PREFIX}{ITEM}", PREFIX=prefix, ITEM="fn main() { }"); + + let content = format!(r#"{MAIN} +pub const COMMIT: &'static str = "{COMMIT}"; +pub const DATE: &'static str = "{Y:04}-{M:02}-{D:02}"; +"#, + MAIN=maybe_static_error, + COMMIT=self.commit.0, + Y=self.rustc_date.0, + M=self.rustc_date.1, + D=self.rustc_date.2); + + let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR not set"); + let out_file = Path::new(&out_dir).join("version.rs"); + fs::write(out_file, content).expect("failed to write version.rs"); + } +} diff --git a/tests/cli/meta_build/included_main.rs b/tests/cli/meta_build/included_main.rs new file mode 100644 index 0000000..7db419b --- /dev/null +++ b/tests/cli/meta_build/included_main.rs @@ -0,0 +1,2 @@ +#![feature(rustc_attrs)] +include!(concat!(env!("OUT_DIR"), "/version.rs")); diff --git a/tests/cli/mod.rs b/tests/cli/mod.rs new file mode 100644 index 0000000..19c3801 --- /dev/null +++ b/tests/cli/mod.rs @@ -0,0 +1,92 @@ +const INJECTION_COMMIT: &'static str = "f8fd4624474a68bd26694eff3536b9f3a127b2d3"; +const INJECTION_LOWER_BOUND: &'static str = "2020-02-06"; +const INJECTION_UPPER_BOUND: &'static str = "2020-02-08"; + +const INJECTION_POINT: InjectionPoint = InjectionPoint { + date: YearMonthDay(2020, 02, 07), + associated_sha: INJECTION_COMMIT, +}; + +mod cli { + pub(crate) mod meta_build; +} + +pub(crate) use self::cli::meta_build; + +mod common { + pub(crate) mod command_invocation; + pub(crate) mod make_a_crate; + pub(crate) mod which_temp; +} + +pub(crate) use self::common::command_invocation; +pub(crate) use self::common::make_a_crate; +pub(crate) use self::common::which_temp; + +use self::meta_build::{DeltaKind, InjectionPoint, Test, YearMonthDay}; +use self::which_temp::{WhichTempDir, WhichTempDirectory}; + +// These tests pass `--preserve` and `--access=github` because that is the best +// way to try to ensure that the tests complete as quickly as possible. + +pub const BASIC_TEST: Test = Test { + crate_name: "cbr_test_cli_basic", + cli_params: &["--preserve", "--access=github", + "--start", INJECTION_LOWER_BOUND, "--end", INJECTION_UPPER_BOUND], + delta_date: INJECTION_POINT, + delta_kind: DeltaKind::Err, +}; + +pub const FIXED_TEST: Test = Test { + crate_name: "cbr_test_cli_fixed", + cli_params: &["--regress=success", + "--preserve", "--access=github", + "--start", INJECTION_LOWER_BOUND, "--end", INJECTION_UPPER_BOUND], + delta_date: INJECTION_POINT, + delta_kind: DeltaKind::Fix, +}; + +// Ordinarily, I would put both of these tests into separate `#[test]` methods. +// However, if you do that, then `cargo test` will run them in parallel, and you +// end up with `cargo-bisect-rustc` racing to install the toolchains it +// downloads. +// +// (It is arguably a bug that we do not gracefully handle this situation.) +// +// In any case, the simplest fix for the test infrastructure is to ensure that +// no tests overlap in the range of dates they search for a regression. +#[test] +fn cli_test() -> Result<(), failure::Error> { + test_cli_core::(&BASIC_TEST)?; + test_cli_core::(&FIXED_TEST)?; + Ok(()) +} + +fn test_cli_core(test: &meta_build::Test) -> Result<(), failure::Error> +where WhichTemp: WhichTempDirectory +{ + let root = WhichTemp::root()?; + let tmp_dir = WhichTemp::target(&root); + let dir = tmp_dir.join(test.crate_name); + + let dir_builder = WhichTemp::dir_builder(); + meta_build::make_crate_files(&dir_builder, &dir, test)?; + + let mut cmd = command_invocation::Context { + cli_params: test.cli_params, + dir: dir.as_path(), + }; + + let command_invocation::Output { status: _, stderr, stdout } = cmd.run()?; + + println!("Command output stdout for {}: \n```\n{}\n```", test.crate_name, stdout); + println!("Command output stderr for {}: \n```\n{}\n```", test.crate_name, stderr); + + // The most basic check: does the output actually tell us about the + // "regressing" commit. + let needle = format!("Regression in {}", test.expected_sha()); + // println!("searching for {:?} in stdout: {:?} stderr: {:?}", needle, stdout, stderr); + assert!(stderr.contains(&needle)); + + Ok(()) +} diff --git a/tests/common/command_invocation.rs b/tests/common/command_invocation.rs new file mode 100644 index 0000000..47bdd48 --- /dev/null +++ b/tests/common/command_invocation.rs @@ -0,0 +1,57 @@ +use std::path::Path; +use std::process::ExitStatus; + +pub(crate) struct Context<'a> { + pub cli_params: &'a [&'a str], + pub dir: &'a Path, +} + +pub struct Output { + pub status: ExitStatus, + pub stdout: String, + pub stderr: String, +} + +impl<'a> Context<'a> { + pub fn run(&mut self) -> Result { + let mut command = test_bin::get_test_bin("cargo-bisect-rustc"); + for param in self.cli_params { + command.arg(param); + } + let dir = self.dir; + println!( + "running `{:?} {}` in {:?}", + command, + self.cli_params.join(" "), + dir.display() + ); + assert!(dir.exists()); + let output = command.current_dir(dir).output()?; + + let stderr = String::from_utf8_lossy(&output.stderr); + + // prepass over the captured stdout, which by default emits a lot of + // progressive info about downlaods that is fine in interactive settings + // but just makes for a lot of junk when you want to understand the + // final apparent output. + let mut stdout = String::with_capacity(output.stdout.len()); + let mut line = String::new(); + for c in &output.stdout { + match *c as char { + '\r' => line.clear(), + '\n' => { + stdout.push_str(&line); + line.clear(); + } + c => line.push(c), + } + } + stdout.push_str(&line); + + Ok(Output { + status: output.status, + stderr: stderr.to_string(), + stdout, + }) + } +} diff --git a/tests/common/make_a_crate.rs b/tests/common/make_a_crate.rs new file mode 100644 index 0000000..bde21f0 --- /dev/null +++ b/tests/common/make_a_crate.rs @@ -0,0 +1,43 @@ +use std::borrow::Cow; +use std::fs::{DirBuilder, File}; +use std::io::{Write}; +use std::path::{Path}; + +type Text<'a> = Cow<'a, str>; + +pub struct Crate<'a> { + pub dir: &'a Path, + pub name: &'a str, + pub build_rs: Option>, + pub cargo_toml: Text<'a>, + pub main_rs: Text<'a>, +} + +impl<'a> Crate<'a> { + pub fn make_files(&self, dir_builder: &DirBuilder) -> Result<(), failure::Error> { + let dir = self.dir; + let cargo_toml_path = dir.join("Cargo.toml"); + let build_path = dir.join("build.rs"); + let src_path = dir.join("src"); + let main_path = src_path.join("main.rs"); + + dir_builder.create(&dir)?; + dir_builder.create(src_path)?; + + if let Some(build_rs) = &self.build_rs { + let mut build_file = File::create(build_path)?; + writeln!(build_file, "{}", build_rs)?; + build_file.sync_data()?; + } + + let mut cargo_toml_file = File::create(cargo_toml_path)?; + writeln!(cargo_toml_file, "{}", self.cargo_toml)?; + cargo_toml_file.sync_data()?; + + let mut main_file = File::create(main_path)?; + writeln!(main_file, "{}", self.main_rs)?; + main_file.sync_data()?; + + Ok(()) + } +} diff --git a/tests/common/which_temp.rs b/tests/common/which_temp.rs new file mode 100644 index 0000000..573c196 --- /dev/null +++ b/tests/common/which_temp.rs @@ -0,0 +1,83 @@ +use tempfile::TempDir; +use std::fs::{DirBuilder}; +use std::path::{Path, PathBuf}; + +pub trait WhichTempDirectory { + type Root; + fn root() -> Result; + fn target(root: &Self::Root) -> &Path; + fn dir_builder() -> DirBuilder; +} + +// If you change this to `GenerateIntoSlashTemp`, you get a fixed directory +// rooted at `/tmp/` so that e.g. inspect the generated code, or even run +// cargo-bisect-rustc yourself there. +// +// Doing so has two drawbacks: 1. You need to clean-up the +// generated directory yourself, and 2. You risk race conditions with +// concurrent test runs. +// +// If you cange this to `GenerateIntoFreshTemp`, you get a fresh directory +// (rooted at whatever is returned from `tempfile::tempdir()`). This reduces +// race conditions (note that `cargo-bisect-rustc` still stores data in shared +// locations like `~/.rustup`, so races can still arise) and allows the test +// suite to clean up the directory itself. +// +// The `WhichTempDir` type is meant to always implement the `WhichTempDirectory` +// trait. +pub(crate) type WhichTempDir = GenerateIntoFreshTemp; + +/// Using `GenerateIntoFreshTemp` yields a fresh directory in some +/// system-dependent temporary space. This fresh directory will be autoamtically +/// cleaned up after each test run, regardless of whether the test succeeded or +/// failed. +/// +/// This approach has the following drawbacks: 1. the directories are fresh, so +/// any state stored in the local directory (like a rust.git checkout) will need +/// to be re-downloaded on each test run, even when using the `--preserve` +/// parameter, and 2. the directories are automatically deleted, which means you +/// cannot readily inspect the generated source code when trying to debug a test +/// failure. +/// +/// The above two drawbacks can be sidestepped by using `GenerateIntoSlashTemp` +/// instead; see below. +pub(crate) struct GenerateIntoFreshTemp; + +/// Using `GenerateIntoSlashTemp` yields a fixed directory rooted at `/tmp/` so +/// that you can inspect the generated code, or even run cargo-bisect-rustc +/// yourself there. +/// +/// This approach has two drawbacks: 1. You need to clean-up the generated +/// directory yourself, and 2. You risk race conditions with concurrent test +/// runs. +pub(crate) struct GenerateIntoSlashTemp; + +impl WhichTempDirectory for GenerateIntoSlashTemp { + type Root = PathBuf; + fn root() -> Result { + Ok(PathBuf::from("/tmp")) + } + fn target(root: &PathBuf) -> &Path { + root + } + fn dir_builder() -> DirBuilder { + // make recursive DirBuilder so that we do not error on pre-existing + // directory. + let mut b = DirBuilder::new(); + b.recursive(true); + b + } +} + +impl WhichTempDirectory for GenerateIntoFreshTemp { + type Root = TempDir; + fn root() -> Result { + Ok(tempfile::tempdir()?) + } + fn target(root: &TempDir) -> &Path { + root.path() + } + fn dir_builder() -> DirBuilder { + DirBuilder::new() + } +} diff --git a/tests/ice.rs b/tests/ice.rs new file mode 100644 index 0000000..ec7e38f --- /dev/null +++ b/tests/ice.rs @@ -0,0 +1,103 @@ +const CRATE_NAME: &'static str = "eventually_ice"; +const INJECTION_COMMIT: &'static str = "6af388b25050bca26710be7e4030e17bf6d8d2f7"; +const INJECTION_LOWER_BOUND: &'static str = "2020-02-20"; +const INJECTION_UPPER_BOUND: &'static str = "2020-02-22"; + +// This main.rs captures a bit of code that has a known ICE that was +// introduced relatively recently, as reported in issue #69615. +const ICE_MAIN_RS: &'static str = std::include_str!("icy_code/included_main.rs"); + +// This test crate encodes a case of an internal compiler error (ICE) that was +// injected relatively recently (see `ICE_MAIN_RS` below). The intention is to +// use this to test the handling of `--regress=ice` option to `rustc`. +// +// Note that the main intention of the test is to capture the distinction +// between flagging a static error in the program from signalling an internal +// error. That is: Using an example here of a *correct* program that causes an +// ICE is not a great test, because it wouldn't necessarily be testing the +// important effect of the `--regress=ice` flag. +// +// In the long term, since we only store binaries over a fixed length of time, +// this test will need to be updated with new examples of ICE's. (For now it +// seems safe to assume that the compiler will always have *some* example +// program that can be used to observe an ICE.) + +const CARGO_TOML: &'static str = r##" +[package] +name = "eventually-ice" +version = "0.1.0" +authors = ["Felix S. Klock II "] +"##; + +mod common { + pub(crate) mod command_invocation; + pub(crate) mod make_a_crate; + pub(crate) mod which_temp; +} + +pub(crate) use self::common::command_invocation; +pub(crate) use self::common::make_a_crate; +pub(crate) use self::common::which_temp; + +use self::which_temp::{WhichTempDir, WhichTempDirectory}; + +#[test] +fn ice_test() -> Result<(), failure::Error> { + test_ice_core::() +} + +fn test_ice_core() -> Result<(), failure::Error> +where + WhichTemp: WhichTempDirectory, +{ + let root = WhichTemp::root()?; + let tmp_dir = WhichTemp::target(&root); + let dir = tmp_dir.join(CRATE_NAME); + + let dir_builder = WhichTemp::dir_builder(); + + (make_a_crate::Crate { + dir: &dir, + name: CRATE_NAME, + build_rs: None, + cargo_toml: CARGO_TOML.into(), + main_rs: ICE_MAIN_RS.into(), + }) + .make_files(&dir_builder)?; + + let mut cmd = command_invocation::Context { + cli_params: &[ + "--preserve", + "--regress=ice", + "--access=github", + "--start", + INJECTION_LOWER_BOUND, + "--end", + INJECTION_UPPER_BOUND, + ], + dir: dir.as_path(), + }; + + let command_invocation::Output { + status: _, + stderr, + stdout, + } = cmd.run()?; + + println!( + "Command output stdout for {}: \n```\n{}\n```", + CRATE_NAME, stdout + ); + println!( + "Command output stderr for {}: \n```\n{}\n```", + CRATE_NAME, stderr + ); + + // The most basic check: does the output actually tell us about the + // "regressing" commit. + let needle = format!("Regression in {}", INJECTION_COMMIT); + // println!("searching for {:?} in stdout: {:?} stderr: {:?}", needle, stdout, stderr); + assert!(stderr.contains(&needle)); + + Ok(()) +} diff --git a/tests/icy_code/included_main.rs b/tests/icy_code/included_main.rs new file mode 100644 index 0000000..0eff602 --- /dev/null +++ b/tests/icy_code/included_main.rs @@ -0,0 +1,10 @@ +#![feature(const_trait_impl, const_fn)] + +pub trait MyTrait { fn method(&self); } + +impl const MyTrait for std::convert::Infallible { + #[inline(always)] + fn method(&self) { match *self {} } +} + +fn main() { } From da1ee9d3888ec20ba23c35507aec47eb7674939d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 15 May 2020 12:27:30 -0400 Subject: [PATCH 4/4] Update `Cargo.lock` according to addition of `test_bin` dep in prior commit. --- Cargo.lock | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index dea958c..b369c7f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -149,6 +149,7 @@ dependencies = [ "tee 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "test_bin 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "xz2 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1372,6 +1373,11 @@ dependencies = [ "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "test_bin" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "textwrap" version = "0.11.0" @@ -1862,6 +1868,7 @@ dependencies = [ "checksum termcolor 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)" = "96d6098003bde162e4277c70665bd87c326f5a0c3f3fbfb285787fa482d54e6e" "checksum termion 1.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "6a8fb22f7cde82c8220e5aeacb3258ed7ce996142c77cba193f203515e26c330" "checksum termios 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "72b620c5ea021d75a735c943269bb07d30c9b77d6ac6b236bc8b5c496ef05625" +"checksum test_bin 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1937bab04b0bd0f2c19628d3408fb6dd78faf5aa60f4bdb03212507a9b7314ba" "checksum textwrap 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d326610f408c7a4eb6f51c37c330e496b08506c9457c9d34287ecc38809fb060" "checksum thread_local 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" "checksum time 0.1.42 (registry+https://github.com/rust-lang/crates.io-index)" = "db8dcfca086c1143c9270ac42a2bbd8a7ee477b78ac8e45b19abfb0cbede4b6f"