Check that result contains expected allergens #134
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #130 Testing for all allergies would fail if the allergens weren't in the same order as the test vector. This checks that the result contains all the expected allergens rather than expecting the result and test vector to be equal.
|
Good change! I would add one more test: that |
|
Are we missing this test in https://github.com/exercism/x-common/blob/master/allergies.json? |
|
Oh wait, do you mean check that |
|
Same test or different test, I don't think I have a strong opinion. I'm happy to merge this as-is. @kytrinyx, I think the tests in the json file are fine. This is just a way to make the canonical tests work for Rust, while still handling the weird edge case that could emerge because of our testing approach. |
|
Great! I'll leave it alone then. No need to over-complicate things. |
|
Fireworks. Marching Bands. Etc. Thank you! |
IanWhitney
pushed a commit
to IanWhitney/xrust
that referenced
this pull request
Jun 6, 2016
Fixes exercism#135 - Introducing tests that were missing - Handling the ordering problem in all tests that check vectors, encapsulating the solution introduced in exercism#134 - Updating test names to not start with `test_` - Cargo starts each test result line with "test ", followed by the test's name.. So these tests looked like "test test_allergic_to_peanuts". - Trying to give the final test a useful name. "ignore non allergen score parts" didn't make much sense to me when I worked on this exercise. I think the new name better describes what we're trying to capture.
IanWhitney
pushed a commit
to IanWhitney/xrust
that referenced
this pull request
Jun 11, 2016
Fixes exercism#135 - Introducing tests that were missing - Handling the ordering problem in all tests that check vectors, encapsulating the solution introduced in exercism#134 - Updating test names to not start with `test_` - Cargo starts each test result line with "test ", followed by the test's name.. So these tests looked like "test test_allergic_to_peanuts". - Trying to give the final test a useful name. "ignore non allergen score parts" didn't make much sense to me when I worked on this exercise. I think the new name better describes what we're trying to capture.
IanWhitney
added a commit
that referenced
this pull request
Jun 11, 2016
* Update tests to canonical set Fixes #135 - Introducing tests that were missing - Handling the ordering problem in all tests that check vectors, encapsulating the solution introduced in #134 - Updating test names to not start with `test_` - Cargo starts each test result line with "test ", followed by the test's name.. So these tests looked like "test test_allergic_to_peanuts". - Trying to give the final test a useful name. "ignore non allergen score parts" didn't make much sense to me when I worked on this exercise. I think the new name better describes what we're trying to capture. Testing iterator equality seems like a nicer way of comparing these two vectors. And I'm now returning a useful error message when the vectors are not equal. If you do ```rust assert_eq!(expected.iter().eq(actual.iter())) ``` The error message is just that the two iterators are not equal. The differing elements are not called out in any way. Not as helpful.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #130
Testing for all allergies would fail if the allergens weren't in the
same order as the test vector.
This checks that the result contains all the expected allergens rather
than expecting the result and test vector to be equal.