Small wording/naming changes; some missing tests #15
Conversation
| @@ -86,8 +86,8 @@ where | |||
|
|
|||
| if subject.is_empty() { | |||
| AssertionFailure::from_spec(self) | |||
| .with_expected("an non empty hashmap".to_string()) | |||
| .with_actual(format!("a hashmap with length <{:?}>", subject.len())) | |||
| .with_expected("a non empty hashmap".to_string()) | |||
There was a problem hiding this comment.
This sounds grammatically more correct
| .with_expected("an non empty hashmap".to_string()) | ||
| .with_actual(format!("a hashmap with length <{:?}>", subject.len())) | ||
| .with_expected("a non empty hashmap".to_string()) | ||
| .with_actual("an empty hashmap".to_string()) |
There was a problem hiding this comment.
This seems a little more expected than always saying "with length <0>"?
There are similar changes to this expected/actual failure message change below for HashSet and Vec/&Vec
| @@ -340,7 +340,7 @@ mod tests { | |||
| // Unfortunately the order of the keys can change. Doesn't seem to make sense to sort them | |||
| // just for the sake of checking the panic message. | |||
| #[should_panic] | |||
| fn should_not_panic_if_hashmap_does_not_contain_key() { | |||
| fn should_panic_if_hashmap_does_not_contain_key() { | |||
There was a problem hiding this comment.
Renamed a couple tests where the name didn't seem to exactly match what the test was doing
| @@ -358,7 +358,7 @@ where | |||
| AssertionFailure::from_spec(spec) | |||
| .with_expected(format!("Completed iterator (read <{:?}>)", read_expected)) | |||
| .with_actual(format!( | |||
| "Iterator item of <{:?}> (read <{:?}>", | |||
| "Iterator item of <{:?}> (read <{:?}>)", | |||
There was a problem hiding this comment.
These looked like missing closing )'s
| @@ -166,7 +166,7 @@ mod tests { | |||
| } | |||
|
|
|||
| #[test] | |||
| #[should_panic(expected = "\n\texpected: option[none]\n\t but was: option<\"Hello\"")] | |||
| #[should_panic(expected = "\n\texpected: option[none]\n\t but was: option<\"Hello\">")] | |||
There was a problem hiding this comment.
Not like this was "wrong" but seems more expected to include the closing > in the expected panic message
| #[test] | ||
| #[should_panic(expected = "\n\texpected: vec to have length <1>\n\t but was: <3>")] | ||
| fn should_panic_if_ref_vec_length_does_not_match_expected() { | ||
| let test_vec = vec![1, 2, 3]; | ||
| assert_that(&&test_vec).has_length(1); |
There was a problem hiding this comment.
My understanding is that this second failed assertion inside a #[should_panic] test wasn't actually being "exercised" because the test would (expectedly) fail as of the first failed assertion
So split these into two separate tests
| assert_that(&&vec![1]).is_empty(); | ||
| } | ||
|
|
||
| #[test] | ||
| fn should_not_panic_if_vec_was_expected_to_be_not_empty_and_is() { |
There was a problem hiding this comment.
Looked like tests for .is_not_empty() were missing so added
Hi, these are some changes I noticed as I read through the code-base
Thanks for the great library, it has become something I reach for when I'm writing tests in Rust!
I'll comment on the individual changes in the PR diff