Conversation
| (TestSuite.empty \"{exercise}\")" | ||
|
|
||
| def genTestCase (_exercise : String) (_case : TreeMap.Raw String Json) : String := | ||
| "" -- Zipper tests are handwritten due to complex operation chaining |
There was a problem hiding this comment.
The tests aren’t complex enough to justify hardcoding them. One of the main purposes of a generator is to make it easy to automatically add new test cases in the future, which is impossible with handwritten tests.
| /- | ||
| You should define: | ||
|
|
||
| 1. A `BinTree` inductive type representing a binary tree where each node |
There was a problem hiding this comment.
It is best if we don't mention "inductive" here. A binary tree may be defined in other ways and it is not the focus of this exercise. We should simply say a BinTree type.
| 1. A `BinTree` inductive type representing a binary tree where each node | ||
| has a value (Int), a left subtree, and a right subtree. | ||
|
|
||
| 2. A `Zipper` structure that allows navigating within a binary tree. |
| return assertEqual none result) | ||
| |>.addTest "left, right, and up" (do | ||
| let zipper := fromTree initialTree | ||
| let result := zipper.left >>= Zipper.up >>= Zipper.right >>= Zipper.up >>= Zipper.left >>= Zipper.right >>= (·.value) |
There was a problem hiding this comment.
We should serialize long sequences of operations in different lines.
| You should define: | ||
|
|
||
| 1. A `BinTree` inductive type representing a binary tree where each node | ||
| has a value (Int), a left subtree, and a right subtree. |
There was a problem hiding this comment.
Considering this is a harder exercise, I think we should make the tree polymorphic and add a few extra test cases for other types (maybe String).
Extra cases may be defined in an extra.json inside the .meta folder for the exercise. They are called in the same way as those in canonical data, but they don't need an uuid.
There was a problem hiding this comment.
-
Fixed. Rewrote the generator to properly parse the canonical data JSON (tree serialization, operation chaining, handling all expected types). Thanks for the feedback.
-
Fixed, changed to "BinTree type".
-
Fixed, changed to "Zipper type".
-
Fixed. Operations are now chained on separate lines using >>=.
-
Done. Made BinTree α and Zipper α polymorphic. Added 3 extra String test cases in .meta/extra.json (data retained, left/right/value, set_value).
|
Hello. Thanks for opening a PR on Exercism 🙂 We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in. You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch. If you're interested in learning more about this auto-responder, please read this blog post. Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it. |
oxe-i
added
x:size/large
|
The initial tree for the tests remains hardcoded, even though the extra cases have their own initial trees (which are simply disregarded). The extra cases are also in invalid format. They should be in a JSON array. This means the test file was not generated from the generator file using the generator script. Since this PR does not follow our contribution guidelines, it will be closed. Thank you for your contribution. |
2 participants
Closes #157