simple-cipher: test that messages longer than the key can be decoded#1460
Conversation
paparomeo
force-pushed
the
simple-cipher-test-decode-message-longer-than-key
branch
from
550460b to
a73f4a9
Compare
rpottsoh
left a comment
rpottsoh
left a comment
There was a problem hiding this comment.
@paparomeo thanks for taking the time to propose these changes. At a quick glance the added test case seems reasonable. I just have a couple of items that should be addressed that I have indicated. This PR will remain open for a few days, at least, to allow time for other people to have an opportunity to chime in.
paparomeo
force-pushed
the
simple-cipher-test-decode-message-longer-than-key
branch
from
a73f4a9 to
e33df06
Compare
paparomeo
force-pushed
the
simple-cipher-test-decode-message-longer-than-key
branch
from
e33df06 to
d91b3cc
Compare
|
All request changes now pushed @rpottsoh. Thank you for the quick feedback! |
paparomeo
changed the title
SleeplessByte
left a comment
SleeplessByte
left a comment
There was a problem hiding this comment.
LGTM. @paparomeo when this has been merged, can you ping me in the JS pr so we can get that merged?
|
Will do @SleeplessByte! |
|
We have three approvals. I am going to go ahead and merge this. Thanks for working on this @paparomeo and making this exercise better! |
| "property": "decode", | ||
| "input": { | ||
| "key": "abc", | ||
| "plaintext": "iboaqcnecbfcr" |
There was a problem hiding this comment.
I highly recommend that this has to be ciphertext, like all the other decode cases in this file.
There was a problem hiding this comment.
Huh, yeah, that slipped by. I agree should be ciphertext. I think this would also constitute a major version change as well.
There was a problem hiding this comment.
Apologies. Missed that one too (the evils of copy+paste!). I'll submit a PR changing to ciphertext. I'll bump the version to major also.
5 participants
I just come across a solution for the "Simple Cipher" exercise from a student, which could encode messages longer than the key but failed to decode messages longer than the key, and yet, because we haven't a test to cover this, all tests would pass giving an illusion of correctness.
This pull request adds a test to also check that a message longer than the key can be decoded.