Skip to content

test: deflake issue-5137 stream count assertion #5243

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.

Sign up for GitHub

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

Open
trivikr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: deflake issue-5137 stream count assertion #5243

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions test/issue-5137.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,16 @@ test('RetryAgent rejects after exhausting retries on HTTP/2 stream timeout', asy
const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))

let streamCount = 0
let resolveStreamCount
const streamCountReached = new Promise(resolve => {
resolveStreamCount = resolve
})

server.on('stream', (stream) => {
streamCount++
if (streamCount === 3) {
resolveStreamCount()
}
// Never respond — simulates a perpetual stream timeout
})

Expand Down Expand Up @@ -57,6 +65,15 @@ test('RetryAgent rejects after exhausting retries on HTTP/2 stream timeout', asy
message: /stream timeout/
})

let streamCountTimeout
await Promise.race([
streamCountReached,
new Promise(resolve => {
streamCountTimeout = setTimeout(resolve, 1000)
})
])
clearTimeout(streamCountTimeout)

Copy link
Copy Markdown
Member

@mcollina mcollina May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, we should assert that there are at least 3 chunks, not exactly 3.

Copy link
Copy Markdown
Member Author

@trivikr trivikr May 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 32?

    streamCount++
    if (streamCount === 3) { // <-- This one
      resolveStreamCount()
    }

Since it checks immediately after incrementing streamCount, the === should be okay.

// Verify that all 3 retries actually reached the server
t.equal(streamCount, 3, 'server should have received all 3 request attempts')

Expand Down
Loading