Skip to content

Commit

Permalink
Deduplicate abort checking after running a command
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Dec 4, 2024
1 parent c0ff3b3 commit bf517eb
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 29 deletions.
7 changes: 2 additions & 5 deletions src/aux_builds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use crate::{
build_manager::{Build, BuildManager},
core::run_command,
custom_flags::Flag,
default_per_file_config, display,
per_test_config::{Comments, TestConfig},
Expand Down Expand Up @@ -123,7 +122,7 @@ impl Build for AuxBuilder {

aux_cmd.arg("--emit=link");
let filename = self.aux_file.file_stem().unwrap().to_str().unwrap();
let output = run_command(&mut aux_cmd)?;
let output = config.config.run_command(&mut aux_cmd)?;
if !output.status.success() {
let error = Error::Command {
kind: "compilation of aux build failed".to_string(),
Expand All @@ -139,9 +138,7 @@ impl Build for AuxBuilder {

// Now run the command again to fetch the output filenames
aux_cmd.arg("--print").arg("file-names");
let output = run_command(&mut aux_cmd)?;

config.aborted()?;
let output = config.config.run_command(&mut aux_cmd)?;

assert!(output.status.success());

Expand Down
2 changes: 1 addition & 1 deletion src/build_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub trait Build {
pub struct BuildManager {
#[allow(clippy::type_complexity)]
cache: RwLock<HashMap<String, Arc<OnceLock<Result<Vec<OsString>, ()>>>>>,
config: Config,
pub(crate) config: Config,
new_job_submitter: Sender<NewJob>,
}

Expand Down
16 changes: 16 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::{
collections::BTreeMap,
num::NonZeroUsize,
path::{Path, PathBuf},
process::{Command, Output},
sync::{atomic::AtomicBool, Arc},
};

Expand Down Expand Up @@ -461,6 +462,21 @@ impl Config {
Ok(())
}
}

pub(crate) fn run_command(&self, cmd: &mut Command) -> Result<Output, Errored> {
self.aborted()?;

let output = cmd.output().map_err(|err| Errored {
errors: vec![],
stderr: err.to_string().into_bytes(),
stdout: format!("could not spawn `{:?}` as a process", cmd.get_program()).into_bytes(),
command: format!("{cmd:?}"),
})?;

self.aborted()?;

Ok(output)
}
}

/// Fail the test when mismatches are found, if provided the command string
Expand Down
15 changes: 0 additions & 15 deletions src/core.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
//! Basic operations useful for building a testsuite
use crate::test_result::Errored;
use color_eyre::eyre::Result;
use crossbeam_channel::unbounded;
use crossbeam_channel::Receiver;
Expand All @@ -10,23 +9,9 @@ use std::num::NonZeroUsize;
use std::path::Component;
use std::path::Path;
use std::path::Prefix;
use std::process::Command;
use std::process::Output;
use std::sync::OnceLock;
use std::thread;

pub(crate) fn run_command(cmd: &mut Command) -> Result<Output, Errored> {
match cmd.output() {
Err(err) => Err(Errored {
errors: vec![],
stderr: err.to_string().into_bytes(),
stdout: format!("could not spawn `{:?}` as a process", cmd.get_program()).into_bytes(),
command: format!("{cmd:?}"),
}),
Ok(output) => Ok(output),
}
}

/// Remove the common prefix of this path and the `root_dir`.
pub(crate) fn strip_path_prefix<'a>(
path: &'a Path,
Expand Down
7 changes: 3 additions & 4 deletions src/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use crate::{
build_manager::{Build, BuildManager},
core::run_command,
custom_flags::Flag,
per_test_config::TestConfig,
test_result::Errored,
Expand Down Expand Up @@ -38,7 +37,7 @@ fn cfgs(config: &Config) -> Result<Vec<Cfg>, Errored> {
let mut cmd = config.program.build(&config.out_dir);
cmd.arg(cfg);
cmd.arg("--target").arg(config.target.as_ref().unwrap());
let output = run_command(&mut cmd)?;
let output = config.run_command(&mut cmd)?;

if !output.status.success() {
return Err(Errored {
Expand Down Expand Up @@ -97,7 +96,7 @@ fn build_dependencies_inner(

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

let output = run_command(&mut build)?;
let output = config.run_command(&mut build)?;

if !output.status.success() {
let stdout = output
Expand Down Expand Up @@ -193,7 +192,7 @@ fn build_dependencies_inner(
.arg(&info.crate_manifest_path);
info.program.apply_env(&mut metadata);
set_locking(&mut metadata);
let output = run_command(&mut metadata)?;
let output = config.run_command(&mut metadata)?;

if !output.status.success() {
return Err(Errored {
Expand Down
5 changes: 1 addition & 4 deletions src/per_test_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,7 @@ impl TestConfig {
cmd.stdin(std::fs::File::open(stdin).unwrap());
}

let output = crate::core::run_command(&mut cmd)?;

// Do not bless aborted tests
self.aborted()?;
let output = build_manager.config.run_command(&mut cmd)?;

let output = self.check_test_result(&cmd, output)?;

Expand Down

0 comments on commit bf517eb

Please sign in to comment.