Report a crashed backend instead of a wall of connection failures - #100
Merged
Conversation
pgrep matches as soon as carta_backend forks, which is well before it binds the port, and `pgrep ... | head -n 1` reports head's exit status, so a backend which died on startup still looked like a success. The macOS action ran `pgrep carta_backend` after each test file but only printed the result. Neither action could tell a dead backend from a failing test. Connect to the port instead, which is the only check that answers the question the tests ask, using bash's own /dev/tcp so no container needs a tool it may not have. Wait up to 120 s for it before the stage starts, and probe again after each test file: a backend which dies part way through takes every remaining file with it, each reporting a connection failure rather than the crash which caused it. The stage now names the file it died on and tails the backend log where it failed, instead of leaving the reason in an uploaded artifact.
pshnghng0318
approved these changes
Sep 1, 2026
pshnghng0318
left a comment
pshnghng0318
left a comment
Contributor
There was a problem hiding this comment.
The changes look good.
This PR allows the ICD tests to check test failures and backend crashes independently.
If the backend crashes, it can be restarted to continue running the remaining tests.
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
2 participants
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.
Description
Fixes #97. Each ICD stage runs the files in its
.testslist, one pernpm test, against a single long-lived backend. If that backend dies partway through, every remaining file reports a connection failure, and nothing in the log says the backend is gone, which file killed it, or what it said on the way out, so the failure you read is the one place the problem is not.This branch makes readiness a property of the port rather than of the process, restarts a backend that dies mid-stage, reports crashes and skipped files separately from real test failures, and points the workflows at the actions in this repository so that any of it actually runs.
Changes
Three scripts hold the harness, and both actions call them:
wait_for_backend.sh: readiness is a property of the port, asked with bash's own/dev/tcprather thanpgrep. 120 s before the first test, one immediate check after every test file, and the tail of the backend log on failure.start_backend.sh: one copy of the launch command line, so a stage can bring a crashed backend back with the invocation it was started with. Clears the port first.run_test_stage.sh: the stage loop, once, instead of one copy per action. Restarts a crashed backend up to 3 times, retries a failed file once, and reports crash sites, retried passes, skipped files, and real failures as four separate lists. A crash fails the stage even if everything that ran passed.Around them:
./ICD-RxJS/.github/actions/, which is what makes any of the above.--portargument rather than by any mention ofcarta_backendand the port, which had matched (and killed) the Apptainer process running the stage itself.matrix.porton both sides, as it already did on Linux, instead of from two literals that had to agree.Restarting is safe because the tests hold no state in the backend: every file opens its own connection and loads its own images, and each
npm testis a separate Jest process.Checklist
For the pull request:
Documentation has been updated (orno documentation changes are needed)