Hey everyone, hope you’re well. Just got into the medium exercises here after some time away. I made an interesting logical error in my first iteration on Crypto Square that happens to nevertheless pass the full test suite.
If I understand right the category of input that my solution doesn’t handle correctly is “r<c when no padding is required.” For example:
test "6 character plaintext results in 3 chunks of 2 characters" do
assert CryptoSquare.encode("Fivers") == "fe ir vs"
end
My solution fails this test. So I think we should add it. But I remember there’s maybe something tricky with Exercism in cases like that, like all the tests get rerun? In my opinion even if this fails previously passing solutions like mine, this would still be the correct course of action.
My solution also fails the following test, but for the same reason, so maybe it’s redundant:
test "2 character plaintext results in 2 chunks of 1 character" do
assert CryptoSquare.encode("IT") == "i t"
end
If I instead do 5 characters where 1 space of padding is required, my solution’s padding handling makes it work. But maybe there are other “happens to work” scenarios besides mine, if you’d like me to add a 5-character test too.
Heya @IsaacG , thanks, great question. I’m not sure whether or not my logical error is likely in other languages, but I’m sure it’s possible.
Changing the problem spec is probably better, though more costly. On the other hand, I think it’s a pretty legitimate hole in the tests, not just an edge case — there are many input lengths where my original solution passes when it shouldn’t; meanwhile, since it’s a medium-level practice exercise, there will be less user solutions to re-validate than an easy-level one.
I can make a PR to the problem spec repo as well in that case. Does this thread belong more in the Exercism category of the forum either way?
The vast majority of practice exercises use the canonical data without any changes.
If the track is currently using the canonical data, that should probably remain unchanged.
Logical errors are generally not tied to any specific language.
Syncing tests from the canonical data to tracks is optional and done per track.
Rerunning tests is optional, controlled per track, as part of the sync.
If you think the tests need updating, first check the canonical data; this particular track may simply not be synced yet.
If you think the problem specs have a gap, please explain the gap (preferably in a language-agnostic manner). Please do not create a PR until you first have the go-ahead from three maintainers.
I can move this post to the appropriate category if you want to pivot to problem spec change discussion.
Agreed. And I checked the canonical data, which does not contain a test for length 2 or 5 or 6.
The gap is that there are ways to implement solutions that seem correct (mine being the example) because they pass the current test suite, but they’re not and they shouldn’t, because they produce incorrect output for all r×c cases where c = r + 1 and there’s no padding required. That is to say, inputs of normalized length 2, 6, 12, 20, 30, 42, etc. (the positive oblong numbers).
Ah, I went and moved the current thread from Programming→Elixir to Exercism already; if that’s not right, I welcome your help to pivot to problem spec change discussion.
And the reason one of normalized length 5 might be worth throwing in, despite not being a positive oblong number nor something my original solution fails, is that there are perhaps other solutions flawed in different ways than mine that might pass the suite when they shouldn’t.
This says there’s a gap with specific sizes and that your code has the gap. It doesn’t actually describe the actual issue. Is there something special about 5 vs 6 vs 7 that would make 6 a particularly useful test case to include? Is this an off-by-one problem?
The canonical data contains both tests where (1) r == c and where (2) r == c + 1. Does the “with padding” vs “without padding” matter? Why? Is there a logic error someone could stumble into in one case and not the other? What sort of logic error? Or is your code doing something unusual and unique? It would be helpful to describe why a solution might benefit from the extra test. It’s hard to assess the value of an additional test without understanding the gap it’s trying to cover.
Once the gap is explains, you can give an example test case that would cover that gap. What is the minimal test cases needed to cover that gap? If one additional case would cover it, there’s no value in adding multiple tests.
Once a gap is clearly explained and a test is given that covers that gap, a proposed test (i.e. the exact proposed input and expected output) is helpful to move things along.
Tests are great for helping cover specific gaps. If there’s no reason to assume a gap exists, and no understanding of what the gap might be, it’s generally not helpful to add tests that have no known value.
As for padding or not, yes, that’s where the oblong numbers come from: the only exact r × (r+1) rectangles (hence no padding) are the only ones my solution fails, but none of them are currently covered by the test suite. Whether or not this is likely to be happened upon by others, I cannot say.
You haven’t explained what the gap is or why a solution might be wrong. I understand that you have code that passes tests but isn’t correct. I don’t understand why. As such, I don’t understand why the new test would help.
Exactly Thank you for rephrasing it more clearly for me. @IsaacG I see you added a like, does that imply this is enough of a “why” to go ahead with a PR?
I’m still a bit unclear why a new test would be needed but it’s a justification. Is enough to propose a specific test (exact input, output) on the forum to cover that gap. Then, only after three maintainers give a thumbs up on the forum, then you would open a PR. You got one go ahead. You need the proposed test and two more maintainers!
Thank you for rephrasing it more clearly for me.