Skip to content

Commit

Permalink
Make errors from make_test_list legible (#1228)
Browse files Browse the repository at this point in the history
In debugging <sourcefrog/cargo-mutants#226> I found that invocations like `~/.rustup/toolchains/beta-x86_64-unknown-linux-gnu/bin/cargo nextest run` cause some test failures in the nextest tree, for reasons I'm still working to understand. 

In `main`, those errors are printed like this, with a pile of ASCII byte values:

```
thread 'basic::test_termination' panicked at nextest-runner/tests/integration/fixtures.rs:383:10:
test list successfully created: CommandFail { binary_id: RustBinaryId("cdylib-example"), command: ["/home/mbp/src/nextest/fixtures/nextest-tests/target/debug/deps/cdylib_example-9222c584dde9dbd0", "--list", "--format", "terse"], exit_status: ExitStatus(unix_wait_status(32512)), stdout: [], stderr: [47, 104, 111, 109, 101, 47, 109, 98, 112, 47, 115, 114, 99, 47, 110, 101, 120, 116, 101, 115, 116, 47, 102, 105, 120, 116, 117, 114, 101, 115, 47, 110, 101, 120, 116, 101, 115, 116, 45, 116, 101, 115, 116, 115, 47, 116, 97, 114, 103, 101, 116, 47, 100, 101, 98, 117, 103, 47, 100, 101, 112, 115, 47, 99, 100, 121, 108, 105, 98, 95, 101, 120, 97, 109, 112, 108, 101, 45, 57, 50, 50, 50, 99, 53, 56, 52, 100, 100, 101, 57, 100, 98, 100, 48, 58, 32, 101, 114, 114, 111, 114, 32, 119, 104, 105, 108, 101, 32, 108, 111, 97, 100, 105, 110, 103, 32, 115, 104, 97, 114, 101, 100, 32, 108, 105, 98, 114, 97, 114, 105, 101, 115, 58, 32, 108, 105, 98, 116, 101, 115, 116, 45, 99, 98, 97, 98, 55, 49, 54, 57, 55, 100, 102, 57, 55, 98, 48, 48, 46, 115, 111, 58, 32, 99, 97, 110, 110, 111, 116, 32, 111, 112, 101, 110, 32, 115, 104, 97, 114, 101, 100, 32, 111, 98, 106, 101, 99, 116, 32, 102, 105, 108, 101, 58, 32, 78, 111, 32, 115, 117, 99, 104, 32, 102, 105, 108, 101, 32, 111, 114, 32, 100, 105, 114, 101, 99, 116, 111, 114, 121, 10] }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```

This is because `unwrap` prints the `Debug` form and the error holds a `Vec<u8>`. 

There could be a few ways to change this but it seemed like the intention of the existing code was to keep the exact output in the error enum as bytes, and only do lossy conversion to UTF-8 from Display. So, I looked for a way to get the Display form printed, and it seemed like a good way was to just return the error, since the tests that call this already themselves return Results.

With this applied the failures are much more helpful:

```
error: creating test list failed

Caused by:
  for `cdylib-example`, command `/tmp/nextest-fixtureQWK72c/src/target/debug/deps/cdylib_example-9222c584dde9dbd0 --list --format terse` exited with code 127
--- stdout:

--- stderr:
/tmp/nextest-fixtureQWK72c/src/target/debug/deps/cdylib_example-9222c584dde9dbd0: error while loading shared libraries: libtest-cbab71697df97b00.so: cannot open shared object file: No such file or directory

---

```
  • Loading branch information
sourcefrog authored Jan 16, 2024
1 parent 1e17c09 commit cfd19f6
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 15 deletions.
4 changes: 2 additions & 2 deletions nextest-runner/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,11 +454,11 @@ impl fmt::Display for PartitionerBuilderParseError {
}
}

