Add JSON Schema for 'canonical-data.json' and update the README.md #602

rbasso · 2017-02-18T13:38:08Z

In #336 (canonical-data.json standardisation discussion), it was recently discussed a JSON Schema to capture the fundamental structure needed for canonical-data.json files.

After a lot of discussions and a few iterations, the resulting schema proposed seems stable and mature enough to become at least a recommended standard, IMHO.

This proposal was designed with the following goals:

Keep/improve human-readability.
Keep it flexible enough to allow designing any reasonable test suite.
Make it more regular and machine-readable.

There where naturally some trade-offs, as it is impossible to satisfy these objectives completely, but I think we got a good balance here.

To get a first impression of what is this about, in this new schema, a simple test suite would look like this:

{ "exercise": "foobar"
, "version" : "1.0.0"
, "cases":
    [ { "description": "Foo'ing a word returns it reversed"
      , "property"   : "foo"
      , "input"      : "lion"
      , "expected"   : "noil"
      }
    , { "description": "Bar'ing a name returns its parts combined"
      , "property"   : "bar"
      , "firstName"  : "Alan"
      , "lastName"   : "Smithee"
      , "expected"   : "ASlmainthee"
      }
    ]
}

Of course, there are more features, like comments, test groups and error signaling!

We expect that this change will soon allow us to:

Automatically verify the canonical-data.json files.
Simplify test generation, allowing more code to be shared among all exercises.
Pave the way for future standardization. As our understanding of test suite design improves and new best practices emerge, it will be easy to patch this schema to capture even more structure.

We know that this proposal will not make it possible to generate a test suite in a fully automatic way. To allow that we would have to significantly change all the test suites, and there was no agreement about how to do it or even if that is desirable.

Considering that this is a very sensible and highly debated topic, we shouldn't merge this PR until we are sure that enough people have a clear understanding of it and think this is an improvement over what we currently have.

We would like to know what do you think about this proposal. So, after studying it for a while, please...

Say ❤️ if you think this is great!
Say 👍 if you think this would make x-common better.
Say 👎 if you think this would make x-common worse.
If you have any question about it, please ask!

Also, if you think this isn't good the way it is, please consider helping us improve it!

Closes #336.

ErikSchierboom · 2017-02-18T20:00:17Z

README.md

+          [ " Test cases can be arbitrarily grouped with a description "
+          , " to make organization easier.                             "
+          ]
+      , "description": "Abnormal input: empty strings and numbers"


Would it perhaps make sense to also have an example with multiple input values?

You're absolutely right, @ErikSchierboom! I'll fix that soon.

Just changed the previous case to use two properties, firstName and lastName, as input. Was this what you had in mind, @ErikSchierboom?

Originally, I just thought to pass an array to input, but I think this is more readable as you now have input value names. Furthermore, if one was to use an array to specify multiple input values, it would become odd to pass an array.

One other option might be to make the content of input be an object. So something like this:

test method with no parameters: "input": {}

test method with one parameter: "input": { "name": "john" }

test method with more than one parameter: "input": { "name": "john", "age": 18 }

test method with one parameter that is an array: "input": { "allergies": ["fish", "peanuts"] }

We can then make input be a required field, which helps ease parsing the canonical data by test generators. What do you think about this option?

It really makes sense but, considering that we are intentionally not standardizing the input data for now, I used distinct key names to showcase that this is possible. This way people will not be mislead thinking that they need to have an input key in every test. Only description and property are mandatory.

I'm afraid that, if we try to standardize too much at once, more people will start to disagree about details and the chances of getting this approved will go down.

My target is to get a general structure approved, leaving all the details for other discussions.

We already added the restrictions to error, which are great, but they make this PR even harder for people to understand. I'm really not sure if trying to add more things now is good idea...

Point taken. Let's leave it as it is then.

Just for the sake of discussion, I found a few possible objections to adding an input object:

We don't really need it. All additional keys in a test case are already implicitly inputs to the test.

It decreases human-readability, adding an unneeded input key and increasing one level of nesting.

Google JSON Style Guide recommends avoiding unneeded objects an favor flatter structures (edit)

It would not fit more general property tests well, and we are trying to be as general as possible here. (edit)

Following the same reasoning, it would also make ~~more~~ sense to have either a expected or an error. ~~Maybe we shouldn't have put the error inside expected~~.

Edit: I think that error is nice the way it is now, and we are only enforcing it's structure if it is present in the expected, so we are not really losing anything with the current proposal, as it still allows other representation for errors. If in the future we decide that it is inconvenient, we can change the recommended encoding, but for now we already agreed on that.

rbasso · 2017-02-21T00:44:51Z

I forgot to mention @exercism/track-maintainers in the first message.

kotp

Just a trivial change.

kotp · 2017-02-21T07:29:14Z

README.md

+      , "property"   : "bar"
+      , "firstName"  : "Alan"
+      , "lastName"   : "Smithee"
+      , "expected"   : "Aslmainthee"


The example suggests more than the description states. Should be "ASlmainthee" I believe.

Why?!

I just kept the first in upper case , because is looks more like a real name.

Anyway, I'll change it if you prefer it the other way.

Because the description states that it returns the parts combined, not modified and combined.

Fixed! Thanks! 👍

stkent · 2017-03-04T22:52:47Z

I wonder if it would be useful to also include a schema version in canonical data? Just an idle thought. But anyway, LGTM!

catb0t · 2017-03-05T02:10:01Z

I really love this. I was the lone collaborator on #336 arguing for more machine-readability (to strike a balance with human-readability) and this is absolutely beautiful IMO.

