Upgrade test runner to interface version 3 (take 2) #9
Conversation
glennj
added
x:module/test-runner
| test_dir_path=$(realpath "${test_dir}") | ||
|
|
||
| bin/run.sh "${test_dir_name}" "${test_dir_path}" "${test_dir_path}" | ||
| bash -x bin/run.sh "${test_dir_name}" "${test_dir_path}" "${test_dir_path}" |
There was a problem hiding this comment.
This verbose debugging output can be removed once we go live. I think not being able to deploy the image was causing me problems before.
This debugging only clutters up the output for the GitHub workflow. Does not affect student solutions
rmonnet
left a comment
rmonnet
left a comment
There was a problem hiding this comment.
I tested bin/run-tests-in-docker.sh on Mac M1 Sequoia 15.7.2 and on Ubuntu 22.04 running an antique 'i7-3632QM CPU @ 2.20GHz' from 2012, both ran the tests successfully.
My comments are more questions than anything.
| @(test) | ||
| test_degenerate_case_with_a_single_a_row :: proc(t: ^testing.T) { |
There was a problem hiding this comment.
I am assuming you didn't set a description or a task id tag here because you tested that on the 'partial-fail' test.
There was a problem hiding this comment.
Right. A couple of points:
- I default the description to the proc name if the comment is missing
- practice exercises don't have tasks. I haven't really thought about enforcing concept exercises to have them, although that's really a job for the check_exercise script.
| @@ -0,0 +1,36 @@ | |||
| { | |||
| "version": 3, | |||
There was a problem hiding this comment.
Is it still a version 3 if we don't populate the tasks id, or do we need to use version 2 for the exercises which don't have taskid (aka practice exercises) even if the test runner is version 3 capable? In other words, is the version in a test report indicating what is contained in the test report (w/ the difference between 2 and 3 being the inclusion of taskids) or is the version in the test report the capability of the test runner?
I tried to look-up the documentation but it is not clear to me.
There was a problem hiding this comment.
It's OK to have the version 3 test-runner interface with practice exercises.
| { | ||
| "version": 1, | ||
| "status": "pass" | ||
| "version": 3, |
There was a problem hiding this comment.
Same question as above about the version in the report since it doesn't contain taskids.
|
|
||
| # Fix a bug in this Odin release. When we upgrade, revisit this. | ||
| RUN sed -E -i '983,984s/\<err\>/marshall_err/g' /src/Odin/core/testing/runner.odin | ||
| RUN sed -E -i '983,984s/\<err\>/marshal_err/g' /src/Odin/core/testing/runner.odin |
There was a problem hiding this comment.
Should be fixed in the next version, here is the commit on the Odin repo odin-lang/Odin@31817be
There was a problem hiding this comment.
The Dockerfile encodes the dev-2025-10 version. When we upgrade that, we'll remove that RUN command.
Oddly, I couldn't reply to that directly. I was having odd results on the previous PR. I figured I needed to have the docker image rebuilt for this PR, and not retrieved from cache. The easiest way I know how to do that is to make a change in the first RUN command. That will force docker to run everthing fresh. jq is not needed in the "build" image, which is why it wasn't there before. No harm to add it, but it will just make the overall image larger which we want to avoid for Exercism's $$$. |
|
Ok, understand the response to my jq question. I saw your previous comment and was wondering how you force the image to rebuild. |
3 participants
The first attempt (that I closed) was mysteriously failing: odin test was aborting with exit status 132. I eventually guessed that I have to force the image to be rebuilt, and this seems to have done the trick.