Skip to content

Commit 1192338

Browse files
committed
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 <[email protected]> ---- 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.)
1 parent 82e0b79 commit 1192338

10 files changed

+555
-0
lines changed

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,4 @@ colored="1.9"
3636
[dev-dependencies]
3737
quickcheck = "0.9.2"
3838
tempfile = "3.1.0"
39+
test_bin= "0.3.0"

tests/cli/meta_build.rs

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
use std::fs::{DirBuilder};
2+
use std::path::{Path};
3+
4+
pub struct InjectionPoint {
5+
pub date: YearMonthDay,
6+
pub associated_sha: &'static str,
7+
}
8+
9+
pub struct Test<'a> {
10+
pub crate_name: &'a str,
11+
pub cli_params: &'a [&'a str],
12+
pub delta_date: InjectionPoint,
13+
pub delta_kind: DeltaKind,
14+
}
15+
16+
impl<'a> Test<'a> {
17+
pub fn expected_sha(&self) -> &str {
18+
self.delta_date.associated_sha
19+
}
20+
}
21+
22+
pub fn make_crate_files(
23+
dir_builder: &DirBuilder,
24+
dir: &Path,
25+
test: &Test)
26+
-> Result<(), failure::Error>
27+
{
28+
(crate::make_a_crate::Crate {
29+
dir,
30+
name: test.crate_name,
31+
build_rs: Some(meta_build(test).into()),
32+
cargo_toml: format!(r##"
33+
[package]
34+
name = "{NAME}"
35+
version = "0.1.0"
36+
authors = ["Felix S. Klock II <[email protected]>"]
37+
"##, NAME=test.crate_name).into(),
38+
main_rs: MAIN_RS.into(),
39+
}).make_files(dir_builder)?;
40+
41+
Ok(())
42+
}
43+
44+
// A test crate to exercise `cargo-bisect-rustc` has three basic components: a
45+
// Cargo.toml file, a build.rs script that inspects the current version of Rust
46+
// and injects an error for the appropriate versions into a build-time generated
47+
// version.rs file, and a main.rs file that include!'s the version.rs file
48+
//
49+
// We only inject errors based on YYYY-MM-DD date comparison (<, <=, >=, >), and
50+
// having that conditonally add a `#[rustc_error]` to the (injected) `fn main()`
51+
// function.
52+
53+
const MAIN_RS: &'static str = std::include_str!("meta_build/included_main.rs");
54+
55+
#[derive(Copy, Clone)]
56+
pub struct YearMonthDay(pub u32, pub u32, pub u32);
57+
58+
#[derive(Copy, Clone)]
59+
pub enum DeltaKind { Fix, Err }
60+
61+
fn meta_build(test: &Test) -> String {
62+
let YearMonthDay(year, month, day) = test.delta_date.date;
63+
let delta_kind = test.delta_kind;
64+
let date_item = format!(r##"
65+
/// `DELTA_DATE` identfies nightly where simulated change was injected.
66+
const DELTA_DATE: YearMonthDay = YearMonthDay({YEAR}, {MONTH}, {DAY});
67+
"##,
68+
YEAR=year, MONTH=month, DAY=day);
69+
70+
let kind_variant = match delta_kind {
71+
DeltaKind::Fix => "Fix",
72+
DeltaKind::Err => "Err",
73+
};
74+
let kind_item = format!(r##"
75+
/// `DELTA_KIND` identfies whether simulated change is new error, or a fix to ancient error.
76+
const DELTA_KIND: DeltaKind = DeltaKind::{VARIANT};
77+
"##,
78+
VARIANT=kind_variant);
79+
80+
format!("{DATE_ITEM}{KIND_ITEM}{SUFFIX}",
81+
DATE_ITEM=date_item, KIND_ITEM=kind_item, SUFFIX=BUILD_SUFFIX)
82+
}
83+
84+
const BUILD_SUFFIX: &'static str = std::include_str!("meta_build/included_build_suffix.rs");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Strategy inspired by dtolnay/rustversion: run `rustc --version` at build time
2+
// to observe version info.
3+
//
4+
// (The dtolnay/rustversion is dual-licensed under APACHE/MIT as of January 2020.)
5+
6+
use std::env;
7+
use std::ffi::OsString;
8+
use std::fs;
9+
use std::path::Path;
10+
use std::process::{self, Command};
11+
12+
#[derive(PartialOrd, Ord, PartialEq, Eq, Debug)]
13+
struct YearMonthDay(u32, u32, u32);
14+
15+
enum DeltaKind { Fix, Err }
16+
17+
fn main() {
18+
let mut context = Context::introspect();
19+
context.generate();
20+
}
21+
22+
struct Context {
23+
commit: Commit,
24+
rustc_date: YearMonthDay,
25+
}
26+
27+
#[derive(PartialOrd, Ord, PartialEq, Eq, Debug)]
28+
struct Commit(String);
29+
30+
impl Context {
31+
fn introspect() -> Context {
32+
let rustc = env::var_os("RUSTC").unwrap_or_else(|| OsString::from("rustc"));
33+
let output = Command::new(&rustc).arg("--version").output().unwrap_or_else(|e| {
34+
let rustc = rustc.to_string_lossy();
35+
eprintln!("Error: failed to run `{} --version`: {}", rustc, e);
36+
process::exit(1);
37+
});
38+
let output = String::from_utf8(output.stdout).unwrap();
39+
let mut tokens = output.split(' ');
40+
41+
let _rustc = tokens.next().unwrap();
42+
let _version = tokens.next().unwrap();
43+
let open_paren_commit = tokens.next().unwrap();
44+
let date_close_paren = tokens.next().unwrap();
45+
46+
let commit = Commit(open_paren_commit[1..].to_string());
47+
48+
let date_str: String =
49+
date_close_paren.matches(|c: char| c.is_numeric() || c == '-').collect();
50+
let mut date_parts = date_str.split('-');
51+
let year: u32 = date_parts.next().unwrap().parse().unwrap();
52+
let month: u32 = date_parts.next().unwrap().parse().unwrap();
53+
let day: u32 = date_parts.next().unwrap().parse().unwrap();
54+
55+
Context { commit, rustc_date: YearMonthDay(year, month, day) }
56+
}
57+
58+
fn generate(&mut self) {
59+
let inject_with_error = match DELTA_KIND {
60+
DeltaKind::Err => self.rustc_date >= DELTA_DATE,
61+
DeltaKind::Fix => self.rustc_date < DELTA_DATE,
62+
};
63+
let prefix = if inject_with_error { "#[rustc_error] " } else { "" };
64+
let maybe_static_error = format!("{PREFIX}{ITEM}", PREFIX=prefix, ITEM="fn main() { }");
65+
66+
let content = format!(r#"{MAIN}
67+
pub const COMMIT: &'static str = "{COMMIT}";
68+
pub const DATE: &'static str = "{Y:04}-{M:02}-{D:02}";
69+
"#,
70+
MAIN=maybe_static_error,
71+
COMMIT=self.commit.0,
72+
Y=self.rustc_date.0,
73+
M=self.rustc_date.1,
74+
D=self.rustc_date.2);
75+
76+
let out_dir = env::var_os("OUT_DIR").expect("OUT_DIR not set");
77+
let out_file = Path::new(&out_dir).join("version.rs");
78+
fs::write(out_file, content).expect("failed to write version.rs");
79+
}
80+
}

tests/cli/meta_build/included_main.rs

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#![feature(rustc_attrs)]
2+
include!(concat!(env!("OUT_DIR"), "/version.rs"));

tests/cli/mod.rs

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
const INJECTION_COMMIT: &'static str = "f8fd4624474a68bd26694eff3536b9f3a127b2d3";
2+
const INJECTION_LOWER_BOUND: &'static str = "2020-02-06";
3+
const INJECTION_UPPER_BOUND: &'static str = "2020-02-08";
4+
5+
const INJECTION_POINT: InjectionPoint = InjectionPoint {
6+
date: YearMonthDay(2020, 02, 07),
7+
associated_sha: INJECTION_COMMIT,
8+
};
9+
10+
mod cli {
11+
pub(crate) mod meta_build;
12+
}
13+
14+
pub(crate) use self::cli::meta_build;
15+
16+
mod common {
17+
pub(crate) mod command_invocation;
18+
pub(crate) mod make_a_crate;
19+
pub(crate) mod which_temp;
20+
}
21+
22+
pub(crate) use self::common::command_invocation;
23+
pub(crate) use self::common::make_a_crate;
24+
pub(crate) use self::common::which_temp;
25+
26+
use self::meta_build::{DeltaKind, InjectionPoint, Test, YearMonthDay};
27+
use self::which_temp::{WhichTempDir, WhichTempDirectory};
28+
29+
// These tests pass `--preserve` and `--access=github` because that is the best
30+
// way to try to ensure that the tests complete as quickly as possible.
31+
32+
pub const BASIC_TEST: Test = Test {
33+
crate_name: "cbr_test_cli_basic",
34+
cli_params: &["--preserve", "--access=github",
35+
"--start", INJECTION_LOWER_BOUND, "--end", INJECTION_UPPER_BOUND],
36+
delta_date: INJECTION_POINT,
37+
delta_kind: DeltaKind::Err,
38+
};
39+
40+
pub const FIXED_TEST: Test = Test {
41+
crate_name: "cbr_test_cli_fixed",
42+
cli_params: &["--regress=success",
43+
"--preserve", "--access=github",
44+
"--start", INJECTION_LOWER_BOUND, "--end", INJECTION_UPPER_BOUND],
45+
delta_date: INJECTION_POINT,
46+
delta_kind: DeltaKind::Fix,
47+
};
48+
49+
// Ordinarily, I would put both of these tests into separate `#[test]` methods.
50+
// However, if you do that, then `cargo test` will run them in parallel, and you
51+
// end up with `cargo-bisect-rustc` racing to install the toolchains it
52+
// downloads.
53+
//
54+
// (It is arguably a bug that we do not gracefully handle this situation.)
55+
//
56+
// In any case, the simplest fix for the test infrastructure is to ensure that
57+
// no tests overlap in the range of dates they search for a regression.
58+
#[test]
59+
fn cli_test() -> Result<(), failure::Error> {
60+
test_cli_core::<WhichTempDir>(&BASIC_TEST)?;
61+
test_cli_core::<WhichTempDir>(&FIXED_TEST)?;
62+
Ok(())
63+
}
64+
65+
fn test_cli_core<WhichTemp>(test: &meta_build::Test) -> Result<(), failure::Error>
66+
where WhichTemp: WhichTempDirectory
67+
{
68+
let root = WhichTemp::root()?;
69+
let tmp_dir = WhichTemp::target(&root);
70+
let dir = tmp_dir.join(test.crate_name);
71+
72+
let dir_builder = WhichTemp::dir_builder();
73+
meta_build::make_crate_files(&dir_builder, &dir, test)?;
74+
75+
let mut cmd = command_invocation::Context {
76+
cli_params: test.cli_params,
77+
dir: dir.as_path(),
78+
};
79+
80+
let command_invocation::Output { status: _, stderr, stdout } = cmd.run()?;
81+
82+
println!("Command output stdout for {}: \n```\n{}\n```", test.crate_name, stdout);
83+
println!("Command output stderr for {}: \n```\n{}\n```", test.crate_name, stderr);
84+
85+
// The most basic check: does the output actually tell us about the
86+
// "regressing" commit.
87+
let needle = format!("Regression in {}", test.expected_sha());
88+
// println!("searching for {:?} in stdout: {:?} stderr: {:?}", needle, stdout, stderr);
89+
assert!(stderr.contains(&needle));
90+
91+
Ok(())
92+
}

tests/common/command_invocation.rs

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
use std::path::Path;
2+
use std::process::ExitStatus;
3+
4+
pub(crate) struct Context<'a> {
5+
pub cli_params: &'a [&'a str],
6+
pub dir: &'a Path,
7+
}
8+
9+
pub struct Output {
10+
pub status: ExitStatus,
11+
pub stdout: String,
12+
pub stderr: String,
13+
}
14+
15+
impl<'a> Context<'a> {
16+
pub fn run(&mut self) -> Result<Output, failure::Error> {
17+
let mut command = test_bin::get_test_bin("cargo-bisect-rustc");
18+
for param in self.cli_params {
19+
command.arg(param);
20+
}
21+
let dir = self.dir;
22+
println!(
23+
"running `{:?} {}` in {:?}",
24+
command,
25+
self.cli_params.join(" "),
26+
dir.display()
27+
);
28+
assert!(dir.exists());
29+
let output = command.current_dir(dir).output()?;
30+
31+
let stderr = String::from_utf8_lossy(&output.stderr);
32+
33+
// prepass over the captured stdout, which by default emits a lot of
34+
// progressive info about downlaods that is fine in interactive settings
35+
// but just makes for a lot of junk when you want to understand the
36+
// final apparent output.
37+
let mut stdout = String::with_capacity(output.stdout.len());
38+
let mut line = String::new();
39+
for c in &output.stdout {
40+
match *c as char {
41+
'\r' => line.clear(),
42+
'\n' => {
43+
stdout.push_str(&line);
44+
line.clear();
45+
}
46+
c => line.push(c),
47+
}
48+
}
49+
stdout.push_str(&line);
50+
51+
Ok(Output {
52+
status: output.status,
53+
stderr: stderr.to_string(),
54+
stdout,
55+
})
56+
}
57+
}

