Conversation
Replace Dhall configuration with spago.yaml using registry-based package set 73.0.0. Update Docker image and test runner for Spago 1.0 compatibility. Key changes: - Dockerfile: node:22-bookworm-slim (Spago 1.0.3 needs node:sqlite), documented dependencies, npm cache cleanup - bin/run.sh: rewrite package name to match pre-compiled lockfile for --offline --pure mode, HOME=/tmp for read-only Docker, sanitize Spago 1.0 output (strip stack traces, new preamble format) - pre-compiled: spago.yaml replaces dhall files - Test examples: spago.yaml, regenerated expected_results.json Companion PR needed in exercism/purescript. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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. |
|
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.
Pull request overview
Migrates the test runner and its fixture projects from Spago 0.21 (Dhall + package-sets) to Spago 1.0.3 (YAML + Registry package set), updating the Docker runtime and runner scripts accordingly.
Changes:
- Replace per-test-project
spago.dhall/packages.dhallwithspago.yamlusing Registry package set73.0.0. - Update runner scripts to execute
spago test --offline --pure, copyspago.lock, and adjust output sanitization + expected test outputs. - Update Docker image base (Node 22) and installation steps for Spago 1.
Reviewed changes
Copilot reviewed 25 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/example-syntax-error/spago.yaml | New Spago 1 YAML config for fixture project. |
| tests/example-syntax-error/spago.dhall | Remove Spago 0.x Dhall config. |
| tests/example-syntax-error/packages.dhall | Remove Dhall package-set reference. |
| tests/example-syntax-error/expected_results.json | Update expected compiler error output formatting. |
| tests/example-success/spago.yaml | New Spago 1 YAML config for fixture project. |
| tests/example-success/spago.dhall | Remove Spago 0.x Dhall config. |
| tests/example-success/packages.dhall | Remove Dhall package-set reference. |
| tests/example-partial-fail/spago.yaml | New Spago 1 YAML config for fixture project. |
| tests/example-partial-fail/spago.dhall | Remove Spago 0.x Dhall config. |
| tests/example-partial-fail/packages.dhall | Remove Dhall package-set reference. |
| tests/example-partial-fail/expected_results.json | Update expected test failure output after sanitization changes. |
| tests/example-empty-file/spago.yaml | New Spago 1 YAML config for fixture project. |
| tests/example-empty-file/spago.dhall | Remove Spago 0.x Dhall config. |
| tests/example-empty-file/packages.dhall | Remove Dhall package-set reference. |
| tests/example-empty-file/expected_results.json | Update expected compiler error output formatting. |
| tests/example-all-fail/spago.yaml | New Spago 1 YAML config for fixture project. |
| tests/example-all-fail/spago.dhall | Remove Spago 0.x Dhall config. |
| tests/example-all-fail/packages.dhall | Remove Dhall package-set reference. |
| tests/example-all-fail/expected_results.json | Update expected test failure output after sanitization changes. |
| pre-compiled/spago.yaml | New Spago 1 YAML config for the precompiled dependency project. |
| pre-compiled/spago.dhall | Remove Spago 0.x Dhall config. |
| pre-compiled/packages.dhall | Remove Dhall package-set reference. |
| pre-compiled/package.json | Bump Spago devDependency to ^1.0.3. |
| bin/update-tests.sh | Update fixtures by rewriting workspace: section in spago.yaml files. |
| bin/run.sh | Switch runner to Spago 1 flow (--offline --pure), copy lockfile, and adjust output sanitization. |
| bin/run-tests.sh | Point test harness at spago.yaml fixtures instead of spago.dhall. |
| Dockerfile | Move to Node 22 base image and update dependency install/prep steps for Spago 1. |
| .dockerignore | Ignore generated pre-compiled/spago.lock in build context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Install PureScript dependencies and pre-compile them. | ||
| # The output/ and .spago/ directories are reused at runtime to avoid | ||
| # recompiling 280+ modules for every student submission. | ||
| COPY pre-compiled pre-compiled/ | ||
| RUN cd pre-compiled \ | ||
| && npm install \ | ||
| && npx spago install \ | ||
| && npm cache clean --force \ | ||
| && rm -rf /root/.npm |
There was a problem hiding this comment.
The image build no longer runs a Spago build step to generate compiled artifacts, but bin/run.sh expects pre-compiled/output to exist and be populated (it copies it to preserve purs cache timestamps). If npx spago install doesn’t produce output/, the runner will either fail the copy step or lose the intended precompilation speedup (and may time out). Consider adding an explicit npx spago build --deps-only (or the Spago 1 equivalent) after install to ensure output/ is created during docker build.
bin/run.sh
Outdated
| -e '/^\[WARNING /,/^$/d') | ||
|
|
||
| # Remove leading/trailing blank lines and collapse runs of 3+ blank lines | ||
| sanitized_spago_output=$(printf '%s\n' "${sanitized_spago_output}" | awk ' |
There was a problem hiding this comment.
Want to take a go at combining the sed and awk? The { a[NR] = $0 } could be matched with the above patterns to ignore the above regexes. You could also do this all in one pass in awk by ignoring the above patterns and tracking the number of blank lines printed. read or command substitution can be used to trim blanks.
stripped=$( awk '
BEGIN { blanks = 2 }
! (/a/ || /b/ || /c/ ... ){
if (NF) {
blanks = 0
print
} else if (blanks < 2) {
print
blanks++
}
}')
printf '%s\n' "$stripped"
Migrate from Spago 0.21.0 (Dhall) to Spago 1.0.3 (YAML + Registry) Replace Dhall configuration (spago.dhall, packages.dhall, .solution.dhall) with spago.yaml using registry-based package set 73.0.0 across all 32 exercises and the template. Key changes: - All exercises: spago.yaml replaces dhall files, invalidators updated - CI workflow: spago 1.0.3, removed psa - scripts/ci: pre-compile shared deps, copy example solutions to src/ (replaces .solution.dhall approach since Spago 1.0 has no custom sources) - scripts/ci-check: compare spago.yaml (stripping name field) - bin/update-exercises.sh: propagate template spago.yaml to exercises [Companion PR in exercism/purescript-test-runner](exercism/purescript-test-runner#64). [no important files changed]
Migrate from Spago 0.21.0 (Dhall config) to Spago 1.0.3 (YAML + Registry package set 73.0.0).
This PR supersedes the one I messed up earlier.
Companion PR for exercism repo: exercism/purescript#323