Replace test case - "invalid character in isbn". #1217
Merged
Conversation
This file contains hidden or 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
Set new test case to be 3-598-P1581-X. It should avoid solutions to pass - when inside characters (P) is treat as 0. 3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 0 * 6 + 1 * 5 + 5 * 4 + 8 * 3 + 1 * 2 + 10 * 1 = 264 264 is divisible by 11.
petertseng
suggested changes
Mar 30, 2018
There was a problem hiding this comment.
Thank you. Good change.
Before this can be Approved, please do two things:
- Change the version in this file from 2.2.0 to 2.3.0 so that versioning systems know the file has changed.
- In the commit message and in the pull request description, use a word in https://help.github.com/articles/closing-issues-using-keywords/ to indicate you have closed isbn-verifier: missing test case for non-digit character in the middle #1212, otherwise the issue won't get closed.
Increased version number. Closes exercism#1212
dnicolaev
changed the title
dnicolaev
changed the title
|
@petertseng thanks for pointing me to what should be done. |
petertseng
approved these changes
Mar 30, 2018
There was a problem hiding this comment.
I verified: the incorrect implementation https://github.com/petertseng/exercism-problem-specifications/blob/verify/exercises/isbn-verifier/verify.rb#L76-L82 now incorrectly accepts this case (a correct impl must reject), therefore this test case is now useful.
Given that:
- there is automated verification
- we had agreement in the linked issue that this is a useful case to add. The discussion has been open for 48 hours.
- it's my good-faith belief that the replaced test case added no value; it has been present without additional comment in Add isbn-verifier exercise #924
My judgment is that there is no further need to wait; let's merge it now
|
Great job, thank you. |
petertseng
added a commit
to exercism/haskell
that referenced
this pull request
Mar 31, 2018
…st (#670) Set new test case to be 3-598-P1581-X. This will actually catch incorrect solutions - when invalid characters (P) are treated as 0. 3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 0 * 6 + 1 * 5 + 5 * 4 + 8 * 3 + 1 * 2 + 10 * 1 = 264 264 is divisible by 11. The previous case did NOT have this property: 3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 2 * 6 + 0 * 5 + 5 * 4 + 0 * 3 + 7 * 2 + 0 * 1 = 249 249 is NOT divisible by 11. exercism/problem-specifications#1212 exercism/problem-specifications#1217
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.
Set new test case to be 3-598-P1581-X.
It should avoid solutions to pass - when inside characters (P) is treat as 0.
3 * 10 + 5 * 9 + 9 * 8 + 8 * 7 + 0 * 6 + 1 * 5 + 5 * 4 + 8 * 3 + 1 * 2 + 10 * 1 = 264
264 is divisible by 11.
Closes #1212