Should we have any sort of automatic verification of canonical-data.json content?

[petertseng]

#105 has gotten us off to a good start to make sure that our JSON files are syntactically correct, but does nothing to check the contents.

I have historically concerned myself (maybe to an unreasonable degree, even) about this second aspect:

• Add sum-of-multiples.json #206 (comment)
• dominoes: Add JSON test data #221 (comment)
• tournament: Add JSON file #287 (comment)
• react: Add JSON test data #358 (comment)
• pov: Add canonical data #390 (comment)
• forth: Add canonical data #394 (comment)
• minesweeper: remove number from input, add mines #456 (comment)
• https://github.com/petertseng/x-common/tree/verify and https://github.com/petertseng/x-common/commits/verify

Since it has now been asked in #456, now is the time to ask what y'all think: Should we have such a thing in our CI?

The current implementation in my verify branch is as follows:

• Each exercise directory may optionally have a verify.rb file.
• The idea is that Travis could run this file if it is present.
• Some verify.rb are more detailed than others (e.g. minesweeper only checks that the inputs are indeed the cleaned version of the outputs but doesn't check that the output is a correct annotation, pov actually has a full solution to the exercise to make sure the input/output pairs are correct)

---

• Pro: Automatic verification helps avoid incorrect data, and after all we are but human.
• Con: We necessarily have to pick some language in which to write the verification scripts, and perhaps such a choice unnecessarily biases this repo toward that language.
  • But at least it's better than nothing, right?
  • And I suppose we could support multiple languages...
• Con: Does this mean we have to reimplement an example solution for each exercise and maintain it in this repo?
  • What's the alternative? Pull in an existing example solution from a language track that has the exercise? Then Travis CI starts to depend on not just this repo but the presence of some other repo as well.

I am happy to clean up my verify branch and submit it as a PR if we decide the pros outweigh the cons, with discussion to find any pros and cons I missed.

---

Aside: As an argument for standardisation (as proposed by #336), if all the JSON had a standardised structure you could imagine that we could remove some of the logic that has to be implemented separately in each verify.rb script.

[Insti]

I vote for No.

Adding an automated checking step is going to add a lot of friction for little benefit.

There are a couple of places where the file is reviewed by humans:

1. Pull requests in x-common
2. When updating tests in the tracks. (Either manually or via generators.)

Any problems with the file will hopefully be detected in one of these places, and anything that gets through both of those is unlikely to be caught by any kind of automated check.

[petertseng]

Hello, and thank you for opening this issue to propose this change.

After our deliberation which you can read in the comments of this issue, we can see that there is insufficient support for this change, so we will have to close this issue.

You are encouraged to continue this practice individually, if you believe it can be of use.

[Insti]

FWIW: I sometimes find myself disagreeing with myself on the above opinion.
I encourage the continuation of personal json validation.

[petertseng]

I took the encouragement to heart.

https://travis-ci.org/petertseng/x-common

[Insti]

What follows are thoughts rather than a proposal:

In xruby we have a shell command (bin/local-status-check) which will test all the tests against an example solution for each problem.

To test updates to the json we could re-generate the tests first and then do the status check.

There are some obstacles in the way of doing this, (mostly over-enthusiastic re-versioning.) but I'm working on eliminating them.

This does run the risk of making one particular implementation seem more important than the others, maybe we just run a similar script for ALL languages.

This requires that there are up to date generators for every language, but this is a goal that could be worked towards.

[petertseng]

Hooray. Every single problem is now being verified and it is all
(for those that can't see images, that says "passing", so the sentence should be considered to end with the words "it is all passing")

Shrug.

[coriolinus]

If we don't build in some kind of automatic validation, we can be sure that at some point some exercise will admit a faulty file by accident.

…

On Thu, Jan 25, 2018 at 11:34 PM, Peter Tseng ***@***.***> wrote: Hooray. Every single problem is now being verified and it is all [image: Build Status] <https://travis-ci.org/petertseng/exercism-problem-specifications> (for those that can't see images, that says "passing", so the sentence should be considered to end with the words "it is all passing") Shrug. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#460 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHdeTjBNLqMVBrb0fnHPoBtPf4Lqr0icks5tOQGFgaJpZM4K_e0j> .

[petertseng]

See also verification of coverage, namely testing that specific mistaken implementations fail at least one test: #1214

[petertseng]

One thing that troubles me about making verification part of CI:

By necessity, the verifier for an arbitrary exercise E is written in written in some language L (after all, it cannot be written in no language).

Potential contributors who wish to add a test case for exercise E may have to change the verifier if the verifier was not already properly equipped to handle the new test case.
Similarly, contributors who wish to change the structure of the tests in some way will also have to change the verifier, by necessity.

The need to change the verifier will disadvantage contributors who do not favour language L.

It is obviously unfortunate to have to disadvantage a portion of contributors.

Edit: Whoops, actually I already mentioned that in the issue description. Please excuse this useless post that added no information, then.