-
Notifications
You must be signed in to change notification settings - Fork 10
Investigate Integration Test Flakiness #528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
9fb887c
7ebe816
ed705fb
4bb3d04
e1b2543
6ec91f0
d68c8db
af82046
022a0f3
9a5e6a3
36a8d2f
4f7a466
d11000d
fa40be7
fc949eb
0530b96
d53b43f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,7 +97,28 @@ | |
| function atomicWriteFile(filePath, content) { | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this from a merge or something? I swear this retry logic was proposed in another PR. |
||
| while (true) { | ||
| try { | ||
| fs.renameSync(tempPath, filePath); | ||
| break; | ||
| } catch (err) { | ||
| if (retries > 0 && (err.code === 'EPERM' || err.code === 'EACCES')) { | ||
| retries--; | ||
| // sleep synchronously to allow the reader to close the file | ||
| const start = Date.now(); | ||
| while (Date.now() - start < 10) {} | ||
| continue; | ||
| } | ||
| // if it fails we should clean up the tmp file | ||
| try { | ||
| fs.unlinkSync(tempPath); | ||
| } catch (cleanupErr) { | ||
|
kriszyp marked this conversation as resolved.
Outdated
|
||
| // ignore cleanup errors | ||
| } | ||
| throw err; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the integration tests running on Node 24, we can use |
||
| import { describe, it, beforeEach } from 'node:test'; | ||
| import assert from 'node:assert/strict'; | ||
| import { setTimeout } from 'node:timers/promises'; | ||
|
|
@@ -95,7 +98,7 @@ describe('15. Custom Functions & components', () => { | |
|
|
||
| it('add_component', () => { | ||
| return req() | ||
| .send({ operation: 'add_component', project: 'add-test' }) | ||
| .send({ operation: 'add_component', project: 'add-test', template: process.platform === 'win32' ? 'file:' + join(__dirname, '../../fixtures/application-template-1.0.0.tgz') : undefined }) | ||
| .expect((r) => assert.equal(r.body.message, 'Successfully added project: add-test', r.text)) | ||
| .expect(200); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, yet related, we probably only want to set
shell: trueon Windows and if we're settingshell: true, then might as well setwindowsHide: true: