WIP: Free fns for Output constructors and add prelude #75
Conversation
rustfmt will split long builers across lines, even when that breaks
logical grouping.
For example
```rust
assert_cli::Assert::command(&["ls", "foo-bar-foo"])
.fails()
.and()
.stderr().contains("foo-bar-foo")
.unwrap();
```
will be turned into
```rust
assert_cli::Assert::command(&["ls", "foo-bar-foo"])
.fails()
.and()
.stderr()
.contains("foo-bar-foo")
.unwrap();
```
which obscures intent.
Normally, I don't like working around tools but this one seems
sufficient to do so.
```rust
assert_cli::Assert::command(&["ls", "foo-bar-foo"])
.fails()
.and()
.stderr(assert_cli::Output::contains("foo-bar-foo"))
.unwrap();
```
Pros
- More consistent with `with_env`
- Can add support for accepting arrays
- Still auto-complete / docs friendly
- Still expandable to additional assertions without much duplication or
losing out on good error reporting
Cons
- More verbose if you don't `use assert_cli::{Assert, Environment, Output}`
Alternatives
- Accept distinct predicates
- e.g. `.stderr(assert_cli::Is::text("foo-bar-foo"))`
- e.g. `.stderr(assert_cli::Is::not("foo-bar-foo"))`
- Strange `text` function
- More structs to `use`
- Less auto-complete / docs friendly (lacks contextual discovery or
whatever the UX term is)
Fixes #70
BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
Originally based on #74
|
@epage what do you think of this? (You just need to look at the second commit) |
killercup
changed the title
|
I'm hoping this isn't pride but I think I prefer the original solution.
Thoughts? If you agree, I'll go ahead and fix up your suggestions on #74. |
|
@epage I usually agree with wildcard imports being bad, but preludes are a special use case as they only re-export the things that you'd usually import anyway. But that's not the main point of the PR, you can expand the glob import—that's actually the one fancy refactoring feature the RLS has currently! Or do something like What do you think about using free functions instead of constructors on Output? My main concern here is making test code be concise and expressive (both when reading and writing it), and I think |
|
High level: I understand the desire to make the API less verbose. I think this is a case of us having slightly different priorities in API design. So I'd call it a tie with you breaking the tie in the direction you see fit. If you go forward with this PR, we should have at least once one example not using a prelude to help people if there is a symbol conflict Details (continued):
|
|
Before working on #80, we should probably decide between
to avoid major conflicts between the output work for #80 and this. |
|
Sorry, I'd love to make progress here. I really feel like I should spend some time thinking about this and maybe experimenting with more real-world code; but I don't have much time right now :( So I'd be open to gather arguments from more people and figuring out a general consensus. |
|
Seems reasonable. |
Based on #74, cf. #74 (review)