Crypto Square - duplicate unit tests, unused stub methods in downloaded files

Programming C#
1

In the Crypto Square C# exercise, there are two issues, although they are benign and do not prevent the exercise from being completed, they are unnecessary and/or confusing.

Two observations below:

  1. I propose that the unit test named below be removed.
    public void Fifty_four_character_plaintext_results_in_7_chunks_the_last_two_with_trailing_spaces()

There are two issues:
a) Its name does not describe the expected output.
b) There is another test after it in the file with the same input, same expected result, and whose name correctly describes the expected result

  1. The provided .cs file contains 3 empty methods which are not called from tests. I think the author intended that they would be used for the exercise, but there are no comments indicating their use. I would recommend either removing these, or possibly adding comments that indicate the intended use of them.
    The methods are:
* public static string NormalizedPlaintext(string plaintext)
* public static IEnumerable<string> PlaintextSegments(string plaintext)
* public static string Encoded(string plaintext)

As before, I can create a PR for one or both of the above if desired.

2

I’m not the C# track maintainer so we’ll need to wait to see what Erik would like to do here, but as a fellow maintainer elsewhere on Exercism, here are my thoughts.

Crypto Square is a practice exercise so the test cases come from problem-specifications/exercises/crypto-square/canonical-data.json at main · exercism/problem-specifications · GitHub. That test is reimplemented (or replaced) by the following test so that test should have been removed in the test suite. C# uses a runnable test generator so I think you’d want to run that and make sure it doesn’t include the seven chunk test. It should be skipped in favor of the following eight chunk test.

As for your other observation, my suspicion is that the stub corresponds to an earlier version of the exercise when those methods were in use. The canonical test data now only expects a single ciphertext property to be tested so only CryptoSquare.Ciphertext() needs to be stubbed out for the student to then implement. I’m assuming this simply fell between the cracks because if the extra methods aren’t called by the test suite, there wouldn’t have been any CI failures to investigate.

3

The extra methods are definitely a remnant of an old canonical data design. Those can be removed.

As for that weird test case, that is odd. I’ll check to see if there is a test generator for this exercise and I’ll report back.

1 Like
4

PR submitted for code changes here.

2 Likes
5

I found another one. Binary Search Tree has an Add method in the stub. Currently, we don’t test this method because the tests add values through the class constructor. Historically, Add was used to add values.

2 Likes