rbasso · 2017-03-05T03:15:42Z

I wonder if it would be useful to also include a schema version in canonical data? Just an idle thought. But anyway, LGTM!

Do you know what is the recommended way to do it, @stkent? The best I could find was this link.

So, unless there is a more generally accepted versioning style, I'm considering following the directions on that link and add the following to schema:

   "self": { "vendor" : "io.exercism"
           , "name"   : "canonical-data"
           , "format" : "jsonschema"
           , "version": "1-0-0"
           },

What do you think about it?

In #336 ('canonical-data.json' standardisation discussion), it was discussed a JSON Schema to capture the fundamental structure needed for `canonical-data.json` files. This resulting schema can be used to automatically verify these files in `x-common`, after they are adapted to match the new format.

rbasso · 2017-03-05T03:45:49Z

@stkent, I just updated the PR to include the schema version according to this specification. Of course we can change the format later, but it seems a good idea to start versioning the schema right now.

@catb0t wrote:

I really love this. I was the lone collaborator on #336 arguing for more machine-readability (to strike a balance with human-readability) and this is absolutely beautiful IMO.

I'm glad you liked it!

Despite some minor modifications, in the last 15 days we had enough support for this schema and it seems there are no major changes needed here, so I'm planning merging this PR in a few hours, after taking a last look at it.

rbasso · 2017-03-05T15:48:07Z

Here is a link which can be used to easily validate a canonical-data.json file against the schema.

ErikSchierboom · 2017-03-06T07:01:21Z

🎉 !

Regenerate all tests

ErikSchierboom reviewed Feb 18, 2017

View reviewed changes

ErikSchierboom approved these changes Feb 19, 2017

View reviewed changes

kotp added the discussion label Feb 20, 2017

kotp reviewed Feb 21, 2017

View reviewed changes

ErikSchierboom mentioned this pull request Feb 21, 2017

bowling: be more explicit about when errors should happen #536

Merged

This was referenced Feb 26, 2017

perfect-numbers: add canonical data #611

Merged

diamond: add canonical data #612

Merged

stkent approved these changes Mar 4, 2017

View reviewed changes

rbasso added 2 commits March 5, 2017 12:25

Update README.md to reflect new JSON Schema

b750246

rbasso merged commit dad6e03 into exercism:master Mar 5, 2017

rbasso deleted the canonical-schema branch March 5, 2017 15:46

rbasso mentioned this pull request Mar 6, 2017

canonical-data.json: Compliance with JSON Schema #625

Closed

75 tasks

behrtam mentioned this pull request Mar 8, 2017

Investigate automated test suite generation based on JSON exercism/python#271

Closed

behrtam mentioned this pull request Apr 6, 2017

Validate the test data version as part of a travis build exercism/discussions#133

Closed

petertseng mentioned this pull request Dec 16, 2017

book-store: Update json for new "input" policy #1037

Merged

rbasso mentioned this pull request Jan 28, 2018

json schema validation: enforce lowerCamelCase keys #987

Open

kytrinyx mentioned this pull request Aug 3, 2018

Validate the test data version as part of a travis build exercism/exercism#4131

Closed

emcoding pushed a commit that referenced this pull request Nov 19, 2018

Merge pull request #602 from Insti/Regenerate_all_tests

8347bbe

Regenerate all tests

petertseng mentioned this pull request Jan 25, 2022

Format using prettier #1917

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add JSON Schema for 'canonical-data.json' and update the README.md #602

Add JSON Schema for 'canonical-data.json' and update the README.md #602

rbasso commented Feb 18, 2017 •

edited

Loading

ErikSchierboom Feb 18, 2017

rbasso Feb 19, 2017

rbasso Feb 19, 2017

ErikSchierboom Feb 19, 2017 •

edited

Loading

rbasso Feb 19, 2017 •

edited

Loading

rbasso Feb 19, 2017 •

edited

Loading

ErikSchierboom Feb 19, 2017

rbasso Feb 20, 2017 •

edited

Loading

rbasso commented Feb 21, 2017

kotp left a comment

kotp Feb 21, 2017

rbasso Feb 21, 2017

kotp Feb 21, 2017

rbasso Feb 21, 2017

stkent commented Mar 4, 2017

catb0t commented Mar 5, 2017

rbasso commented Mar 5, 2017

rbasso commented Mar 5, 2017 •

edited

Loading

rbasso commented Mar 5, 2017

ErikSchierboom commented Mar 6, 2017

Add JSON Schema for 'canonical-data.json' and update the README.md #602

Add JSON Schema for 'canonical-data.json' and update the README.md #602

Conversation

rbasso commented Feb 18, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom Feb 19, 2017 • edited Loading

Choose a reason for hiding this comment

rbasso Feb 19, 2017 • edited Loading

Choose a reason for hiding this comment

rbasso Feb 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbasso Feb 20, 2017 • edited Loading

Choose a reason for hiding this comment

rbasso commented Feb 21, 2017

kotp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stkent commented Mar 4, 2017

catb0t commented Mar 5, 2017

rbasso commented Mar 5, 2017

rbasso commented Mar 5, 2017 • edited Loading

rbasso commented Mar 5, 2017

ErikSchierboom commented Mar 6, 2017

rbasso commented Feb 18, 2017 •

edited

Loading

ErikSchierboom Feb 19, 2017 •

edited

Loading

rbasso Feb 19, 2017 •

edited

Loading

rbasso Feb 19, 2017 •

edited

Loading

rbasso Feb 20, 2017 •

edited

Loading

rbasso commented Mar 5, 2017 •

edited

Loading