/// An error that occures while operating on a
/// An error that occurs while operating on a
/// [`TestFilterBuilder`](crate::test_filter::TestFilterBuilder).
#[derive(Clone, Debug, Error)]
pub enum TestFilterBuilderError {
/// An error that occured while constructing test filters.
/// An error that occurred while constructing test filters.
#[error("error constructing test filters")]
Construct {
/// The underlying error.
Expand Down
16 changes: 8 additions & 8 deletions nextest-runner/tests/integration/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fn test_list_tests() -> Result<()> {
set_env_vars();

let test_filter = TestFilterBuilder::any(RunIgnored::Default);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
let mut summary = test_list.to_summary();

for (name, expected) in &*EXPECTED_TESTS {
Expand Down Expand Up @@ -90,7 +90,7 @@ fn test_run() -> Result<()> {
set_env_vars();

let test_filter = TestFilterBuilder::any(RunIgnored::Default);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
let config = load_config();
let profile = config
.profile(NextestConfig::DEFAULT_PROFILE)
Expand Down Expand Up @@ -198,7 +198,7 @@ fn test_run_ignored() -> Result<()> {
vec![expr],
)
.unwrap();
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
let config = load_config();
let profile = config
.profile(NextestConfig::DEFAULT_PROFILE)
Expand Down Expand Up @@ -275,7 +275,7 @@ fn test_filter_expr_with_string_filters() -> Result<()> {
vec![expr],
)
.unwrap();
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
for test in test_list.iter_tests() {
if test.name == "tests::call_dylib_add_two" {
assert!(
Expand Down Expand Up @@ -334,7 +334,7 @@ fn test_filter_expr_without_string_filters() -> Result<()> {
let test_filter =
TestFilterBuilder::new(RunIgnored::Default, None, Vec::<String>::new(), vec![expr])
.unwrap();
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
for test in test_list.iter_tests() {
if test.name.contains("test_multiply_two") || test.name == "tests::call_dylib_add_two" {
assert!(
Expand Down Expand Up @@ -363,7 +363,7 @@ fn test_string_filters_without_filter_expr() -> Result<()> {
vec![],
)
.unwrap();
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
for test in test_list.iter_tests() {
if test.name.contains("test_multiply_two")
|| test.name.contains("tests::call_dylib_add_two")
Expand Down Expand Up @@ -395,7 +395,7 @@ fn test_retries(retries: Option<RetryPolicy>) -> Result<()> {
set_env_vars();

let test_filter = TestFilterBuilder::any(RunIgnored::Default);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
let config = load_config();
let profile = config
.profile("with-retries")
Expand Down Expand Up @@ -544,7 +544,7 @@ fn test_termination() -> Result<()> {
)
.unwrap();

let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;
let config = load_config();
let profile = config
.profile("with-termination")
Expand Down
5 changes: 3 additions & 2 deletions nextest-runner/tests/integration/fixtures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: MIT OR Apache-2.0

use camino::{Utf8Path, Utf8PathBuf};
use color_eyre::eyre::{Context, Result};
use duct::cmd;
use guppy::{graph::PackageGraph, MetadataCommand};
use maplit::{btreemap, btreeset};
Expand Down Expand Up @@ -363,7 +364,7 @@ impl FixtureTargets {
&self,
test_filter: &TestFilterBuilder,
target_runner: &TargetRunner,
) -> TestList<'_> {
) -> Result<TestList<'_>> {
let test_bins: Vec<_> = self.test_artifacts.values().cloned().collect();
let double_spawn = DoubleSpawnInfo::disabled();
let ctx = TestExecuteContext {
Expand All @@ -380,7 +381,7 @@ impl FixtureTargets {
self.env.to_owned(),
get_num_cpus(),
)
.expect("test list successfully created")
.context("Failed to make test list")
}
}

Expand Down
6 changes: 3 additions & 3 deletions nextest-runner/tests/integration/target_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ fn test_listing_with_target_runner() -> Result<()> {
set_env_vars();

let test_filter = TestFilterBuilder::any(RunIgnored::Default);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty());
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &TargetRunner::empty())?;

let bin_count = test_list.binary_count();
let test_count = test_list.test_count();
Expand All @@ -174,7 +174,7 @@ fn test_listing_with_target_runner() -> Result<()> {
);
let (_, target_runner) = runner_for_target(None).unwrap();

let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &target_runner);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &target_runner)?;

assert_eq!(bin_count, test_list.binary_count());
assert_eq!(test_count, test_list.test_count());
Expand Down Expand Up @@ -207,7 +207,7 @@ fn test_run_with_target_runner() -> Result<()> {
assert_eq!(passthrough_path(), runner.binary());
}

let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &target_runner);
let test_list = FIXTURE_TARGETS.make_test_list(&test_filter, &target_runner)?;

let config = load_config();
let profile = config
Expand Down

0 comments on commit cfd19f6

Please sign in to comment.