Skip to content

test: deflake test-fastutf8stream-periodicflush - #67355

Open
christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:deflake-periodicflush
Open

christianaurichzm wants to merge 1 commit into
nodejs:mainfrom
christianaurichzm:deflake-periodicflush

Conversation

@christianaurichzm

Copy link
Copy Markdown
Member

test-fastutf8stream-periodicflush failed twice in the 2026-11-09 reliability report with EBADF on write.

The manual flush case calls flush() and then destroy() right away. destroy() closes the fd without waiting for the write that flush() started, so on a busy machine the close can happen first and the write fails. The file was also read on a 500 ms timer instead of after the flush.

This moves the destroy() and the read into the flush callback.

It doesn't fail for me locally, but delaying every fs.write() by 50 ms with the preload below gives the same stack trace as CI. With it the test fails 196/200 on main and 0/200 with this change.

const fs = require('fs');
const orig = fs.write;
fs.write = function(...args) { setTimeout(() => orig.apply(fs, args), 50); };

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-11-09.md

The manual flush case called destroy() right after flush(). destroy()
closes the fd without waiting for the write that flush() started, so
under load the close can win and the write fails with EBADF. The file
was also read on a 500 ms timer instead of after the flush.

Destroy the stream and read the file from the flush callback.

Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Nov 9, 2026
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.69%. Comparing base (768315c) to head (36322b3).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #67355      +/-   ##
==========================================
- Coverage   90.69%   90.69%   -0.01%     
==========================================
  Files         797      797              
  Lines      279572   279572              
  Branches    53778    53788      +10     
==========================================
- Hits       253569   253565       -4     
- Misses      16287    16297      +10     
+ Partials     9716     9710       -6     

see 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@christianaurichzm christianaurichzm added the flaky-test Issues and PRs involving tests that fail intermittently in CI. label Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flaky-test Issues and PRs involving tests that fail intermittently in CI. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants