Feature Request: wait and/or then methods after writing to stdin

[volks73]

I came across this crate recently, but I cannot remember where...maybe from the CLI-WG? Anyways, it provides a lot of the functionality I wrote in the past for testing some internal projects, so I am excited to start using it in the future, and add tests for some of my Rust CLI applications. Great Work!

I am currently working on a project where I am writing messages into stdin and reading from stdout. I have some tests that deliberately result in relatively long execution times (on the order of 10's of seconds). The general test follows: (1) write message to stdin, (2) wait some amount of time depending on expected processing time, (3) read stdout and check for correctness. For these tests, where a long execution time occurs, I have not found a way to use this crate. I believe the problem is that the stdin handle from the internal Command is being dropped after writing to stdin but before the command has had time to write to stdout, which leaves the contents of stdout from the wait_with_output method empty. Any correctness assertions on the output then fail.

Instead of using this crate, here is a non-working example of the test I implemented:

#[test]
fn slow_reply_works() {
    const RESPONSE_TIME: u64 = 10; // seconds
    let mut child = Command::new("some_cmd") // <-- Not actual command
        .stdin(Stdio::piped())
        .stdout(Stdio::piped())
        .spawn()
        .expect("Child command");
    {
        let stdin = child.stdin.as_mut().expect("Stdin");
        stdin.write_all("slowReplyMessage".as_bytes()).expect("Write to stdin");
        // Without this, the `stdin` handle is dropped. My CLI Under Test (CUT) exits when 
        // stdin reaches EOF or a Broken Pipe occurs. So, there is no time for
        // the cmd to complete the long/slow computation and write anything to stdout 
        // when the output is later retrieved with the `wait_with_output` method. The
        // output is empty and any assertion for correctness fails.
        thread::sleep(Duration::from_secs(RESPONSE_TIME));
    }
    let output = child.wait_with_output().expect("Wait on child");
    // Correctness assertions for the output follow here, but without the
    // `thread::sleep`, the output is empty. Omitted for clarity.
    // ...
}

I would like to see a wait, hold, or delay method added to the Assert struct as I believe the Assert struct is essentially doing the same time as my non-working example test, but does not have the thread::sleep line. If such a method existed, the above test could be rewritten more succinctly using this crate as follows:

#[test]
fn slow_reply_works() {
    const RESPONSE_TIME: u64 = 10; // seconds
    Assert::command(&["some_cmd"])
        .stdin("slowReplyMessage").wait(RESPONSE_TIME)
        .succeeds()
        .and()
        .stdout().contains("expected message after delay")
        .unwrap();
}

I believe this is cleaner and easier to read, but I understand wait might be confusing since there is the wait and wait_with_output methods for the Command struct in std::process. So, maybe hold or delay would be better, with a slight personal preference at the moment towards "hold"?

However, this got me thinking that maybe a wait/hold/delay method is too restrictive? There might be other tests in the future that need to do more than just wait for some time after writing to stdin. So, maybe a then method with a predicate would be better? The above example with the wait/hold/delay method could be re-implemented as follows:

#[test]
fn slow_reply_works() {
    const RESPONSE_TIME: u64 = 10; // seconds
    Assert::command(&["some_cmd"])
         // Not sure if anything needs to be passed to the closure, handle to stdin?
        .stdin("slowReplyMessage").then(|| { 
            // Waits for the slow reply to complete, just like the previous example, 
            // but other actions can be executed.
            thread::sleep(Duration::from_secs(RESPONSE_TIME);
        })
        .succeeds()
        .and()
        .stdout().contains("expected message after delay")
        .unwrap();
}

Of course, both a wait/hold/delay and then methods could be added because the then implementation of wait/hold/delay is a little more verbose/noisy. I am open to alternatives for the then method name as well.

Any thoughts or comments about this proposed feature/enhancement would be greatly appreciated.

Thanks again for the great crate and work!

[epage]

So the crux of the issue is this:

        // Without this, the `stdin` handle is dropped. My CLI Under Test (CUT) exits when 
        // stdin reaches EOF or a Broken Pipe occurs. So, there is no time for
        // the cmd to complete the long/slow computation and write anything to stdout 
        // when the output is later retrieved with the `wait_with_output` method. The
        // output is empty and any assertion for correctness fails.

So is Command sending an EOF once wait_with_output is called? I sure hope it isn't breaking the pipe; that'd be bad. I assume this command is doing RPC and doesn't know when it should wait for replies? This seems like an obscure use case which means I probably won't prioritize this among my different rust efforts. I'm also trying to decide how much benefit it would be to include in the API (the hard life of a maintainer, trying to decide when to say "no").

If we move forward with this, it seems the most ideal would be a wait_for(pipe, pred: IntoOutputPredicate) that reads from stdout or stderr and looks for specific content because waiting for timeouts is brittle at worst and slows your tests at best.

(IntoOutputPredicate would be a trait within assert_cli I'm planning on making when I integrate predicates-rust to make it easy for using a Predicate<[u8]> or Predicate<str>, maybe even accepting a &str though choosing the right default will be interesting).

[volks73]

So is Command sending an EOF once wait_with_output is called?

I believe so. According to the documentation for the wait_with_output method (emphasis mine):

The stdin handle to the child process, if any, will be closed before waiting. This helps avoid deadlock: it ensures that the child does not block waiting for input from the parent, while the parent waits for the child to exit.

The current implementation for my application is to exit on stdin EOF. Maybe I am missing something in my implementations to avoid this problem?

I assume this command is doing RPC and doesn't know when it should wait for replies?

You are correct. The CUT implements a RPC server over stdio and/or TCP depending on configuration, and it continuously reads from stdin and writes to stdout from two separate threads to mimic the independent, bi-directional communication of a TCP socket. I implemented RPC communication over stdio for embedding but also specifically for testing the application. If I use the TCP server, then I would potentially need hundreds, or possibly even thousands, of simultaneous TCP connections, or I would have to run the tests in serial as opposed to parallel (increasing test time). Using stdio communication, I can avoid running out of TCP connections for testing and still run in parallel. I know there are other ways to test RPCs without having to "spin up" a server for each one, but there are times when I want to test the whole process and have confidence the framing, (de)serialization, routing, etc. all work as expected.

This seems like an obscure use case...

Yes, after writing the above discussion and thinking about it some more, I agree that this is probably an edge case. I don't know if enough developers have encountered, or will encounter, this quirk when working with stdio and the Command type to warrant the expansion in the API of this crate and I appreciate the honesty and difficulty in saying "No". I just really liked the cleanliness, readability, and succinctness of my tests where I could use this crate and wanted to apply it to all of my tests.

So, I understand leaving out a wait/hold/delay method, but what about the more "generic" then and and_then methods for stdin? More broadly, implementing chaining for stdin? The API already has chaining for stdout and stderr. Something like:

#[test]
fn chaining_stdin_messages() {
    Assert::command(&["some_cmd"])
        // `then` would be a wrapper around `and_then` but, selfishly, the
        // `and_then` method would provide a mechanism to implement a delay for
        // my tests.
        .stdin("message1").then("message2").then("message3").and_then(|stdin| stdin.write_all("message4"))
        .succeeds()
        .and()
        .stdout().contains("expected message after delay")
        .unwrap();
}

An IntoInputPredicate similar to the IntoOutputPredicate trait could be provided/implemented. Right now, a test needs to compose the message or data into one big input before the test.

[epage]

Yes, after writing the above discussion and thinking about it some more, I agree that this is probably an edge case. I don't know if enough developers have encountered, or will encounter, this quirk when working with stdio and the Command type to warrant the expansion in the API of this crate and I appreciate the honesty and difficulty in saying "No". I just really liked the cleanliness, readability, and succinctness of my tests where I could use this crate and wanted to apply it to all of my tests.

To be clear, I'm not outright saying no, just wanting to better understand. Additionally, I'm grateful we at least have the issue so even if we close it, others might find it in the future.

So, I understand leaving out a wait/hold/delay method, but what about the more "generic" then and and_then methods for stdin? More broadly, implementing chaining for stdin? The API already has chaining for stdout and stderr. Something like:

That chaining is just combining predicates, its not doing much else. If there is a use case for chaining but right now, I'm not seeing it beyond your case.

btw would something like rexpect be useful? See https://crates.io/crates/rexpect

Something else to note, #98 is us experimenting with making the API more of a toolbox so you can create your own solutions with it.

[volks73]

I will take a look at rexpect, thanks for the recommendation.

In the mean time, I was experimenting the other day and came up with a change that keeps the public API usage unchanged, but allows "optional" access to the stdin handle of the child process. I thought I would share and created PR #107 that includes this experiment and some examples for the stdin method with the new implementation.

[volks73]

Okay, this issue/feature request and related PR (#107) can be closed. I have a solution for my use case using the assert_cmd crate that does not need any changes to this crate. Instead, I used StdInCommand and StdInCommandExt from the assert_cmd crate as templates and the changes in PR #107 to create InputCommand, InputCommandExt, and StdinWriter, which together adds the input and then methods to process::Command. I put all of this in a sub-module for my integration tests and used them with other features from the assert_cmd crate to create readable and compact tests for my CLI application and its examples.

The assert_cmd crate made it really easy to create a custom test fixture. I believe this is the intention of the crate. Thank you for the all of the work and information.

I have created a gist of my implementation for reference and in case anyone else is interested. I am open to comments on it.

[epage]

Glad the new assert_cmd is working out!