exercism / generic-test-runner

GNU Affero General Public License v3.0
2 stars 10 forks source link

First automated PR on newly created jq-test-runner fails #80

Closed glennj closed 1 year ago

glennj commented 2 years ago

This is https://github.com/exercism/jq-test-runner/pull/1 "Sync org-wide files to upstream repo"

The CI workflow fails:

https://github.com/exercism/jq-test-runner/runs/8211450383?check_suite_focus=true

bin/run.sh: line 56: jq: not found
...
Error: Process completed with exit code 1.

@ErikSchierboom @iHiD

glennj commented 2 years ago

It seems the straightforward fix is to uncomment this from the Dockerfile

FROM alpine:3.10

# TODO: install packages required to run the tests
# RUN apk add --no-cache jq coreutils
glennj commented 2 years ago

But digging one shovel-ful deeper, the tests are going to fail (by design) because the run.sh script calls a non existing RUN_TEST_COMMAND.

The real approach is to keep exercism-bot away until the test-runner is declared "active" (somehow).

ErikSchierboom commented 1 year ago

@glennj What do you think about enabling jq and then commenting RUN_TEST_COMMAND?

glennj commented 1 year ago

How about

# Run the tests for the provided implementation file and redirect stdout and
# stderr to capture it
test_output=$(
    echo "REPLACE THESE LINES WITH YOUR COMMAND TO RUN THE TESTS!" 2>&1
    false
)
# You'll want something like this, substituting the actual command of course
#   test_output=$(command_to_run_tests 2>&1)

That will also provide a non-zero exit status for the variable assignment, giving the status: "fail" result.

ErikSchierboom commented 1 year ago

What about:

# Run the tests for the provided implementation file and redirect stdout and
# stderr to capture it
test_output=$(false)
# 
# TODO: substitute "false" with the actual command to run the test:
# test_output=$(command_to_run_tests 2>&1)

This is slightly shorter and to me reads a bit better. Thoughts?

glennj commented 1 year ago

Yes! Perfect.

If you assign to me, I can submit a PR

ErikSchierboom commented 1 year ago

Done

glennj commented 1 year ago

I just had a thought. I raised this issue because the "sync org wide files" PR fails CI for new tooling repos. There's really no reason for this PR to fail: we do want to sync those files.

Should we use test_output=$(true) to make things pass until the track maintainers get around to updating their run.sh?

EDIT: no that's not right: the run.sh script still completes successfully if the test status is "fail". false it is then.