fix(runner): recognise runtime errors from the exit code, not only English text - #1000
Merged
Merged
Conversation
…glish text Closes #998. detect_runtime_error matched English phrases -- "command not found" and about twenty more. bash translates its diagnostics, so under any locale with the message catalogue installed none of them matched and a genuine failure-to-run was reported as a plain assertion failure, losing the Error/Failed distinction that exists to tell those apart. The repo runs Spanish, Brazilian and Japanese locale jobs, so this was a supported configuration, not a hypothetical. Exit codes carry the same fact without any text: the shell reserves 127 for "could not find it" and 126 for "found it, could not run it". Both are consulted only after the text scan draws a blank, so English behaviour is unchanged -- including the more specific message the text produces, which is still preferred when available. The gain is wider than the locale case. A test that redirects the diagnostic away still exits 127: function test_error() { set -e invalid_function_name arg1 arg2 &>/dev/null } That is a real fixture in this suite. Its Error message used to be empty, because the text the scan needed had been sent to /dev/null; it now reads "command not found (exit code 127)". Snapshot updated to match, and the diff is worth reading -- it replaces an empty message with the cause. Two structural notes. extract_result_counts now runs before detect_runtime_error so the exit code is available to it; that is safe because it is documented and verified as a pure read that commits nothing. And the line scan moved into its own helper: it used to `return` from the caller on a text miss, which would have skipped the new fallback entirely. 1717 sequential / 1676 parallel; baseline + 4, RED first.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #998
detect_runtime_errormatched English phrases —command not foundand ~20 more. bash translates its diagnostics, so under any locale with the message catalogue installed none of them matched, and a genuine failure-to-run was reported as a plain assertion failure — losing theError/Faileddistinction that exists to tell those apart.The repo runs Spanish, Brazilian and Japanese locale jobs, so this was a supported configuration, not a hypothetical.
💡 Changes
Exit codes carry the same fact without any text: the shell reserves 127 for "could not find it" and 126 for "found it, could not run it". Both are consulted only after the text scan draws a blank, so English behaviour is unchanged — including the more specific message the text produces, which is still preferred when available.
🎁 Wider than the locale case
A test that redirects the diagnostic away still exits 127. This is a real fixture in the suite:
Its
Errormessage used to be empty — the text the scan needed had gone to/dev/null. It now readscommand not found (exit code 127). The snapshot diff is worth reading: it replaces an empty message with the cause.🔧 Two structural notes
extract_result_countsnow runs beforedetect_runtime_errorso the exit code is available to it — safe because it's documented and verified as a pure read that commits nothing.returnfrom the caller on a text miss, which would have skipped the new fallback entirely. Caught because the new tests stayed red after the first implementation.✅ Verification
4 new tests, RED first, covering: translated text + 127, translated text + 126, an ordinary failure exit code that must not classify, and English text still winning over the code.
make sa·make lint·bash build.sh bin -v→✅ Build verified ✅· 1717 sequential / 1676 parallel-simple-strict.