bob: improve test descriptions re: absence of letters #1282
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
rpottsoh
changed the title
rpottsoh
previously requested changes
Aug 1, 2018
cmccandless
approved these changes
Aug 1, 2018
|
The version will need to be changed to 1.3.0 if you end up adding new cases. |
|
I could easily go either way. I just wanted to see what other people thought about adding new test cases given the same solution would still technically pass the tests. The test names should clue students in now that it's not the "correct" way, but not stop you from using the same logic. |
rpottsoh
approved these changes
Aug 1, 2018
|
@Cohen-Carlisle it appears as though you are able to merge this. The commits should squashed before the PR is merged. |
I ran across a solution that passed the tests but violated the spirit of the tests. The student was checking for shouting by checking for: * the absence of lower case letters AND * (a string ending in `!` OR * a string without numbers) This means that "1,2,3!" would be shouting, but "JUST 1 NUMBER" wouldn't, which seems to violate the spirit of tests titled: * shouting numbers * shouting with no exclamation mark
Cohen-Carlisle
force-pushed
the
bob-numbers-letters-test-name
branch
from
27ca6a3 to
5e7ca4d
Compare
|
I actually cannot merge as I'm a member of exercism/alumni, but I've squashed the commits. |
|
@Cohen-Carlisle got you covered! |
sshine
added a commit
to exercism/sml
that referenced
this pull request
Oct 9, 2018
This includes: - 1.0.0 -> 1.1.0: "Calm down, I know what I'm doing!" in [1]. - 1.1.0 -> 1.2.0: No-op in [2] - 1.2.0 -> 1.2.1: "no letters" in [3]. - 1.2.1 -> 1.3.0: "I HATE THE DMV" in [4]. - 1.3.0 -> 1.4.0: Grammatical error in testdata in [5]. [1]: exercism/problem-specifications#1025 [2]: exercism/problem-specifications#1056 [3]: exercism/problem-specifications#1282 [4]: exercism/problem-specifications#1293 [5]: exercism/problem-specifications#1319
ZapAnton
added a commit
to ZapAnton/rust
that referenced
this pull request
Nov 20, 2018
Relevant PRs: - exercism/problem-specifications#1282 - exercism/problem-specifications#1293 - exercism/problem-specifications#1319 Appart from the new tests, some old tests were renamed / got their input value modified.
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.
I ran across a solution that passed the tests but violated the spirit of
the tests. The student was checking for shouting by checking for:
!ORThis means that "1,2,3!" would be shouting, but "JUST 1 NUMBER"
wouldn't, which seems to violate the spirit of tests titled:
Should a test be added for this case, or is changing test descriptions to be more clear enough?