Skip to content

simple-cipher: validating input strings #1298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xarxziux opened this issue Aug 17, 2018 · 7 comments
Closed

simple-cipher: validating input strings #1298

xarxziux opened this issue Aug 17, 2018 · 7 comments

Comments

@xarxziux
Copy link

I've already raised this as a separate issue in exercism/javascript#445, but have just discovered that I should be raising it here.

The unit tests for the Simple Cipher include testing for an error if the cipher is all caps, or all digits, but this is in adequate for the description. Part of the instructions state: "Let's make your substitution cipher a little more fault tolerant by ... ensuring that the key contains only lowercase letters".

Unfortunately this is not checked for, and I've seen one case where the submission passed the tests but did not conform to the description. The addition of tests for spaces or punctuation marks in the key would have failed the submission.

The specification needs to be updated to include these kind of checks.

@rpottsoh
Copy link
Member

description
testdata

@rpottsoh
Copy link
Member

@xarxziux thanks for bringing this issue to light. Would you be willing to submit a PR that fixes this issue?

@xarxziux
Copy link
Author

Willing: yes, able: not at the moment. If you're OK with waiting a week or so, then I'll put something up.

@rpottsoh
Copy link
Member

There is no rush.

@Insti Insti changed the title Simple Cipher - unit test specification does not match description simple-cCipher - unit test specification does not match description Aug 24, 2018
@Insti Insti changed the title simple-cCipher - unit test specification does not match description simple-cipher: Test specification does not match description Aug 24, 2018
@Insti
Copy link
Contributor

Insti commented Aug 24, 2018

I'm in favour of removing the case checking from the description rather than adding it to the tests. This exercise is about simple cipher not string validation.

See also: #523 (which might not support my argument.)

@Insti Insti changed the title simple-cipher: Test specification does not match description simple-cipher: validating input strings Aug 24, 2018
@Insti
Copy link
Contributor

Insti commented Aug 24, 2018

See also: #902

xarxziux added a commit to xarxziux/exercism-problem-specifications that referenced this issue Sep 1, 2018
Expand unit tests for validating input strings as per issue exercism#1298.
Three tests added covering:
- Keys containing a single upper case letter.
- Keys containing a space.
- Keys containing a punctuation mark.
xarxziux added a commit to xarxziux/exercism-problem-specifications that referenced this issue Sep 1, 2018
Expand unit tests for validating input strings as per issue exercism#1298.
Three tests added covering:
- Keys containing a single upper case letter.
- Keys containing a space.
- Keys containing a punctuation mark.
xarxziux added a commit to xarxziux/exercism-problem-specifications that referenced this issue Sep 1, 2018
Expand unit tests for validating input strings as per issue exercism#1298.
Three tests added covering:
- Keys containing a single upper case letter.
- Keys containing a space.
- Keys containing a punctuation mark.
@rpottsoh
Copy link
Member

#1346 has been merged and #1345 closed, which should now close this issue as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants