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

Review/feedback #16

Merged
merged 10 commits into from
Mar 22, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ homepage = "https://github.com/killercup/assert_cli"
documentation = "http://killercup.github.io/assert_cli/"
readme = "README.md"
categories = ["development-tools::testing"]
keywords = ["cli", "testing"]
keywords = ["cli", "testing", "assert"]
build = "build.rs"

[dependencies]
Expand Down
20 changes: 10 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
# Assert CLI

> Test CLI Applications.
> **Test CLI Applications** - This crate checks the output of a child process is as expected.

Currently, this crate only includes basic functionality to check the output of a child process
is as expected.

[![Build Status](https://travis-ci.org/killercup/assert_cli.svg)](https://travis-ci.org/killercup/assert_cli) [![Coverage Status](https://coveralls.io/repos/killercup/assert_cli/badge.svg?branch=master&service=github)](https://coveralls.io/github/killercup/assert_cli?branch=master)

**[Documentation](http://killercup.github.io/assert_cli/)**
[![Build Status](https://travis-ci.org/killercup/assert_cli.svg)](https://travis-ci.org/killercup/assert_cli) [![Documentation](https://img.shields.io/badge/docs-master-blue.svg)][Documentation]

## Install

Expand Down Expand Up @@ -54,8 +49,8 @@ fn main() {
}
```

If you want to check for the program's output, you can use `print` or
`print_exactly`:
If you want to match the program's output _exactly_, you can use
`prints_exactly`:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a random comment.

When first poking around with this crate I found the semantics of prints a bit surprising. I expected a strict equality assertion. Then I learned that there is prints_exactly.

It's very likely a matter of taste and I might even change my mind (I often do), but I think I would prefer the strict assertion over the fuzzy assertion. I tried hard to think of a different naming, where the strict variant would have the shorter name, but I couldn't find something nice.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried hard to think of a different naming, where the strict variant would have the shorter name, but I couldn't find something nice.

Me neither! That's one of the main reason I made the exact method's name longer.

The previous version only had exact matching, and it was difficult to get right, because you had to include every new line (Maybe with carriage return on Windows? Who knows!) and control character.

This reminds of of another thing I want to support: Multiple fuzzy checks. I'll open an issue for that in a minute.


```rust,should_panic="Assert CLI failure"
#[macro_use] extern crate assert_cli;
Expand All @@ -67,13 +62,16 @@ fn main() {
}
```

this will show a nice, colorful diff in your terminal, like this:
... which has the benefit to show a nice, colorful diff in your terminal,
like this:

```diff
-1337
+92
```

More detailed information is available in the [documentation]. :-)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link doesnt work because the link ref definition starts with an uppercase D

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it's in the markdown specs, but github and my atom IDEs markdown preview treat it case-insensitive:
https://github.com/colin-kiegel/assert_cli/blob/1f29f4f25978bedf0bf476f5a27bedb0ea6a83e5/README.md

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


## License

Licensed under either of
Expand All @@ -89,3 +87,5 @@ Unless you explicitly state otherwise, any contribution intentionally
submitted for inclusion in the work by you, as defined in the Apache-2.0
license, shall be dual licensed as above, without any additional terms or
conditions.

[Documentation]: http://killercup.github.io/assert_cli/
41 changes: 22 additions & 19 deletions src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
static ERROR_PREFIX: &'static str = "CLI assertion failed";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Why static and not const?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I tend to forget that there are consts too. ^^

I'll link it in a todo list, at the top comment. :-)


error_chain! {
foreign_links {
Io(::std::io::Error);
Expand All @@ -7,42 +9,43 @@ error_chain! {
StatusMismatch(cmd: Vec<String>, expected: bool) {
description("Wrong status")
display(
"Command {:?} {got} but expected it to {expected}",
"{}: `(command `{}` expected to {})` (command {})",
ERROR_PREFIX,
cmd.join(" "),
got = if *expected { "failed" } else { "succeed" },
expected = if *expected { "succeed" } else { "failed" },
expected = if *expected { "succeed" } else { "fail" },
got = if *expected { "failed" } else { "succeeded" },
)
}
ExitCodeMismatch(cmd: Vec<String>, expected: Option<i32>, got: Option<i32>) {
description("Wrong exit code")
display(
"Command {:?} exited with code {:?} but expected it to be {:?}",
cmd.join(" "), got, expected,
"{}: `(exit code of `{}` expected to be `{:?}`)` (exit code was: `{:?}`)",
ERROR_PREFIX,
cmd.join(" "),
expected,
got,
)
}
OutputMismatch(expected: String, got: String) {
OutputMismatch(output_name: String, cmd: Vec<String>, expected: String, got: String) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also embed the OutputType enum directly - but then it must be public and would appear in the docs.

description("Output was not as expected")
display(
"Expected output to contain\n{}\nbut could not find it in\n{}",
"{}: `({} of `{}` expected to contain `{:?}`)` (output was: `{:?}`)",
ERROR_PREFIX,
output_name,
cmd.join(" "),
expected,
got,
)
}
ExactOutputMismatch(diff: String) {
ExactOutputMismatch(output_name: String, cmd: Vec<String>, diff: String) {
description("Output was not as expected")
display("{}", diff)
}
ErrorOutputMismatch(expected: String, got: String) {
description("Stderr output was not as expected")
display(
"Expected stderr output to contain\n{}\nbut could not find it in\n{}",
expected,
got,
"{}: `({} of `{}` was not as expected)`\n{}\n",
ERROR_PREFIX,
output_name,
cmd.join(" "),
diff.trim()
)
}
ExactErrorOutputMismatch(diff: String) {
description("Stderr output was not as expected")
display("{}", diff)
}
}
}
Loading