Refactor current tests and start to use assert_cmd
#24
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24 +/- ##
=========================================
Coverage ? 13.56%
=========================================
Files ? 46
Lines ? 4349
Branches ? 1705
=========================================
Hits ? 590
Misses ? 3167
Partials ? 592Continue to review full report at Codecov.
|
mssun
changed the title
assert_cliassert_cmd
The assert_cli is said to be deprecated: assert-rs/assert_cli#41
|
Do you need to wait for that feature? The alternative is just to read the file and pass it in. The main benefits of that assert_cmd feature are (1) large file support and (2) convenience. |
Yes, reading file and pass it in is a solution. I think with the feature provided by Anyway, I will work on refactoring other test cases and come back to this when it is ready. @epage Do you think it will take much more time to have this feature done? |
I probably won't get to it until tomorrow night. If that is too long, feel free to prepare a PR. |
tests/gnu/test_arch.rs
Outdated
| new_cmd!() | ||
| .assert() | ||
| .success() | ||
| .stdout("x86_64\n") |
There was a problem hiding this comment.
We're considering trimming whitespace for you.
Could you post your thoughts on assert-rs/assert_cmd#19
| #[macro_export] | ||
| macro_rules! pred_str_contains { | ||
| ($str:expr) => { | ||
| predicate::str::contains($str).from_utf8() |
There was a problem hiding this comment.
RE from_utf8
Right now, the only automatic conversions that happen in our API are u8 slice to predicate and str to predicate.
Feel free to open an issue about converting Predicate<str> into Predicate<[u8]>. or any other ideas to simplify the use of the API.
There was a problem hiding this comment.
Yes, this one bother me a lot. I think it could work, but realize it doesn't when compile the code. It took me some time to find out the reason because the documentation of assert_cmd and predicates are mixed. I should check with these two docs.
There was a problem hiding this comment.
Could you open issues on any documentation problems you came across? We're wanting to make sure this is accessible for people starting a first Rust project to create a CLI. My challenge is the curse of knowledge: I'm too familiar with it to see know what pitfalls a new user will run into.
mssun
changed the title
assert_cmdassert_cmd
|
I think I'm done with the refactoring. @Arcterus |
|
|
||
| #[test] | ||
| #[ignore] | ||
| fn test_chmod_reference_file() { |
There was a problem hiding this comment.
Can you add this back? It should be supported eventually (I just need to figure out how to get it to work with clap).
There was a problem hiding this comment.
It's OK, because I cannot test my rewrite right now. So, let's write test cases then when it's ready.
|
The tests seem to run noticeably slower now, although this might just be due to fork+exec being slower than spawning a thread. On the other hand, it should be more resilient in the case of programs setting global state (e.g. we might be able to test the |
|
I don't why the first two cases are blocked. Also, not sure if it is related with this issue: assert-rs/assert_cmd#6 |
|
Ah that may be related to some extent. Those test cases work fine for me locally so I’m not sure what’s up with Travis. |
This PR will refactor current test cases and change to
assert_cmd,predicates,assert_fs, etc.