fix: properly close HTTP server connections in test cleanup#614
Closed
fix: properly close HTTP server connections in test cleanup#614
Conversation
In Node.js 19+, server.close() alone doesn't close existing keep-alive connections. The test HTTP servers need to call closeAllConnections() before close() to ensure all connections are properly terminated. This fixes the CI timeout issue where tests would pass but the process would hang indefinitely waiting for connections to close. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The CLI tests were hanging in CI because the Node.js process wouldn't exit after tests completed. This adds a wrapper script that spawns entail and force-exits after a short grace period when tests complete. This is a more robust solution than trying to close all connections manually, as it handles any case where the event loop remains active. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous wrapper approach didn't work because spawn waits for the child process to exit. This version monitors stdout for the test summary (Duration:) and if the process hasn't exited after 30 seconds, it force exits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous approach using shell: true caused stdout buffering issues where the test output would only be seen after the shell exited. Since the underlying HTTP connections prevent exit, the wrapper's timeout detection never triggered. This change: - Spawns entail directly with Node.js instead of via shell - Adds a 10-minute hard timeout as a failsafe - Adds a 5-second post-completion timeout to force exit after tests done 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace the custom entail test framework with Mocha, which is already used by 10+ packages in the monorepo. This fixes the CI timeout issue where tests would pass but the process would hang indefinitely waiting for HTTP keep-alive connections to close. Key changes: - Convert bin.spec.js and lib.spec.js to Mocha format (.test.js) - Use beforeEach/afterEach to create fresh context per test - Properly close HTTP connections with closeAllConnections() in teardown - Remove test/run.js wrapper that was a workaround attempt - Update package.json to use mocha test runner 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
1 participant
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.
don't review this yet - waiting to see if it actually fixes the issue