crypto-square: tests do not actually verify whitespace #895
Comments
|
While non-core exercises (the ones that are often associated with a low chance of immediate or any feedback, and which I've proposed that we're able to disable feedback for in exercism/exercism#5153) don't receive a lot of attention, you are quite right. You are welcome to submit an additional test. It would probably be a worthy addition to the canonical tests also, even though they are still locked down. So let's put a pin in that for now. |
|
At this point, the problem-specifications repository has been re-opened. Before then it was always an option to just add a test in this repository only. Would you be interested in doing either at this point, @nicuveo? |
|
I see that the decision to ignore whitespace was a deliberate decision, with the reasoning in #259. However, my judgment will be that as long as our output is a String, it's better to keep it as the problem description says, for it would be against the description if a student emitted an output with a lot of leading spaces (among others). If we change the output to a [String], then we could freely choose whether to require the trailing spaces or forbid them. |
Fixes exercism#895, the complaint that the whitespace was being ignored. The canonical test data itself is fine; the problem was that the test suite was ignoring whitespace when comparing the actual result to the expected result. The decision to ignore whitespace was a originally deliberate decision, with the reasoning in exercism#259. However, maintainers' current judgment is that as long as our output is a String, it's better to keep it as the problem description says, for it would be against the description if a student emitted an output with a lot of leading spaces (among others). If the track changes the output to a [String], then the maintainers could freely choose whether to require the trailing spaces or forbid them.
Fixes exercism#895, the complaint that the whitespace was being ignored. The canonical test data itself is fine; the problem was that the test suite was ignoring whitespace when comparing the actual result to the expected result. The decision to ignore whitespace was a originally deliberate decision, with the reasoning in exercism#259. However, maintainers' current judgment is that as long as our output is a String, it's better to keep it as the problem description says, for it would be against the description if a student emitted an output with a lot of leading spaces (among others). If the track changes the output to a [String], then the maintainers could freely choose whether to require the trailing spaces or forbid them.
|
I considered changing this in exercism/problem-specifications#1827, but my main observation is that since this is a ciphering exercise, the focus isn't on human understandability of the output. |
The problem statement explains that text should be justified, and the canonical tests do seem to suggest that we should verify that (some "expected" strings have trailing whitespace).
However, we currently do not compare the strings: we do several tests to provide very detailed error messages... but we ignore all those whitespace issues in the process of doing so.
I would be fairly easy to fix: just one additional test that checks that the two strings are identical. But I chose to open an issue rather than just create a PR, to discuss how to best approach this.
The text was updated successfully, but these errors were encountered: