Check for output()?.status.success() when calling a binary where need…

…ed (#1915)

<!-- Reference any GitHub issues resolved by this PR -->

Closes #1866

## Introduced changes

<!-- A brief description of the changes -->

- Check for output()?.status.success() when calling a binary where
needed

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`

crates/forge-runner/src/profiler_api.rs

@@ -1,10 +1,11 @@
-use anyhow::{anyhow, Context, Result};
+use anyhow::{Context, Result};
+use shared::command::CommandExt;
 use std::process::Stdio;
 use std::{env, fs, path::PathBuf, process::Command};
 
 pub const PROFILE_DIR: &str = "profile";
 
-pub fn run_profiler(test_name: &String, trace_path: &PathBuf) -> Result<()> {
+pub fn run_profiler(test_name: &str, trace_path: &PathBuf) -> Result<()> {
  let profiler = env::var("CAIRO_PROFILER")
  .map(PathBuf::from)
  .ok()
@@ -13,20 +14,14 @@ pub fn run_profiler(test_name: &String, trace_path: &PathBuf) -> Result<()> {
  fs::create_dir_all(&dir_to_save_profile).context("Failed to create a profile dir")?;
  let path_to_save_profile = dir_to_save_profile.join(format!("{test_name}.pb.gz"));
 
- let mut cmd = Command::new(profiler);
- cmd.arg(trace_path);
- cmd.arg("--output-path");
- cmd.arg(&path_to_save_profile);
- cmd.stdout(Stdio::inherit());
- cmd.stderr(Stdio::inherit());
+ Command::new(profiler)
+ .arg(trace_path)
+ .arg("--output-path")
+ .arg(&path_to_save_profile)
+ .stdout(Stdio::inherit())
+ .stderr(Stdio::inherit())
+ .output_checked()
+ .with_context(||format!("cairo-profiler failed to generate the profile for test {test_name} - inspect the errors above for more info"))?;
 
- let exit_status = cmd.output().context("Failed to run cairo-profiler")?.status;
-
- if exit_status.success() {
- Ok(())
- } else {
- Err(anyhow!(
- "cairo-profiler failed to generate the profile for test {test_name} - inspect the errors above for more info"
- ))
- }
+ Ok(())
 }

crates/forge/test_utils/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2021"
 
 [dependencies]
 anyhow.workspace = true
+shared.workspace = true
 blockifier.workspace = true
 assert_fs.workspace = true
 camino.workspace = true

crates/forge/test_utils/src/runner.rs

@@ -1,5 +1,5 @@
 use crate::tempdir_with_tool_versions;
-use anyhow::{anyhow, bail, Context, Result};
+use anyhow::{anyhow, Context, Result};
 use assert_fs::{
  fixture::{FileTouch, FileWriteStr, PathChild},
  TempDir,
@@ -14,6 +14,7 @@ use indoc::formatdoc;
 use scarb_api::{
  get_contracts_map, metadata::MetadataCommandExt, ScarbCommand, StarknetContractArtifacts,
 };
+use shared::command::CommandExt;
 use std::{
  collections::HashMap,
  fs,
@@ -76,16 +77,13 @@ impl Contract {
  ))
  .unwrap();
 
- let build_output = Command::new("scarb")
+ Command::new("scarb")
  .current_dir(&dir)
  .arg("build")
  .stdout(Stdio::inherit())
  .stderr(Stdio::inherit())
- .output()
+ .output_checked()
  .context("Failed to build contracts with Scarb")?;
- if !build_output.status.success() {
- bail!("scarb build did not succeed")
- }
 
  let scarb_metadata = ScarbCommand::metadata()
  .current_dir(dir.path())

crates/forge/test_utils/src/running_tests.rs

@@ -6,6 +6,7 @@ use forge::test_filter::TestsFilter;
 use forge_runner::contracts_data::ContractsData;
 use forge_runner::test_crate_summary::TestCrateSummary;
 use forge_runner::{RunnerConfig, RunnerParams};
+use shared::command::CommandExt;
 use std::path::PathBuf;
 use std::process::Command;
 use std::process::Stdio;
@@ -15,14 +16,13 @@ use tokio::runtime::Runtime;
 
 #[must_use]
 pub fn run_test_case(test: &TestCase) -> Vec<TestCrateSummary> {
- let test_build_output = Command::new("scarb")
+ Command::new("scarb")
  .current_dir(test.path().unwrap())
  .arg("snforge-test-collector")
  .stdout(Stdio::inherit())
  .stderr(Stdio::inherit())
- .output()
+ .output_checked()
  .unwrap();
- assert!(test_build_output.status.success());
 
  let rt = Runtime::new().expect("Could not instantiate Runtime");
 

crates/forge/tests/e2e/common/runner.rs

@@ -2,6 +2,7 @@ use assert_fs::fixture::{FileWriteStr, PathChild, PathCopy};
 use assert_fs::TempDir;
 use camino::Utf8PathBuf;
 use indoc::formatdoc;
+use shared::command::CommandExt;
 use snapbox::cmd::{cargo_bin, Command as SnapboxCommand};
 use std::process::Command;
 use std::str::FromStr;
@@ -176,7 +177,7 @@ pub(crate) fn get_remote_url() -> String {
  } else {
  let output = Command::new("git")
  .args(["remote", "get-url", "origin"])
- .output()
+ .output_checked()
  .unwrap();
 
  String::from_utf8(output.stdout)
@@ -197,7 +198,7 @@ pub(crate) fn get_current_branch() -> String {
  } else {
  let output = Command::new("git")
  .args(["rev-parse", "--abbrev-ref", "HEAD"])
- .output()
+ .output_checked()
  .unwrap();
 
  String::from_utf8(output.stdout).unwrap().trim().to_string()

crates/forge/tests/integration/setup_fork.rs

@@ -16,6 +16,7 @@ use tokio::runtime::Runtime;
 
 use forge::compiled_raw::RawForkParams;
 use forge_runner::{RunnerConfig, RunnerParams};
+use shared::command::CommandExt;
 use test_utils::runner::{assert_case_output_contains, assert_failed, assert_passed, Contract};
 use test_utils::running_tests::run_test_case;
 use test_utils::test_case;
@@ -106,14 +107,13 @@ fn fork_aliased_decorator() {
 
  let rt = Runtime::new().expect("Could not instantiate Runtime");
 
- let test_build_output = Command::new("scarb")
+ Command::new("scarb")
  .current_dir(test.path().unwrap())
  .arg("snforge-test-collector")
  .stdout(Stdio::inherit())
  .stderr(Stdio::inherit())
- .output()
+ .output_checked()
  .unwrap();
- assert!(test_build_output.status.success());
 
  let result = rt
  .block_on(run(

crates/scarb-api/Cargo.toml

@@ -5,6 +5,7 @@ edition.workspace = true
 
 [dependencies]
 anyhow.workspace = true
+shared.workspace = true
 camino.workspace = true
 scarb-metadata.workspace = true
 scarb-ui.workspace = true

crates/scarb-api/src/version.rs

@@ -2,6 +2,7 @@ use crate::ScarbCommand;
 use anyhow::{Context, Result};
 use regex::Regex;
 use semver::Version;
+use shared::command::CommandExt;
 use std::str::from_utf8;
 
 pub struct ScarbVersionOutput {
@@ -17,7 +18,7 @@ impl VersionCommand {
  let scarb_version = ScarbCommand::new()
  .arg("--version")
  .command()
- .output()
+ .output_checked()
  .context("Failed to execute `scarb --version`")?;
 
  let version_output = from_utf8(&scarb_version.stdout)

crates/shared/src/command.rs

@@ -0,0 +1,22 @@
+use anyhow::{bail, Context, Ok, Result};
+use std::process::{Command, Output};
+
+pub trait CommandExt {
+ fn output_checked(&mut self) -> Result<Output>;
+}
+
+impl CommandExt for Command {
+ fn output_checked(&mut self) -> Result<Output> {
+ let command = self.get_program().to_string_lossy().to_string();
+
+ let output = self
+ .output()
+ .with_context(|| format!("Failed to run {command}"))?;
+
+ if !output.status.success() {
+ bail!("Command {command} failed with status {}", output.status);
+ }
+
+ Ok(output)
+ }
+}

crates/shared/src/lib.rs

@@ -5,6 +5,7 @@ use anyhow::{anyhow, Result};
 use starknet::providers::jsonrpc::HttpTransport;
 use starknet::providers::JsonRpcClient;
 
+pub mod command;
 pub mod consts;
 pub mod print;
 pub mod rpc;

crates/sncast/src/helpers/scarb_utils.rs

@@ -6,7 +6,7 @@ use scarb_api::{
  ScarbCommand, ScarbCommandError, StarknetContractArtifacts,
 };
 use scarb_ui::args::PackagesFilter;
-use shared::print::print_as_warning;
+use shared::{command::CommandExt, print::print_as_warning};
 use std::collections::HashMap;
 use std::env;
 use std::str::FromStr;
@@ -22,7 +22,7 @@ pub fn get_scarb_manifest_for(dir: &Utf8Path) -> Result<Utf8PathBuf> {
  .current_dir(dir)
  .arg("manifest-path")
  .command()
- .output()
+ .output_checked()
  .context("Failed to execute the `scarb manifest-path` command")?;
 
  let output_str = String::from_utf8(output.stdout)

crates/universal-sierra-compiler-api/Cargo.toml

@@ -5,6 +5,7 @@ edition.workspace = true
 
 [dependencies]
 anyhow.workspace = true
+shared.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 which.workspace = true

crates/universal-sierra-compiler-api/src/lib.rs

@@ -11,6 +11,7 @@ use std::str::from_utf8;
 use tempfile::Builder;
 
 pub use command::*;
+use shared::command::CommandExt;
 
 mod command;
 
@@ -77,7 +78,7 @@ pub fn compile_sierra_at_path(
  sierra_file_path,
  ])
  .command()
- .output()
+ .output_checked()
  .context(
  "Error while compiling Sierra. \
  Make sure you have the latest universal-sierra-compiler binary installed. \