simple-cipher: update tests and add version file

[FridaTveit]

The simple-cipher tests should be updated to exactly match the canonical data. Also a version file should be added to match the canonical data version.

[RonakLakhotia]

@FridaTveit Can I give this a try?

[sjwarner-bp]

Go for it @RonakLakhotia 🎉

[RonakLakhotia]

@FridaTveit @sjwarner-bp for the case of "Key is made only of lowercase letters", what should be the input string?

[sjwarner-bp]

I might be misunderstanding, but I also find that bit of the canonical data confusing.

The input object is empty, which is quite different to the rest of them.

I'll take a closer look at the exercise when I am able to, and see if I can make sense of it, otherwise we might have to ask someone in the problem specifications repo 🙂

[RonakLakhotia]

sure. No problem 👍

[FridaTveit]

@RonakLakhotia I think what you want to test is:

Cipher cipher = new Cipher();
String key = cipher.getKey();

And then check that key only contains lowercase letters. So you don't need an input string because you're not actually encrypting or decrypting anything, you're just checking the key. Does that make sense? 🙂

[RonakLakhotia]

@FridaTveit I think I get your point. Will give it a try now and get back to you if I face issues. Thanks 😄

[anurag-rai]

Hi, is this open to be discussed? Or is @RonakLakhotia still working on it?
Basically, I wanted to ask:

1. Three Test modules are currently present but do not match the three cases in the problem-spec. Does this need a rewrite to exactly the spec? The spec has no mention of default Key.
2. The second case of spec mentions "Substitution cipher". Does it imply default key?
3. Some extra test cases are currently written (handling of invalid keys) but these are not mentioned in the spec. Will these test cases still be included in the test modules?

[FridaTveit]

Hi @anurag-rai 🙂 I think it's been long enough that it's safe to assume @RonakLakhotia is not working on it anymore. To answer your questions:

1. It doesn't really matter if we use three test files or one as long as we use the same tests as the canonical data. I believe we've used three files in this case since the exercise has three distinct steps and rather a lot of tests so it might be easier for the user if they're split into different test files. We've added a hints section to the README explaining how the different test files work. Does anyone else have any strong feelings about using one test file or several @exercism/java? 🙂
   The first few tests in the canonical data are for "Random key cipher" which will use a randomly generated key which I believe is what you mean by a default key 🙂

2. The second case supplies a key to be used for each test 🙂

3. If we have tests that aren't in the canonical data then they should be removed. There are a few exceptions when we've needed to test for edge cases that occur in Java but not all other languages. In that case the added test should include a comment explaining why it's deviating from the canonical data. However I can't see anything like that in the simple cipher tests 🙂

Does that help? Thanks for wanting to contribute! 😄

[hgvanpariya]

@FridaTveit : anyone is working on this ?

[FridaTveit]

@hgvanpariya I don't think so unless @anurag-rai is working on it? 🙂

[anurag-rai]

please feel free to work on it :)

[hgvanpariya]

Thank you , I'm working on this ...

[hgvanpariya]

@FridaTveit : need your code review comments ....

[RonakLakhotia]

@FridaTveit is this issue still valid?

[FridaTveit]

@RonakLakhotia yes 🙂