Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the Debug implementation of Command #142

Closed
nwalfield opened this issue Jan 13, 2022 · 1 comment · Fixed by #143
Closed

Improve the Debug implementation of Command #142

nwalfield opened this issue Jan 13, 2022 · 1 comment · Fixed by #143

Comments

@nwalfield
Copy link

In my project, I build up a command to run dynamically. For instance, I have a test where I check that it is possible to specify the input on the command line or via stdin. It looks something like this:

for stdin in [false, true] {
        let mut cmd = Command::cargo_bin("sq")?;
        cmd.args(&["revoke",
                   "certificate",
                   "compromised", "oh no!"]);

        if let Some(dir) = tmp_dir.as_ref() {
            let alice_pgp = dir.path().join("alice.pgp");
            let mut file = File::create(&alice_pgp)?;
            file.write_all(&alice_tsk)?;

            cmd.args(&["--certificate", &*alice_pgp.to_string_lossy()]);
        } else {
            cmd.write_stdin(alice_tsk);
        }
}

I wanted to print out what is going to be run, so I added: eprintln!("Running: {:?}", cmd);. That prints:

Running: Command { cmd: "/tmp/sequoia-build/debug/sq" "revoke" "certificate" "compromised" "xyzzy" "--certificate" "/tmp/.tmpSZuU4h/alice.pgp", stdin: None, timeout: None }
Running: Command { cmd: "/tmp/sequoia-build/debug/sq" "revoke" "certificate" "compromised" "xyzzy", stdin: Some([197, 88, 4, 97, 224, 39, 221, 22, 9, 43, 6, 1, 4, 1, 218, 71, 15, 1, 1, 7, 64, 10, 134, ... pages and pages of numbers ...

In the case where I feed the data via stdin, that is very noisy. (I would have been happy to just print the command, but cmd is private and I didn't find a getter. Perhaps it makes sense to add one.) I think it would be better to do something like the following:

  • If stdin is, say, only printable ascii characters print it as such
  • Otherwise, print it as nicely formatted hex similar to xxd:
$ echo foobar | xxd 
00000000: 666f 6f62 6172 0a                        foobar.

Thanks for the great project!

epage added a commit to epage/assert_cmd that referenced this issue Jan 13, 2022
@nwalfield
Copy link
Author

Thanks a lot for the quick response!

I just tried it, and it works much better. It's not great for binary data:

 stdin: Some("\xC5X\x04a\xE0p\x97\x16\t+\x06\x01\x04\x01\xDAG\x0f\x01\x01\x07@\xD1\xDBH\xB...

but it's a definite improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant