Skip to content

Commit

Permalink
speed up tests by doing rustup which
Browse files Browse the repository at this point in the history
`cargo test` goes from 2.4s to 1.6s
  • Loading branch information
Noratrieb committed Sep 23, 2023
1 parent ea5fb6e commit 50dc094
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 34 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

70 changes: 56 additions & 14 deletions src/build.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::{bail, Context, Result};
use anyhow::{bail, ensure, Context, Result};
use rustfix::diagnostics::Diagnostic;
use serde::Deserialize;
use std::{
Expand Down Expand Up @@ -48,11 +48,12 @@ struct BuildInner {
#[derive(Debug)]
enum BuildMode {
Cargo {
cargo_path: PathBuf,
/// May be something like `miri run`.
subcommand: Vec<String>,
},
Script(PathBuf),
Rustc,
Rustc(PathBuf),
}

impl Build {
Expand All @@ -68,16 +69,22 @@ impl Build {
.unwrap_or_default();

let mode = if options.rustc {
BuildMode::Rustc
let rustc = rustup_which("rustc")?;
BuildMode::Rustc(rustc)
} else if let Some(script) = &options.script_path {
BuildMode::Script(script.clone())
} else {
let subcommand = split_args(&options.cargo_subcmd);
BuildMode::Cargo { subcommand }
let cargo_path = rustup_which("cargo")?;
BuildMode::Cargo {
cargo_path,
subcommand,
}
};

let lint_mode = if options.rustc {
BuildMode::Rustc
let rustc = rustup_which("rustc")?;
BuildMode::Rustc(rustc)
} else if let Some(script) = options
.script_path_lints
.as_ref()
Expand All @@ -90,7 +97,12 @@ impl Build {
.as_deref()
.map(split_args)
.unwrap_or_else(|| split_args(&options.cargo_subcmd));
BuildMode::Cargo { subcommand }
let cargo_path = rustup_which("cargo")?;

BuildMode::Cargo {
cargo_path,
subcommand,
}
};

let verify = if options.no_verify {
Expand Down Expand Up @@ -136,15 +148,20 @@ impl Build {
}

let (is_ice, cmd_status, output) = match &inner.mode {
BuildMode::Cargo { subcommand } => {
let mut cmd = self.cmd("cargo");
BuildMode::Cargo {
cargo_path,
subcommand,
} => {
let mut cmd = self.cmd(cargo_path);

cmd.args(subcommand);

if inner.allow_color {
cmd.arg("--color=always");
}

extra_cargoflags(&mut cmd);

cmd.args(&inner.extra_args);

for env in &inner.env {
Expand All @@ -162,8 +179,8 @@ impl Build {
output,
)
}
BuildMode::Rustc => {
let mut cmd = self.cmd("rustc");
BuildMode::Rustc(rustc) => {
let mut cmd = self.cmd(rustc);
cmd.args(["--edition", "2021"]);
cmd.arg(&inner.input_path);

Expand Down Expand Up @@ -244,13 +261,18 @@ impl Build {
}

let diags = match &inner.lint_mode {
BuildMode::Cargo { subcommand } => {
let mut cmd = self.cmd("cargo");
BuildMode::Cargo {
cargo_path,
subcommand,
} => {
let mut cmd = self.cmd(cargo_path);

cmd.args(subcommand);

cmd.arg("--message-format=json");

extra_cargoflags(&mut cmd);

cmd.args(&inner.extra_args);

for env in &inner.env {
Expand All @@ -262,8 +284,8 @@ impl Build {

grab_cargo_diags(&output)?
}
BuildMode::Rustc => {
let mut cmd = self.cmd("rustc");
BuildMode::Rustc(rustc) => {
let mut cmd = self.cmd(rustc);
cmd.args(["--edition", "2021", "--error-format=json"]);
cmd.arg(&inner.input_path);

Expand Down Expand Up @@ -317,6 +339,26 @@ impl Build {
}
}

fn extra_cargoflags(cargo: &mut Command) {
cargo.arg("--offline");
}

pub fn rustup_which(tool: &str) -> Result<PathBuf> {
let output = Command::new("rustup")
.arg("which")
.arg(tool)
.output()
.context("running rustup which")?;

ensure!(output.status.success(), "rustup which failed");

Ok(String::from_utf8(output.stdout)
.context("rustup which returned invalid utf8")?
.trim()
.to_owned()
.into())
}

#[derive(Debug)]
pub struct BuildResult {
reproduces_issue: bool,
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ mod formatting;
mod passes;
mod processor;

pub use build::rustup_which;

#[cfg(this_pulls_in_cargo_which_is_a_big_dep_i_dont_like_it)]
mod expand;

Expand Down
1 change: 1 addition & 0 deletions testsuite/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ edition = "2021"

[dependencies]
anyhow = "1.0.70"
cargo-minimize = { path = ".." }
fs_extra = "1.3.0"
once_cell = "1.17.1"
rayon = "1.7.0"
Expand Down
10 changes: 10 additions & 0 deletions testsuite/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# testsuite

The test suite works the following way:

We have a bunch of files in `$WORKSPACE/full-tests`, every file is a test. We then run
`cargo-minimize` on that. `~MINIMIZE-ROOT` are required to be present in the minimization,
and we expect `~REQUIRE-DELETED` to be deleted by cargo-minimize.

We use `bin/regression_checked` as our custom script to verify whether it "reproduces", where
for us, "reproduces" means "all roots are present and the code compiles".
6 changes: 4 additions & 2 deletions testsuite/src/bin/regression_checker.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use anyhow::bail;

fn main() -> anyhow::Result<()> {
let cargo = std::env::var("MINIMIZE_CARGO").expect("MINIMIZE_CARGO");

if std::env::var("MINIMIZE_LINTS").as_deref() == Ok("1") {
std::process::Command::new("cargo")
std::process::Command::new(&cargo)
.arg("check")
.spawn()
.unwrap()
Expand All @@ -18,7 +20,7 @@ fn main() -> anyhow::Result<()> {

testsuite::ensure_roots_kept(&proj_dir, roots)?;

let check = std::process::Command::new("cargo")
let check = std::process::Command::new(&cargo)
.arg("check")
.spawn()
.unwrap()
Expand Down
43 changes: 25 additions & 18 deletions testsuite/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ pub fn ensure_roots_kept(
Ok(())
}

fn run_build(command: &mut Command) -> Result<()> {
fn run_build(cargo: &Path, command: &mut Command) -> Result<()> {
command.env("MINIMIZE_CARGO", cargo);
let exit = command
.spawn()
.context("failed to spawn command")?
Expand All @@ -45,17 +46,22 @@ fn run_build(command: &mut Command) -> Result<()> {
}

pub fn full_tests() -> Result<()> {
run_build(Command::new("cargo").args([
"build",
"-p",
"cargo-minimize",
"-p",
"testsuite",
"--bin",
"regression_checker",
"--bin",
"cargo-minimize",
]))
let cargo = cargo_minimize::rustup_which("cargo")?;

run_build(
&cargo,
Command::new(&cargo).args([
"build",
"-p",
"cargo-minimize",
"-p",
"testsuite",
"--bin",
"regression_checker",
"--bin",
"cargo-minimize",
]),
)
.context("running cargo build")?;

let this_file = Path::new(file!())
Expand Down Expand Up @@ -94,28 +100,28 @@ pub fn full_tests() -> Result<()> {
.map(|child| {
let path = child.path();

build(&path, &regression_checker_path)
build(&cargo, &path, &regression_checker_path)
.with_context(|| format!("building {:?}", path.file_name().unwrap()))
})
.collect::<Result<Vec<_>>>()?;
} else {
for child in children {
let path = child.path();

build(&path, &regression_checker_path)
build(&cargo, &path, &regression_checker_path)
.with_context(|| format!("building {:?}", path.file_name().unwrap()))?;
}
}

Ok(())
}

fn setup_dir(path: &Path) -> Result<(TempDir, PathBuf)> {
fn setup_dir(cargo: &Path, path: &Path) -> Result<(TempDir, PathBuf)> {
let tempdir = tempfile::tempdir()?;

let proj_name = path.file_name().unwrap().to_str().unwrap();
let proj_name = if let Some(proj_name) = proj_name.strip_suffix(".rs") {
let out = Command::new("cargo")
let out = Command::new(cargo)
.arg("new")
.arg(proj_name)
.current_dir(tempdir.path())
Expand Down Expand Up @@ -143,8 +149,8 @@ fn setup_dir(path: &Path) -> Result<(TempDir, PathBuf)> {
Ok((tempdir, proj_dir))
}

fn build(path: &Path, regression_checker_path: &Path) -> Result<()> {
let (_tempdir, proj_dir) = setup_dir(path).context("setting up tempdir")?;
fn build(cargo: &Path, path: &Path, regression_checker_path: &Path) -> Result<()> {
let (_tempdir, proj_dir) = setup_dir(cargo, path).context("setting up tempdir")?;
let mut cargo_minimize_path = PathBuf::from("target/debug/cargo-minimize");
if cfg!(windows) {
cargo_minimize_path.set_extension("exe");
Expand All @@ -168,6 +174,7 @@ fn build(path: &Path, regression_checker_path: &Path) -> Result<()> {
let minimize_roots = start_roots.join(",");

cmd.env("MINIMIZE_RUNTEST_ROOTS", &minimize_roots);
cmd.env("MINIMIZE_CARGO", cargo);

let out = cmd.output().context("spawning cargo-minimize")?;
let stderr = String::from_utf8(out.stderr).unwrap();
Expand Down

0 comments on commit 50dc094

Please sign in to comment.