Skip to content

Commit

Permalink
Generated sys tests: Better command error handling
Browse files Browse the repository at this point in the history
The generated tests for `sys` crates sometimes call `Command::output`.
It is used to find CFLAGS arguments for tested packages and run the
executables created from `constant.c` and `layout.c` tests.
`Command::output`, by default, captures `stdout` and `stderr`.
This means that, by default, no feedback for why the tests fail will be
printed. The only information shown to the end-user is the command that
was ran.

You could copy/paste and run yourself the command printed in the error
message, but it might not be evident as a "next step" for users.

This is unlike the compilation command, which uses
`cmd.spawn()?.wait()?`. We can see compilation error messages, but not
pkg-config error messages.

To solve this, we:

1. Inherit `stderr` from the parent descriptor. This will print the
   command's error message in the terminal.
2. Add the command's `stdout` to the error message returned by
   `pkg_config_cflags` and `get_c_output`.

Now. Errors generated by the spawned process are visible to the end
user, which makes test errors more actionables.

The output is included for exhaustiveness. It may not be useful, but it
makes sense to give to the user all the information necessary.
  • Loading branch information
nicopap committed Aug 13, 2023
1 parent 76bbe9b commit a9190fb
Showing 1 changed file with 11 additions and 7 deletions.
18 changes: 11 additions & 7 deletions src/codegen/sys/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ fn generate_abi_rs(
writeln!(w, "use std::error::Error;")?;
writeln!(w, "use std::ffi::OsString;")?;
writeln!(w, "use std::path::Path;")?;
writeln!(w, "use std::process::Command;")?;
writeln!(w, "use std::process::{{Command, Stdio}};")?;
writeln!(w, "use std::str;")?;
writeln!(w, "use tempfile::Builder;")?;
writeln!(w)?;
Expand Down Expand Up @@ -381,9 +381,11 @@ fn pkg_config_cflags(packages: &[&str]) -> Result<Vec<String>, Box<dyn Error>> {
let mut cmd = Command::new(pkg_config);
cmd.arg("--cflags");
cmd.args(packages);
cmd.stderr(Stdio::inherit());
let out = cmd.output()?;
if !out.status.success() {
return Err(format!("command {cmd:?} returned {}", out.status).into());
let (status, stdout) = (out.status, str::from_utf8(&out.stdout)?);
return Err(format!("command {cmd:?} failed, {status:?}\nstdout: {stdout}").into());
}
let stdout = str::from_utf8(&out.stdout)?;
Ok(shell_words::split(stdout.trim())?)
Expand Down Expand Up @@ -502,13 +504,15 @@ fn get_c_output(name: &str) -> Result<String, Box<dyn Error>> {
let cc = Compiler::new().expect("configured compiler");
cc.compile(&c_file, &exe)?;
let mut abi_cmd = Command::new(exe);
let output = abi_cmd.output()?;
if !output.status.success() {
return Err(format!("command {abi_cmd:?} failed, {output:?}").into());
let mut cmd = Command::new(exe);
cmd.stderr(Stdio::inherit());
let out = cmd.output()?;
if !out.status.success() {
let (status, stdout) = (out.status, str::from_utf8(&out.stdout)?);
return Err(format!("command {cmd:?} failed, {status:?}\nstdout: {stdout}").into());
}
Ok(String::from_utf8(output.stdout)?)
Ok(String::from_utf8(out.stdout)?)
}
const RUST_LAYOUTS: &[(&str, Layout)] = &["####
Expand Down

0 comments on commit a9190fb

Please sign in to comment.