Improve e2e test CI reliability - #301
Open
benthecarman wants to merge 4 commits into
Open
benthecarman wants to merge 4 commits into
benthecarman wants to merge 4 commits into
Conversation
|
👋 Thanks for assigning @tankyleo as a reviewer! |
benthecarman
force-pushed
the
fix-e2e-ci-reliability
branch
from
September 29, 2026 05:13
015181e to
5047dc1
Compare
f3r10
reviewed
Sep 29, 2026
f3r10
left a comment
f3r10
left a comment
Contributor
There was a problem hiding this comment.
LGTM. Just one comment, about keeping both ports reserved until the child is ready to take them over.
| std::fs::write(&config_path, &config_content).unwrap(); | ||
|
|
||
| // Keep both ports reserved until the child is ready to take them over. | ||
| drop((grpc_listener, p2p_listener)); |
Contributor
There was a problem hiding this comment.
I'm not sure the comment above matches the code.
It says both ports stay reserved until the child is ready to take them over. The listeners are dropped first, and only then does spawn_server_process start ldk-server. After that drop, is anything still holding the ports, or are they free until the child binds them?
I ran the end-to-end tests and they passed, so I can't show that this gap causes a failure. I just want to check whether the comment describes what the code does.
Keep test listeners outside the ephemeral range and prevent port reuse within a test process. Fail promptly when a server exits during startup and reap children even when initialization fails. Give PostgreSQL fixtures unique table names across test processes so reusing a port does not reuse database state. AI assistance: OpenAI Codex.
Keep a second channel open so LDK Node does not reconnect the peer while its force-close notification is being delivered. Require the exact local and remote closure reasons and verify the other channel stays usable. Wait for funding and both channels to be usable before closing, then observe closure events before mining. Remove the alternate processing error handling from the test. AI assistance: OpenAI Codex.
Channel opening returns before its funding transaction is broadcast. Wait for the transaction before mining announcement confirmations, then require that exact channel to be usable on both peers. Existing channels must not satisfy readiness for a newly opened channel. Include routing updates in gossip timeout diagnostics. AI assistance: OpenAI Codex.
An already-claimed HTLC can leave the commitment during restart, lowering the closing fee and increasing the claimable balance by the same amount. Compare exact funds per channel including that fee after outstanding outgoing claims resolve, while retaining the crash-recovery scenario. Cover the observed 43-satoshi fee change and ensure a one-satoshi loss still fails the funds comparison. AI assistance: OpenAI Codex.
benthecarman
force-pushed
the
fix-e2e-ci-reliability
branch
from
September 29, 2026 18:32
5047dc1 to
984772b
Compare
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
3 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.
Few things codex found to improve the e2e test reliability on CI:
within a test process.
reusing a port does not reuse database state.