Repository navigation
Making the test runner Guile-only - #79
Conversation
|
Hello 👋 Thanks for your PR. This repo does not currently have dedicated maintainers. Our guardians team will attempt to review and merge your PR, but it will likely take longer for your PR to be reviewed. If you enjoy contributing to Exercism and have a track-record of doing so successfully, you might like to become an Exercism maintainer for this track. Please feel free to ask any questions, or chat to us about anything to do with this PR or the reviewing process on the Exercism forum. (cc @exercism/guardians) |
There was a problem hiding this comment.
🔵 Needs a closer look
It is a large runtime migration with a hand-written pretty-printer whose exact output can only be confirmed by running Guile against the fixtures, so final correctness needs human verification.
0 open findings
What changed in this PR
This PR converts the Scheme test runner from a dual Chez+Guile design to Guile-only. The old runner ran each solution under both Chez and Guile and picked the result with fewer failures; it relied on Chez's pretty-printer for formatting the test_code field. The new code/exercise.scm runs the solution's test.scm only under Guile, and re-implements the result parsing, JSON writer, and a custom compact pretty-printer that mimics Chez's output so the regenerated fixtures stay close to the previous format. Supporting infrastructure (Dockerfile, env.sh, run.sh, README) is updated to drop Chez, and the Chez-specific C-linking fixture is removed in favor of the existing Guile one.
Changes:
- Replace
code/exercise.ss(Chez) withcode/exercise.scm(Guile), including a custom pretty-printer and a corrected JSON writer. - Update
Dockerfile,bin/env.sh, andbin/run.shto remove Chez and invokeguiledirectly. - Convert
README.orgtoREADME.md, remove the Chez C-linking fixture, and regenerate affectedexpected_results.jsonfiles to match Guile's error/pretty-print output.
| File | Description |
|---|---|
| code/exercise.scm | New Guile-only runner: process exec, result parsing, custom pretty-printer, JSON writer. |
| code/exercise.ss | Removed old Chez implementation. |
| bin/run.sh | Invokes guile code/exercise.scm instead of piping into scheme. |
| bin/env.sh | Drops dynamic Chez scheme.h discovery; keeps Guile include path. |
| Dockerfile | Removes chez-scheme package, symlink, and static archive cleanup. |
| README.md / README.org | Replaces Org README with Markdown; documents Guile-only plan and a fixture gotcha. |
| .gitignore | Ignores CLAUDE.md. |
| tests/collatz-partial-failure-with-debug/collatz-conjecture.scm | Adds #f destination to format for Guile compatibility. |
| tests/.../expected_results.json (several) | Regenerated to match Guile error messages and fill-formatting. |
| tests/building-and-linking-c-chez/* | Removed Chez C-linking fixture (replaced by -guile). |
I did not find concrete, blocking defects in the changed lines. The new json-write is actually more robust than the old one (string-null? instead of (eq? x "")), the exception handling uses valid Guile 3 constructs (with-exception-handler … #:unwind? #t), and the removed-Chez changes are consistent across Docker/scripts/README. The unchanged fixtures imply the Guile output matches the old Chez output for those cases, which the CI fixture diffs validate end-to-end.
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
FWIW, Copilot seems pretty happy with Claude's work. Given the friction between the respective companies, this is probably not the default. |
IsaacG
left a comment
IsaacG
left a comment
There was a problem hiding this comment.
I don't speak Scheme but if all the example solutions pass with this version of the test runner, that's a pretty good sign this is working!
| tests/**/*.o | ||
| tests/**/*.so | ||
|
|
||
| CLAUDE.md |
There was a problem hiding this comment.
Ideally, no. I am trying to get git to ignore these files globally, but haven't yet succeeded.
It's a bit of noise (hopefully temporary), but a lot less serious than committing the file, I think?
|
I'll delay merging this until tomorrow.
|
colinleach
changed the title
|
Though I am not sure if my experience is related to the recent change to the Scheme test runner, I have encountered a strange error. Today, I submitted my solution to the list ops exercise of the Scheme track and got a terse error message "syntax error". What is weird is that Guile 3.0 flawlessly runs the same code on my Linux box (and Chez does, too, if you care). I've also tried submitting |
|
Thanks for letting me know, @borderite, and apologies for the problem. There is something strange about the new List Ops exercise. Like you, I see it work perfectly in local testing, and also in GitHub CI. However, yesterday I tried it in the Exercism online editor (with the old test runner) and saw the deeply unhelpful "syntax error" message. I hoped the test runner change would make things better. It hasn't, but also not worse: different test runners, same problem. This is proving hard to diagnose. I'll keep trying. Meanwhile, I tried several other exercises in the online editor, and they all work well with both old and new test runners. |
|
You are welcome. |
Removes all the Chez Scheme aspects of the test runner, as discussed on the Forum.
Quite a big PR, and straying far outside my expertise. I admit that I relied on Opus 5.5 to do a lot of the work (Jeremy would probably approve, I'm not sure about other maintainers, and I retain a sense of guilt).
Please view this with a skeptical eye. I'm sure @BNAndras will have an opinion, and (other?) @exercism/guardians too.
The old version used Chez for pretty-printing test results.
code/exercise.scmnow includes a custom pretty-printer for Guile, which tries to match the Chez output as closely as possible.