Add optional access to Command's stdin handle #107

volks73 · 2018-05-18T22:24:22Z

I was experimenting and I thought I should share some changes based on the discussion in #106.

Key Changes:

Added the InputPredicate trait to provide access to Command's stdin handle
Added implementations of the InputPredicate trait to leave public API unchanged and maintain compatibility
Added doc-tests/examples to the stdin method
Added the Assert::example constructor (Feature proposal: Run examples as top level concept, with compilation profile #96)

I also tested these changes with one of my other projects. The non-working example in #106 becomes with this change:

#[test]
fn slow_reply_works() {
    const RESPONSE_TIME: u64 = 10; // seconds
    Assert::example("an_example")
        .stdin("slowReplyMessage")
        .stdin(Wait(RESPONSE_TIME))
        .succeeds()
        .and()
        .stdout().is("expected message")
        .unwrap();
}

where Wait is defined elsewhere and it is a tuple struct that implements the InputPredicate trait.

I have created tests for any new feature, or added regression tests for bugfixes.
cargo test succeeds
Clippy is happy: cargo +nightly clippy succeeds
Rustfmt is happy: cargo +nightly fmt succeeds

Add access to Command's stdin handle Add InputPredicate type for stdin This trait allows for the computation of the contents to be written to stdin using the `stdin` method of the `Assert` type. A series of `From<T>` implementations are added for the trait so that there is no functionality change to the `stdin` method. By changing to the trait, a "handle" is provided for a user to access stdin during a test. There is no functionality change to the public API, but the `stdin` method could be used in a chain to write content to stdin over time. This can be useful when testing the functionality of a CLI application with streaming input. Add `stdin_contents` field to debug implementation Change lifetimes for trait implementation The 'static lifetime requirement is changed to 'a for the From<&str> implementation of the InputPredicate type. This makes its usage a little more flexible. Add example with a delay An example of composing stdin content over time as a stream to the CLI application under test is added to the `stdin` method. Fix tests Add documentation to `InputPredicate` trait Remove excessive whitespace Add some documentation to explain examples Add `From` implementation A `From<&[u8]>` implementation was missing for the `InputPredicate` trait to avoid a regression. Add export of `InputPredicate` to public API This is needed so users can define types that implement the trait in their tests. Change `From` trait bounds to be generic After some usage with creating another test, types that implemented the `InputPredicate` trait also needed to implement the `From` trait for the `Box<InputPredicate>` to work. So, the built-in `From` trait to create a boxed `InputPredicate` is changed to be more generic, so (maybe?) the conversion trait does not need to be implement for all types. Refactor formatting for consistency Add example of using type An example is added for the `stdin` method that demonstrates how to define an `InputPredicate` implementation using a unit struct instead of a closure. This improves code re-use and arguably readability/fluency. Add `example` constructor The `example` constructor is added to the `Assert` struct as a helper method to run an example from the crate instead of having to use the `command` constructor. This is similar to the `cargo_binary` but uses the `--example` option instead of the `--bin` option. Refactor formatting with rustfmt Fix comment grammer Refactor formatting Add `stdin_contents` field to debug implementation Fix tests Add documentation to `InputPredicate` trait Remove excessive whitespace Add export of `InputPredicate` to public API This is needed so users can define types that implement the trait in their tests. Refactor formatting for consistency Add `example` constructor The `example` constructor is added to the `Assert` struct as a helper method to run an example from the crate instead of having to use the `command` constructor. This is similar to the `cargo_binary` but uses the `--example` option instead of the `--bin` option. Refactor formatting with rustfmt Fix comment grammer Refactor formatting

epage

So in #106 I mentioned

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").

This addresses me not implementing it but I'm still not sure about it

Seems a bit obscure of a use case
Contemplating how well it will fit with the new API. Ideally we'll have that in by the end of the month.

In general, this use case seems better served by a command/response approach.

epage · 2018-05-20T03:03:39Z

src/assert.rs

+    /// Run a specific example of the current crate.
+    ///
+    /// Defaults to asserting _successful_ execution.
+    pub fn example<S: AsRef<OsStr>>(name: S) -> Self {


From commit message

The example constructor is added to the Assert struct as a helper
method to run an example from the crate instead of having to use the
command constructor. This is similar to the cargo_binary but uses
the --example option instead of the --bin option.

Could you split this out into a dedicated PR so we can address these topics separately?

epage · 2018-05-20T03:11:19Z

src/assert.rs

@@ -529,6 +647,52 @@ impl OutputAssertionBuilder {
    }
 }

+/// A type for writing to stdin during a test.
+pub trait InputPredicate {


When we redo the API to use the predicates crate, this name will become confusing. Maybe StdinWriter?

I'd say we could reuse the Write trait but that'd be annoying with providing trait implementations. Maybe we don't make it so automatic but provide a helper content factory or something?

The `InputPredicate` trait is renamed to `StdinWriter` to avoid confusion and naming conflicts with upcoming API changes and coming usage refactoring to use the `predicate` crate upstream.

volks73 · 2018-05-24T22:01:55Z

This addresses me not implementing it but I'm still not sure about it

Seems a bit obscure of a use case

While I agree the origin of this PR and my initial issue submission (#106) is an edge case and somewhat out of the blue with a more common usage to read stdin as a block data in one shot, like from file redirection, streaming data to and from a CLI application via pipes is a common practice. The current API, and after an admittedly quick review of the forthcoming new API (#98), only supports writing single blocks of data to a CLI application, yet stdin is a stream where data can be written to it over time. I thought these changes would be a nice way to extend functionality to test the streaming nature of stdin if needed while still maintaining the simplicity and elegance that makes this crate attractive for testing CLI applications.

Contemplating how well it will fit with the new API. Ideally we'll have that in by the end of the month.

If the new API is intended to be a toolkit for creating CLI test frameworks, then I would appreciate some way to test the stream nature of stdin in the new API and/or supply a mechanism to expose the stdin handle from Command. I would be happy to spend some time on investigating this for the new API.

In general, this use case seems better served by a command/response approach.

Can you expand on this a little more? I do not know what you are referring to. I was under the impression that the current API for this crate is a command/response approach.

Maybe StdinWriter?

This is more descriptive of its usage and works for me.

epage · 2018-05-29T12:32:52Z

Could you look at https://github.com/assert-rs/assert_cmd and see how this new API works for your needs?

Note: assert_cmd is basically what assert_cli is today. I think killercup wants to take assert_cli and turn it into a higher level, more opinionated crate.

volks73 · 2018-05-29T14:23:31Z

Could you look at https://github.com/assert-rs/assert_cmd and see how this new API works for your needs?

Yes. I will take a look, thank you.

volks73 mentioned this pull request May 18, 2018

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

Closed

epage suggested changes May 20, 2018

View reviewed changes

volks73 mentioned this pull request May 21, 2018

Add example constructor for Assert #108

Merged

4 tasks

volks73 added 2 commits May 20, 2018 22:04

Rename InputPredicate to StdinWriter`

c3a13f5

The `InputPredicate` trait is renamed to `StdinWriter` to avoid confusion and naming conflicts with upcoming API changes and coming usage refactoring to use the `predicate` crate upstream.

Merge remote-tracking branch 'upstream/master'

1d63c7c

Merge remote-tracking branch 'upstream/master'

c9587f6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional access to Command's stdin handle #107

Add optional access to Command's stdin handle #107

volks73 commented May 18, 2018

epage left a comment

epage May 20, 2018

epage May 20, 2018

volks73 commented May 24, 2018

epage commented May 29, 2018

volks73 commented May 29, 2018

Add optional access to Command's stdin handle #107

Are you sure you want to change the base?

Add optional access to Command's stdin handle #107

Conversation

volks73 commented May 18, 2018

epage left a comment

Choose a reason for hiding this comment

epage May 20, 2018

Choose a reason for hiding this comment

epage May 20, 2018

Choose a reason for hiding this comment

volks73 commented May 24, 2018

epage commented May 29, 2018

volks73 commented May 29, 2018