README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date #551

petertseng · 2017-02-12T23:53:08Z

Note that while there is a new schema proposed in #336, it's believed by me that everything being said in this PR is still applicable even when that schema is applied:

msg will still be gone
descriptions will still follow the principles
errors will still be defined as they are in Representation of undefined/exceptional values in canonical-data.json #401

So this may be evaluated without having to wait for #336

petertseng · 2017-02-13T00:00:26Z

Previous discussion of descriptions: #451 (comment)

ErikSchierboom · 2017-02-14T07:46:44Z

README.md

@@ -47,9 +47,14 @@ variations.

 Each test case has the the following keys:
 - description: which will be used to name each generated test
+  - the description should not simply explain **what** each case is (that is redundant information)
+  - the description should explain **why** each case is there
+  - for example, what kinds of incorrect implementations might this case help us find?
 - 'variable names': one or more variable names with values which will be passed to the solution method


I was wondering about the "will be passed to the solution method" part. Shouldn't that be "will be passed to the test method"?

so I guess here "solution method" means "method in the solution". In leap, we'd get "year":1996 telling us to do is_leap 1996 or isLeap(1996) or similar. I do think "solution method" makes more sense than "test method". Basically I think it's to make clear that it's what the student wrote and is submitting.

If it still bears improvement, maybe I could offer "function under test", or simply "the submitted function" (maybe too specific, if not all submissions are single functions). I would have also offered "the student's function" but that canonicalises the student word that I use, whereas other people use others (practitioner, exercist, user...)

Ah I see. Good point then.

ErikSchierboom · 2017-02-14T07:48:02Z

This PR seems like a great improvement.

rbasso · 2017-02-15T09:36:09Z

README.md

- expected: the expected result (this would be -1 when we expect an exception)
- msg: a nice message for the failing case
+- expected: the expected result
+  - if an error is expected, the value at `"expected"` should be an object containing exactly one property, `"error"`, whose value is a string.


In this proposal, how should one differentiate between a missing value (optional return) and an error? Is it possible, or both should use the same structure?

This is also a point of attention to me. In #336 I suggest to use null as the value to represent a missing value. Languages with support for optional types can then use their optional type instead of null.

let's talk at #336 (comment) shall we!

Ah, I rubber-ducked myself into finding a reasonable answer. I've added the possibility of null, on the assumption that my guess is correct. I'm very open to corrections, of course.

ErikSchierboom · 2017-02-14T19:35:50Z

README.md

@@ -47,9 +47,14 @@ variations.

 Each test case has the the following keys:
 - description: which will be used to name each generated test
+  - the description should not simply explain **what** each case is (that is redundant information)
+  - the description should explain **why** each case is there
+  - for example, what kinds of incorrect implementations might this case help us find?


This line seems to belong to the line above (about the why). Should it really be on a new line?

you're right, there is no good reason to put it on another

We would also need to clean up remaining instances, so this is necessary but not sufficient for #401. This relates to discussions in #338 about how to distinguish between `null` and error cases.

This isn't used anymore. Grep for `msg` and the only instance seen is rail-fence-cipher, and it's used as an input in that case,o Evidence of our past intent to remove the `msg` key: * #163 * [2dee540][cmt1] * [1483fe9][cmt2] [cmt1]: 2dee540 [cmt2]: 1483fe9

ErikSchierboom · 2017-02-16T11:45:58Z

I think this looks great now!

rbasso · 2017-02-17T07:09:13Z

README.md

- expected: the expected result (this would be -1 when we expect an exception)
- msg: a nice message for the failing case
+- expected: the expected result
+  - if the input is valid but there is no result for the input, the value at `"expected"` should be `null`.


While the last proposal in #336 doesn't enforce the existence of a expected property, if it is present, it can now also be null.

ErikSchierboom · 2017-02-17T07:16:46Z

I think we can merge this now, right?

petertseng changed the title ~~README: bring expectation of canonical-data.json values up-to-date~~ README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date Feb 12, 2017

ErikSchierboom reviewed Feb 14, 2017

View reviewed changes

rbasso reviewed Feb 15, 2017

View reviewed changes

ErikSchierboom reviewed Feb 15, 2017

View reviewed changes

rbasso mentioned this pull request Feb 15, 2017

canonical-data.json standardisation discussion (was: Malformed data?) #336

Closed

petertseng added 3 commits February 15, 2017 20:16

README: explain principles for descriptions

add5995

README: Use null and new error value defined in #401

1f238c4

We would also need to clean up remaining instances, so this is necessary but not sufficient for #401. This relates to discussions in #338 about how to distinguish between `null` and error cases.

README: remove msg

9afe4c1

This isn't used anymore. Grep for `msg` and the only instance seen is rail-fence-cipher, and it's used as an input in that case,o Evidence of our past intent to remove the `msg` key: * #163 * [2dee540][cmt1] * [1483fe9][cmt2] [cmt1]: 2dee540 [cmt2]: 1483fe9

ErikSchierboom approved these changes Feb 16, 2017

View reviewed changes

petertseng mentioned this pull request Feb 17, 2017

Representation of undefined/exceptional values in canonical-data.json #401

Closed

rbasso approved these changes Feb 17, 2017

View reviewed changes

petertseng merged commit c4e23d0 into exercism:master Feb 17, 2017

petertseng deleted the readme-desc branch February 17, 2017 07:42

petertseng mentioned this pull request Feb 21, 2017

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

Merged

petertseng mentioned this pull request Sep 18, 2017

Discussion: Getting rid of 'null' and "error" in canonical-data? #905

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date #551

README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date #551

petertseng commented Feb 12, 2017

petertseng commented Feb 13, 2017

ErikSchierboom Feb 14, 2017

petertseng Feb 14, 2017

ErikSchierboom Feb 14, 2017

ErikSchierboom commented Feb 14, 2017

rbasso Feb 15, 2017

ErikSchierboom Feb 15, 2017

petertseng Feb 16, 2017

petertseng Feb 16, 2017

ErikSchierboom Feb 14, 2017

petertseng Feb 16, 2017

ErikSchierboom commented Feb 16, 2017

rbasso Feb 17, 2017

ErikSchierboom commented Feb 17, 2017

README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date #551

README: bring expectation of canonical-data.json values (description, errors, no msg) up-to-date #551

Conversation

petertseng commented Feb 12, 2017

petertseng commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikSchierboom commented Feb 14, 2017

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

Choose a reason for hiding this comment

ErikSchierboom commented Feb 16, 2017

Choose a reason for hiding this comment

ErikSchierboom commented Feb 17, 2017