tests/common/make_a_crate.rs

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
use std::borrow::Cow;
2+
use std::fs::{DirBuilder, File};
3+
use std::io::{Write};
4+
use std::path::{Path};
5+
6+
type Text<'a> = Cow<'a, str>;
7+
8+
pub struct Crate<'a> {
9+
pub dir: &'a Path,
10+
pub name: &'a str,
11+
pub build_rs: Option<Text<'a>>,
12+
pub cargo_toml: Text<'a>,
13+
pub main_rs: Text<'a>,
14+
}
15+
16+
impl<'a> Crate<'a> {
17+
pub fn make_files(&self, dir_builder: &DirBuilder) -> Result<(), failure::Error> {
18+
let dir = self.dir;
19+
let cargo_toml_path = dir.join("Cargo.toml");
20+
let build_path = dir.join("build.rs");
21+
let src_path = dir.join("src");
22+
let main_path = src_path.join("main.rs");
23+
24+
dir_builder.create(&dir)?;
25+
dir_builder.create(src_path)?;
26+
27+
if let Some(build_rs) = &self.build_rs {
28+
let mut build_file = File::create(build_path)?;
29+
writeln!(build_file, "{}", build_rs)?;
30+
build_file.sync_data()?;
31+
}
32+
33+
let mut cargo_toml_file = File::create(cargo_toml_path)?;
34+
writeln!(cargo_toml_file, "{}", self.cargo_toml)?;
35+
cargo_toml_file.sync_data()?;
36+
37+
let mut main_file = File::create(main_path)?;
38+
writeln!(main_file, "{}", self.main_rs)?;
39+
main_file.sync_data()?;
40+
41+
Ok(())
42+
}
43+
}

0 commit comments

Comments
 (0)