pov 1.1.0: Make exercise schema-compliant #704
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
That is better. By the way, you could maybe convince me to make some of those "cousins" replacements smaller. As I explained in the commit message, it's not really interesting to have a large tree for an error case. |
In #699 we discussed that referring to trees by name rather than embedding has a few costs: * Requires schema change. * `shared` or `resources` section must be exercise-dependent by nature. * More complex test generators. * Harder for a human to understand the test (must refer back and forth between the individual case and the "trees" listing). We have decided this is not worth it. Extra work on the `cousins` tree is required to reduce its usage, so it will not yet be embedded.
We would like to reduce the tree so that: 1. It's clearer what is being tested 2. The duplication in the test file is kept to fewer lines This helps us be rid of the top-level `"trees"` key as desired in #699. The usages of "cousin" that were replaced: **Three error cases**: 1. "Errors if target does not exist in a large tree" 2. "Errors if destination does not exist" 3. "Errors if source does not exist" In all three cases, a "large" tree is all that is necessary. It doesn't necessarily have to be the entire "cousin" tree. Besides, it's not very interesting to see whether a student can *fail* to find a nonexistent node in a large tree, because the student can just as easily achieve it by not doing any searching at all. It's much more intersting to see whether the student can find an existent node in a large tree. So these error cases really don't need a very large tree. A "large" tree is all that is needed, but it doesn't necessarily have to so big so as to be the entire "cousin" tree. **One case of "Can find path from nodes other than x"**: Nothing in this case requires the use of the specific "cousins" tree. Since the test contents have now changed, this is now version 1.1.0.
|
I've been suggested to bump this to 1.1.0 By the way, I prefer this NOT be squashed on merge. |
|
Q: You changed the test inputs. How do we know the new outputs match the new inputs? The schema check only checks for conformance to the schema, and not the actual correctness of the test data. A: I did it before, and I'll do it again. https://github.com/petertseng/x-common/blob/verify/exercises/pov/verify.rb |
75 tasks
rbasso
approved these changes
Mar 11, 2017
petertseng
added a commit
that referenced
this pull request
Mar 12, 2017
petertseng
changed the title
petertseng
added a commit
to exercism/haskell
that referenced
this pull request
May 9, 2017
This was the main change in exercism/problem-specifications#704 exercism/problem-specifications#710 is an inconsequential change that does not affect our tests. Two other changes in 1.1.0 that aren't exactly followed: * As a part of conforming to the canonical schema, trees were embedded into the test data, but we have no need to do it - let's reduce duplication, and besides we are safe from accidental mutation. * The "cannot trace" cases were changed to use simpler trees than cousins, but we would have had to introduce another tree to do that (one with kids and siblings). We'll stick with cousins.
emcoding
pushed a commit
that referenced
this pull request
Nov 19, 2018
config.json: Add blurb, fix Gitter name
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
You probably want to review commit-by-commit (but hey don't let me tell you what to do, if you want to review the whole thing at once you are free to do it!).
I suppose there can still be a reformatting too (you'll notice a
"cases": [{) line but...testGroupis necessarily and both types that might be elements of it are objects, I have absolutely no compunction about putting the[{on the same line!cases, sorry.As part of the work in #625