In most exercises in canonical-data, reimplementations appear just after the deprecated test case they reimplement, but this is not enforced.
There was recently a discussion about this after bob test cases were reordered and a deprecated test case was placed long after its reimplementation.
In my opinion, not having some predictable order between the two severely reduces the readability of canonical-data, forcing maintainers and contributors to read the entire document for each test case to make sure it has not been reimplemented somewhere.
I propose that we choose some ordering and enforce it with a CI test. I don’t have a strong opinion on which ordering, as long as it is stable across exercises. But since most exercises already place the reimplementation just after the deprecated test case, this seems like the most frictionless option.
How would you handle a “reimplemented test”, when the test is split into multiple new test cases? Or the new test should be in another logical context (e.g. a different cases array) than the old one?
I don’t think enforcing an order is the right way to go here. A guideline in the documentation, maybe?
It seems this discussion is still ongoing so a PR feels premature without some consensus. As I noted on that PR, the reimplementing test should immediately follow the one it replaces. Otherwise, this change doesn’t improve readability if I’m manually adding tests without generator. I have to read multiple unrelated test cases to see if there’s a reimplementation later for the one I’m reading now. Otherwise, I’d add a test and then remove it later when I get there. If the reimplementing test is next, I can more easily see that I should skip this test in favor of the next one.
I think all cases have an uuid, their own input and expected keys. They also appear flattened in tests.toml. When they are part of a larger set, this usually only affects their description. As far as I know, this outer “case” doesn’t even have an uuid.
If I’m correct, I believe “outer” cases, which don’t have an uuid, can’t really be reimplemented. The reimplementation would be of each individual “inner” case, which should appear inside the “outer” cases array right after the case it reimplements.
For example, in forth an “outer” test such as “parsing and numbers” is just a set of two cases that can be individually reimplemented.
If we’re proposing “rules”, we should consider the possibility that a reimplementation will be in a different “context”. Otherwise things may be broken the first time that happens.
I believe you’re correct that contexts do not have a UUID.
I’ve got to agree with Glenn’s comment in the other thread that the tests.toml should be used to filter tests and that the data shouldn’t be constrained or have implicit order based meaning. Requiring the data be ordered in a very specific manner seems like the wrong solution to whatever problem is being solved here.
Yeah, this seems fragile. Can a test be reimplemented by multiple separate tests? If we force them to immediately follow the reimplemented test, which goes first? So even my idea falls flat in that scenario. Ultimately, I’m inclined to agree with Glenn and Isaac here.
I think the focus here shouldn’t be on a strict ordering, but on some kind of ordering, albeit loose but in any case stable, that ensures some degree of readability for the canonical data.
If a case is reimplemented by multiple separate tests, they can be in any order between them, as long as I know where they, as a set, are located in relation to the test they reimplement.
A guideline might work too. In fact, it has been working, since the vast majority of exercises already follow the “rule” I’m suggesting. Even reimplementations added much later are almost always placed next to the test they reimplement, not in some random order.
Even list-ops and sgf-parsing, which were mentioned as exceptions to the rule, place the reimplementations close to the cases reimplemented. sgf-parsing puts it just before (instead of just after) and list-ops put a block of reimplementations just after the set of reimplemented cases.
Other than those 2, ledger puts its reimplementation at the end of the file (also a sensible ordering). The only odd case is grade-school.
So, with the exception of grade-school, I think all exercises have a predictable ordering between deprecated cases and their reimplementation, most placing the reimplementation just after.
Note that in anagram the 3 reimplementations of case with uuid “85757361-4535-45fd-ac0e-3810d40debc1” are placed just after that same case. For readability purposes, it doesn’t matter which comes first.
Now compare with the situation where one of those reimplementations is placed at the top of the file, the other at the bottom and the third at some random place at the middle. Impossible to read.