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

Created on 29 Nov 2016  Â·  9Comments  Â·  Source: exercism/problem-specifications

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:

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.

picture petertseng

Most helpful comment

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.

picture petertseng on 5 Oct 2018
👍2

All 9 comments

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.

Insti picture Insti on 30 Nov 2016

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.

petertseng picture petertseng on 7 Dec 2016
❤1

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

Insti picture Insti on 7 Dec 2016

I took the encouragement to heart.

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

petertseng picture petertseng on 7 Dec 2016
🎉1

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.

Insti picture Insti on 7 Dec 2016

Hooray. Every single problem is now being verified and it is all Build Status
(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.

petertseng picture petertseng on 25 Jan 2018
🎉1

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 notifications@github.com
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
https://github.com/exercism/problem-specifications/issues/460#issuecomment-360623556,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHdeTjBNLqMVBrb0fnHPoBtPf4Lqr0icks5tOQGFgaJpZM4K_e0j
.

coriolinus picture coriolinus on 25 Jan 2018

See also verification of coverage, namely testing that specific mistaken implementations fail at least one test: https://github.com/exercism/problem-specifications/issues/1214

petertseng picture petertseng on 3 Apr 2018

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.

petertseng picture petertseng on 5 Oct 2018
👍2
Was this page helpful?
0 / 5 - 0 ratings

Related issues

shaleh picture shaleh  Â·  3Comments
dvberkel picture dvberkel  Â·  5Comments
petertseng picture petertseng  Â·  5Comments
kytrinyx picture kytrinyx  Â·  4Comments
budmc29 picture budmc29  Â·  3Comments