Skip to content
This repository has been archived by the owner on Dec 29, 2021. It is now read-only.

Fluent Output Tests Look off with rustfmt #70

Closed
epage opened this issue Oct 27, 2017 · 4 comments
Closed

Fluent Output Tests Look off with rustfmt #70

epage opened this issue Oct 27, 2017 · 4 comments
Labels

Comments

@epage
Copy link
Collaborator

epage commented Oct 27, 2017

The API looks weird when run through rustfmt because it puts each function on a separate line, so it clouds up what the predicate is operating on

  • Keep it, tools are meant to work for us
  • Go back to having a function per variant
  • Follow Environment and take in a predicate parameter
    • Easier to do multiple predicates (option to take in an array)
    • While it helps in places, I found it annoying in clap to access a bunch of different types
    • More linking is needed to help people find all of the relevant predicates

(Split of from #41)

@epage epage added the question label Oct 27, 2017
@epage
Copy link
Collaborator Author

epage commented Oct 27, 2017

RE Accepting predicates (from #41)

One thing in favor of accepting predicates as parameters is that someone wanted to pass multiple predicates at once which is more of an issue with contains and negated contains/is and not is.

But! At that point me might as well do .stderr(|x: OutputAssertThingy| -> bool { x.is("bar") || (x.contains("baz") || !x.is("bazinga")) }) to not reinvent boolean expressions. Which I think was something someone was asking for in another issue anyway (cf. #57).

I think there is still value in specialized predicates for better error reporting. We could just use From to auto-convert a Fn into one of the predicates.

@killercup
Copy link
Collaborator

Error reporting is a good point! We should aim for nice errors, as that is what people are using the crate for.

I didn't specify what OutputAssertThingy is in that quote… so maybe we can try to write a type that overloads boolean operators in a very clever way?

epage added a commit to epage/assert_cli that referenced this issue Oct 28, 2017
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 assert-rs#70

BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
epage added a commit to epage/assert_cli that referenced this issue Oct 28, 2017
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 assert-rs#70

BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
epage added a commit to epage/assert_cli that referenced this issue Oct 28, 2017
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 assert-rs#70

BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
epage added a commit to epage/assert_cli that referenced this issue Oct 28, 2017
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 assert-rs#70

BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
epage added a commit to epage/assert_cli that referenced this issue Feb 2, 2018
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 assert-rs#70

BREAKING CHANGE: `.stdout().contains(text)` is now
`.stdout(assert_cli::Output::contains(text)`, etc.
@epage
Copy link
Collaborator Author

epage commented Apr 7, 2018

In addition to the existing PR exploring this, #98 would also possibly resolve this.

@epage
Copy link
Collaborator Author

epage commented May 29, 2018

Addressed in assert_cmd
https://github.com/assert-rs/assert_cmd
assert_cli is going to morph into a wider-focused tool, including things like assert_fs. See #41

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

Successfully merging a pull request may close this issue.

2 participants