INCATools / ontology-development-kit

Bootstrap an OBO Library ontology
http://incatools.github.io/ontology-development-kit/
BSD 3-Clause "New" or "Revised" License
212 stars 53 forks source link

`run.sh` should return an error code upon failure #981

Closed gouttegd closed 5 months ago

gouttegd commented 5 months ago

The run.sh wrapper script may return an exit code of zero (indicating success) even when the workflow ends up in failure.

That’s because the run.sh script ends like this (excerpt simplified for brevity):

docker run $ODK_DOCKER_OPTIONS $BIND_OPTIONS -w $WORK_DIR \
           --env-file run.sh.env \
           --rm -ti obolibrary/$ODK_IMAGE:$ODK_TAG $TIMECMD "$@"

case "$@" in
*update_repo*|*release*)
    echo "Please remember to update your ODK image from time to time: https://oboacademy.github.io/obook/howto/odk-update/."
    ;;
esac

If the name of the workflow does not match *update_repo* or *release*, then the docker run command is the last executed command, and the return code of the script is the return code of that command (so if the workflow fails, the script itself fails) – all good.

But if the name of the workflow does match *update_repo* or *release*, then regardless of of the outcome of the docker run command, the last command executed by the script is the echo command. This means the script will unconditionally returns zero (the return value of echo) even if the docker run command (and thus, the actual workflow) failed.

This is devastating for any ontology that uses, e.g., make prepare_release as the test command in its CI check, because it means the CI check will never report failure!

The simplest fix is to add a set -e at the beginning of the script to ensure that it exits immediately upon any error. Alternatively, add an explicit check of the return code of the docker run command before echoing the “please remember to update” message.

matentzn commented 5 months ago

+1 on adding set -e to the run.sh file. I think this is all that is needed.