wordy: test that various syntax errors are in fact syntax errors #1383
Conversation
| The parser should reject: | ||
|
|
||
| * Unsupported operations ("What is 52 cubed?") | ||
| * Non-math questions ("Who is the President of the United States") |
There was a problem hiding this comment.
Even if this PR is rejected, I'd suggest adding these lines in any case!
| "description": "reject two operations in a row", | ||
| "property": "answer", | ||
| "input": { | ||
| "question": "What is 1 plus plus 2?" |
There was a problem hiding this comment.
let's ignore the fact that in some languages, 1 + + 2 is a valid expression that evaluates to 3. The reason for the validity is that the second + sign is a unary plus. We are probably assuming in these word problems that the plus refers only to the binary operation.
exercises/wordy/canonical-data.json
Outdated
| @@ -134,6 +134,38 @@ | |||
| "question": "Who is the President of the United States?" | |||
| }, | |||
| "expected": false | |||
There was a problem hiding this comment.
I think all instances of "expected": false should be converted to syntax errors in this PR.
There was a problem hiding this comment.
👍
will happen when rebasing on top of #1334
|
I'm going to add another case that I think will be useful to test: |
|
With regard to #902 - one very good argument against testing invalid inputs there was that we can imagine there is an additional layer above the problem that takes on the job of filtering out any invalid inputs. For problems like For a problem like |
By explicitly testing these we are making sure that the implementation fails in an appropriate way, rather than failing in an inappropriate way or erroneously giving an answer. For example, in languages using an optional for their answer, ensuring that [None is returned instead of panicking/aborting][1]. [1]: exercism/rust#704 (comment) This is beneficial because the problem is partially about building a parser (even if very rudimentary), and it's common for parsers to have to deal with errors like this. We may otherwise see some unfortunately fragile parsers.
|
Since I have an approval, if no further comments are forthcoming, one should expect me to merge this when 120 hours have passed since the PR was submitted. |
There was a problem hiding this comment.
@petertseng at first these changes bothered me. But I have had time to consider them and I think they are spot on. What you have done seems to me to dovetail nicely with the conversation that took place at #1346.
…#779) In keeping with [1.3.0][], we want to test that this fails in an expected way rather than an unexpected way. [1.3.0]: exercism/problem-specifications#1383 Specifically for Haskell, we want to make sure that this results in `None` rather than accessing an out-of-bounds index due to always assuming that the inputs will have at least three space-separated elements (of the form "What is X...?"). exercism/problem-specifications#1401
…764) In keeping with [1.3.0][], we want to test that this fails in an expected way rather than an unexpected way. [1.3.0]: exercism/problem-specifications#1383 Specifically for Rust, we want to make sure that this results in `None` rather than accessing an out-of-bounds index due to always assuming that the inputs will have at least three space-separated elements (of the form "What is X...?"). exercism/problem-specifications#1401
By explicitly testing these we are making sure that the implementation
fails in an appropriate way, rather than failing in an inappropriate way
or erroneously giving an answer.
For example, in languages using an optional for their answer, ensuring
that
Noneis returned instead of panicking/aborting.This is beneficial because the problem is partially about building a
parser (even if very rudimentary), and it's common for parsers to have
to deal with errors like this. We may otherwise see some unfortunately
fragile parsers.
Intentional merge conflict for the same reason as #1382