Skip to content

Commit

Permalink
Infrastructure for testing the command line tool.
Browse files Browse the repository at this point in the history
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 <bjorn3@users.noreply.github.com>

----

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 rust-lang/rustfmt#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_<name>` 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 rust-lang/rustfmt#2781 ) with an additional note
in the comment above the definition.)
  • Loading branch information
pnkfelix committed May 15, 2020
1 parent 82e0b79 commit 1192338
Show file tree
Hide file tree
Showing 10 changed files with 555 additions and 0 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ colored="1.9"
[dev-dependencies]
quickcheck = "0.9.2"
tempfile = "3.1.0"
test_bin= "0.3.0"
84 changes: 84 additions & 0 deletions tests/cli/meta_build.rs
Original file line number Diff line number Diff line change
@@ -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 <pnkfelix@pnkfx.org>"]
"##, 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");
80 changes: 80 additions & 0 deletions tests/cli/meta_build/included_build_suffix.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
2 changes: 2 additions & 0 deletions tests/cli/meta_build/included_main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#![feature(rustc_attrs)]
include!(concat!(env!("OUT_DIR"), "/version.rs"));
92 changes: 92 additions & 0 deletions tests/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -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::<WhichTempDir>(&BASIC_TEST)?;
test_cli_core::<WhichTempDir>(&FIXED_TEST)?;
Ok(())
}

fn test_cli_core<WhichTemp>(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(())
}
57 changes: 57 additions & 0 deletions tests/common/command_invocation.rs
Original file line number Diff line number Diff line change
@@ -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<Output, failure::Error> {
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,
})
}
}
43 changes: 43 additions & 0 deletions tests/common/make_a_crate.rs
Original file line number Diff line number Diff line change
@@ -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<Text<'a>>,
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(())
}
}
Loading

0 comments on commit 1192338

Please sign in to comment.