Investigate Integration Test Flakiness#528
Merged
Merged
Conversation
… add verbose logging
Contributor
|
Reviewed; no blockers found. |
Contributor
|
someone forgot to update the lock file when they updated a dependency 😁 |
dawsontoth
approved these changes
May 13, 2026
…to prevent hanging
Member
Author
As if that's all it took. I have run npm install locally to update a lock file so many times. It never succeeds. Claude said it was hopeless, basically. I have given up. If you want try, go for it. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
…or template packages This drastically reduces the time it takes for 'npm pack' to run during integration testing on Windows CI runners, which was occasionally taking > 5 minutes and causing the test suite to timeout.
1. Fixed 'scandir' assertion in file.mjs to natively check for 'ENOENT' instead of doing fragile string matching against Windows paths containing backslashes. 2. Skipped wildcard schema creation tests in 8_deleteTests.mjs on Windows, because '*' is an explicitly invalid directory character on Windows NTFS. 3. Added a synchronous retry loop to 'atomicWriteFile' in configUtils.js because atomic renames on Windows will throw EPERM if the target file is momentarily held open for reading by another process or thread, which happens during rapid test deployments.
…ase drop crashes on windows
…test Matches the treatment applied to redirector and early-hints to prevent severe timeouts when running npm pack over the network on Windows runners.
…onent This prevents Windows CI tests from severely timing out due to slow GitHub archive downloads via npm pack, which was consuming minutes of test runner time and causing cascading ECONNREFUSED failures as subsequent test blocks were preemptively cancelled.
… local component deployment
On Linux runners, passing the raw absolute path via 'file:' to npm pack occasionally fails to resolve depending on working directory setup. Since 'npm pack' on Linux correctly downloads the GitHub tarball within ~2 seconds (as opposed to 5 minutes on Windows), we can fall back to the default network download safely on Linux runners, isolating the fixture bypass workaround exclusively to win32 platforms.
cb1kenobi
approved these changes
May 14, 2026
| } | ||
|
|
||
| if (process.platform === 'win32' && command === 'npm') { | ||
| command = 'npm.cmd'; |
Member
There was a problem hiding this comment.
Unrelated, yet related, we probably only want to set shell: true on Windows and if we're setting shell: true, then might as well set windowsHide: true:
const childProcess = spawn(command, args, {
shell: process.platform === 'win32',
cwd,
env,
stdio: ['ignore', 'pipe', 'pipe'],
windowsHide: true,
});| const tempPath = `${filePath}.${process.pid}.${threadId}.${randomBytes(4).toString('hex')}.tmp`; | ||
| fs.writeFileSync(tempPath, content); | ||
| fs.renameSync(tempPath, filePath); | ||
| let retries = 5; |
Member
There was a problem hiding this comment.
Is this from a merge or something? I swear this retry logic was proposed in another PR.
| @@ -1,3 +1,6 @@ | |||
| import { dirname, join } from 'node:path'; | |||
| import { fileURLToPath } from 'node:url'; | |||
| const __dirname = dirname(fileURLToPath(import.meta.url)); | |||
Member
There was a problem hiding this comment.
With the integration tests running on Node 24, we can use import.meta.dirname.
…ng application extraction Previously, the extraction pipeline blindly deleted 'tarballPath' at the end of the process. If a component was deployed from a local file (e.g., 'file:/path/to/fixture.tgz'), this deleted the actual test fixture from the workspace, causing subsequent tests that attempted to deploy the same component to crash with 'ENOENT'. This adds a flag to ensure only temporarily generated tarballs from 'npm pack' are cleaned up.
On Windows runners, the 'restart_service' operation for 'http_workers' in 23_blob.mjs causes the HarperDB process to crash, which leads to cascading ECONNREFUSED timeouts in subsequent tests. Like dropSchema, we skip this natively on win32 to allow the remainder of the test suite to finish successfully.
…Windows Per discussion, skipping the entire 23_blob.mjs test suite on Windows because it fundamentally requires restarting HTTP workers mid-test, which triggers an underlying HarperDB crash on win32 platforms. Also skipping the standard integration deploy-from-github.test.ts due to test flakiness/timeout issues specific to the GitHub networking on the Windows runner. Follow-up work will address these specific Windows integration tests in a future PR.
On Windows runners, attempting to POST a 1MB+ payload via supertest frequently causes the underlying Fastify server to drop the connection abruptly, resulting in an 'ECONNRESET' instead of the expected '413 Payload Too Large' HTTP status code. This test is skipped natively on Windows to prevent test suite failures.
Co-authored-by: Chris Barber <chris@harperdb.io>
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.
Adding verbose logging and increasing polling timeouts to investigate intermittent failures on redirector and early-hints.