crypto-square: Completely rewrite tests.

[rbasso]

This is a first draft of a new test suite based on what is being discussed in #259.

This diverges completely from x-common.

The test cases here are just examples of what needs to be tested, but the texts should probably be changed.

[petertseng]

I have a suggestion that could be crazy - do you think that since these are property-based tests that instead of having the tests in this order:

• text 1, property 1
• text 1, property 2
• ...
• text 2, property 1
• ...

Instead it should be:

• text 1, property 1
• text 2, property 1
• ...
• text 1, property 2
• ...

To me that seems the most likely to allow the student to work incrementally - first get one property working, and make sure it works for all the cases. As opposed to having to get a complete implementation first. Is it too crazy?

[rbasso]

Not crazy at all. At first I was writing it exactly like that.

In the end, I thought that the students would be happier encrypting the first plaintext as soon as possible.

For the first case, filtering out the spaces is enough to normalize the data. Also, because it is a filled square, it is easier to figure out a way to transpose it.

The later plaintexts force the student to refine their functions to deal with more complex normalization and reordering.

Do you think it would be more fun to get each each step completely solved at a time before taking the next one?

[rbasso]

From what I see, both are equally incremental, because each plaintext adds more complexity in normalizing and ordering, so I don't have a strong opinion about which one is better.

Your call. I can rewrite it if you want 👍 ❓

[petertseng]

This made me reflect about how I operate. I think I do like having the full thing working as soon as possible, and then refine - so the way it is currently. I do like figuring out the pieces but I at least want to make sure the pieces fit together. So that means the way it is.

It kind of makes me wish we could offer both and let the student choose, but then they have to read the test file. Let's try it this way unless you have a better idea.

[petertseng]

One more thing - are you satisfied with "ao dg" rather than ["ao", "dg"]? I suppose it is OK if the spaces are only used for one thing (I know the original concern was that the spaces were being used for both padding and for joining the chunks)

[rbasso]

I thought about that for a while, but encode is an encryption function, so String -> String is almost unavoidable, if it where an intermediary function, String -> [String] would certainly be a good fit.

Edit: Using a string separated by spaces we could share the test data with x-common, even if it is padded with spaces.

About the padding, I made everything possible to avoid the problem. If someone decides to pad or separate blocks with any kind of isSpace character, it will work, because we just check the list returned by words.

Should we be more prescriptive about the output?

[rbasso]

What about the plaintext? Any text you would like to put there?

They where only placeholders with the right properties (symbols, spaces, number of alphanumeric character, case)...

[petertseng]

What about the plaintext? Any text you would like to put there?

Ah... I have no particular requests other than those that meet the requirements, really. It's been too long since I've done this exercise and/or reviewed others' so I don't remember what mistaken implementations there could be and what tests would catch them.

[rbasso]

Should we merge it and/or create an issue in x-common and wait for the discussion?

[petertseng]

Should we be more prescriptive about the output?

It doesn't seem very important to me. It's true that someone could now join with intercalate (replicate 1000 '\n')" if they really wanted and that this would be weird, but I don't see too much benefit to be more prescriptive about it (do you?)

Should we merge it and/or create an issue in x-common and wait for the discussion?

I think we have made the tests better for our track. I think we need not wait.

We already have track-specific tests in some instances (at least clock, custom-set, luhn, sublist, maybe others!), so it appears that adherence to x-common isn't always required (should we make a separate discussion on how we decide when it is OK to deviate?)

Another factor in my reasoning is that the property-based test will be difficult to represent in a JSON file. There is no canonical JSON file for https://github.com/exercism/x-common/tree/master/exercises/diamond , and the best I could do in dominoes was to type the properties at https://github.com/exercism/x-common/blob/master/exercises/dominoes/canonical-data.json#L27 (and still Rust is the only track that has those - the other three tracks just test true/false).

We can always go back if one day they are better.

The test cases here are just examples of what needs to be tested, but the texts should probably be changed.

Last question before merging - did you still want to change these? I didn't have any particular suggestions, since I think everything that can be tested already is.

[rbasso]

... I don't see too much benefit to be more prescriptive about it (do you?)

I believe that, as a rule, we should not test things that are not necessary to the challenge. Exact matching limits the number of possible interesting solutions.

... so it appears that adherence to x-common isn't always required (should we make a separate discussion on how we decide when it is OK to deviate?)

We could, but I think it is better to decide on a case-by-case basis.

... Last question before merging - did you still want to change these?

I'm fine with that. We can change if we sync with x-common in the future. 👍