Better error message when changing to a directory that doesn't exist

[jyn514]

Currently, the error looks like this:

thread 'reports_broken_links' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', /home/joshua/.local/lib/cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/assert_cmd-1.0.1/src/assert.rs:59:36

The actual issue is that I tried to change to a directory that doesn't exist:

	Command::cargo_bin("cargo-deadlinks")
		.unwrap()
		.arg("deadlinks")
		.current_dir("./tests/broken_links")  // this directory does not exist

This is made worse by the backtrace, which points to the wrong line:

   3: core::result::Result<T,E>::unwrap
             at /home/joshua/.local/lib/rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/result.rs:973
   4: <&mut std::process::Command as assert_cmd::assert::OutputAssertExt>::assert
             at /home/joshua/.local/lib/cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/assert_cmd-1.0.1/src/assert.rs:59
   5: broken_links::reports_broken_links
             at ./tests/broken_links.rs:9  // this is `Command::cargo_bin`, not `current_dir`

[epage]

at ./tests/broken_links.rs:9 // this is Command::cargo_bin, not current_dir

Pure speculation: the panic doesn't happen in current_dir but when you call asset(). Since its a single expression, the backtrace points to the first line of the expression

Using the new #[track_caller] would help in that it'd point to your code, rather than assert_cmd code, but that isn't available for trait items and we're using extension traits.

Not sure what there is that we could do at this point. Any ideas?

[jyn514]

It looks like the panic comes from here:

assert_cmd/src/assert.rs

Line 60 in b4d4beb

let output = self.output().unwrap();

I think if you removed the unwrap() and returned an error instead it would be more helpful, or if you want to avoid changing the API you could panic with 'failed to run the command' or something like that.

[epage]

I think if you removed the unwrap() and returned an error instead it would be more helpful,

The caller is wanting to assert on the result of the run, so panicing seems appropriate there. If caller needs access to the error, they should explicitly call .output().

However, you are right that we should have a custom panic! in there rather than just an unwrap.

[epage]

1.0.2 should now be released with a proper panic message