Reimplementation appears before the original case in `bob`

Exercism Building Exercism
1

A recent PR reordered the test cases in bob, and the test case with uuid “66953780-165b-4e7e-8ce3-4bcb80b6385a” now appears after its reimplementation (uuid “2c7278ac-f955-4eb4-bf8f-e33eb4116a15”).

This is the first time I’ve seen this behavior. The original usually appears before its reimplementation. The generator in the x86-64 track relied on this ordering, so this case was silently dropped after the sync. I almost didn’t notice.

I updated the generator and things should work now regardless, but other tracks may have generators that rely on the same assumption. I think we should reorder this specific case so it appears before the reimplementation.

1 Like
2

Agree that maybe we should have “original” cases before reimplemented. I don’t think the ordering messed with the Python generator, but now need to go check… :thinking:

Edited for update: The Python generator’s fine. But I still think that reimplemented test cases should come after “normal” ones. Either directly after, or grouped at the end.

3

This point was also raised by @keiraville in [bob] Sort test cases by IsaacG · Pull Request #2662 · exercism/problem-specifications · GitHub. There are a handful of cases in the problem-specs where the reimplementation is either before the test being replaced or several spots after it.

I feel this is a defect in the respective generators, not the problem specs. That said, I realize at least one track is affected, maybe two, so I’d be okay +1’ing having the reimplementation occur immediately after the test being replaced as long as we do it across the board and not just for bob.

4

I think the most important thing here is having some predictable order between an original test and its reimplementation.

This is not just about a handful of generators, but mainly about documentation. Without a predictable order, I have to read the entire canonical data to make sure a test has not been reimplemented. And there are cases in some exercises which are reimplemented more than once, so I would have to keep notes or use a script to format the document before reading it.

Also, bear in mind not all tracks have a generator, some write test files manually.

2 Likes
5

The canonical data doesn’t have any intrinsic order. Relying on it to have some specific unenforced order is … not great.

We could explicitly require a specific order (eg some relationship between an exercise and anything that reimplements it). If we’re requiring that, we should enforce that requirement with CI checks.

Personally I think the reimplements field is pretty confusing and that a reimplemented_by reverse pointer would be a lot easier to work with. I’m not sure if it makes sense to rewrite the schema, though. Removing fields would break and tools that rely on it.

On the other hand, we could probably add a small tool that reads the spec and either outputs the same in a specific order and/or adds a reverse reimplemnted_by reverse pointer.

Regarding this particular file, we could totally reorder that one test case … but in the current state, I would recommend against assuming any particular order to the data.

1 Like
6

Same opinion as other maintainers, with one piece of advice I’ve learned along the way:

  • use the tests.toml file to tell you which test cases are included and which are not
    • configlet already has the “reimplemented” logic, you don’t need to reinvent it
    • tests.toml is easy enough to parse without using a TOML parser
  • then while you’re iterating over the canonical_data.json you can refer to your list of included UUIDs.
2 Likes
7

Because I was bored, here’s a small Python script that takes an exercise slug as an arg and dumps the canonical data with a reimplemented_by field added to exercises that are reimplemented.

8

Folks, I didn’t write the generator in the x86-64 track. It was already there.

I believe we should enforce some ordering, yes. ln the vast majority of exercises, the reimplementation appears just after the deprecated test case. This is true even if the reimplementation is added much after the following test cases.

That ordering also improves readability quite a bit, as I mentioned before. It is basically impossible to read the canonical data without an external tool if there is no predictable order.

Yes, we can have a script for that and recommend maintainers and contributors to download it before reading canonical data. But this should be documented, this script should be hosted somewhere and be readily available.

We can also assume canonical data shouldn’t be read directly, just processed by a generator. But then things would be unnecessarily much harder for tracks without a generator, and for other contributors too.

I think enforcing an order between a deprecated test case and its reimplementation(s) is probably the best solution here.

9

Does it ignore the tests.toml file and include test cases that aren’t in the tests.toml?

10

If you feel that’s the best approach, I would suggest you propose making this a requirement with a CI test and we can discuss that proposal directly. This thread is going in a few direction so having a more narrow thread can be helpful (though I suspect that was the original intent of this thread).

11
12

I always thought this ordering was already enforced, as this appears to be the usual practice. This is why I proposed to change only this exercise. But since this is not the case, I think, yes, this should be enforced.