test: mark test-file-write-stream4 as flaky#57927
test: mark test-file-write-stream4 as flaky#57927joyeecheung wants to merge 1 commit intonodejs:mainfrom
Conversation
The flake has been detected since 2021-10-30 and still has been affecting recent PRs significantly (failing 8 PRs out of the last 100 CI run up until 2025-04-18). The underlying cause is still under investigation but in the meantime it's better to mark it as a known flake to keep the CI functional.
|
If we have to do this, I would prefer to manually call the GC when the test finishes, and add a TODO comment to remove it when #54918 is fixed. |
|
I mean something like this diff --git a/test/parallel/test-file-write-stream4.js b/test/parallel/test-file-write-stream4.js
index 6b3862fa714..1aa538f7781 100644
--- a/test/parallel/test-file-write-stream4.js
+++ b/test/parallel/test-file-write-stream4.js
@@ -1,3 +1,4 @@
+// Flags: --expose-gc
'use strict';
// Test that 'close' emits once and not twice when `emitClose: true` is set.
@@ -17,4 +18,8 @@ const fileWriteStream = fs.createWriteStream(filepath, {
});
fileReadStream.pipe(fileWriteStream);
-fileWriteStream.on('close', common.mustCall());
+fileWriteStream.on('close', common.mustCall(() => {
+ // TODO(lpinca): Remove the forced GC when
+ // https://github.com/nodejs/node/issues/54918 is fixed.
+ globalThis.gc({type: 'major'});
+}));
The advantage is that the test is still testing what it is supposed to test. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #57927 +/- ##
========================================
Coverage 90.24% 90.24%
========================================
Files 630 630
Lines 185705 185933 +228
Branches 36407 36450 +43
========================================
+ Hits 167591 167803 +212
+ Misses 11002 10985 -17
- Partials 7112 7145 +33 🚀 New features to boost your workflow:
|
|
I remember that @jakecastelli mentioned that doing GC on shutdown doesn't actually make the flake go away? |
I missed that. Can you find where? |
Reduce `test/parallel/test-file-write-stream4.js`. Refs: nodejs#57927
Reduce `test/parallel/test-file-write-stream4.js`. Refs: nodejs#57927
Reduce `test/parallel/test-file-write-stream4.js` flakyness. Refs: nodejs#57927
Reduce `test/parallel/test-file-write-stream4.js` flakiness. Refs: nodejs#57927
|
Hi folks, I think @joyeecheung meant a private slack direct message where I reached out to her when I found the manual gc from cc side didn't resolve the flakness of the test. But it was my fault where I triggered the gc at the wrong spot. Later on I came up with this solution but we decided to not go with it. Hopefully this helps @lpinca |
|
This seems to have been gone after the V8 upgrade, or if not #58047 should make it less likely to appear. |
The flake has been detected since 2021-10-30 and still has been affecting recent PRs significantly (failing 8 PRs out of the last 100 CI run up until 2025-04-18). The underlying cause is still under investigation but in the meantime it's better to mark it as a known flake to keep the CI functional.
This flake affects at least Windows, Linux and macOS, according to https://github.com/nodejs/reliability/blob/main/reports/2025-04-18.md and likely affects all platforms. So the status is applied to all platforms in this PR.
Refs: nodejs/reliability#102
Refs: https://github.com/nodejs/reliability/blob/main/reports/2025-04-18.md
Refs: #56883
Refs: #54918