Skip to content
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

test: deflake http2-pipe-named-pipe #40842

Closed

Conversation

lpinca
Copy link
Member

@lpinca lpinca commented Nov 17, 2021

Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

`http2-url-tests.js` is misleading. Use the same name of the source file
instead.
Wait for all data to be read before sending the response and closing
the client.

Fixes: nodejs#40277
@lpinca lpinca force-pushed the deflake/test-http2-pipe-named-pipe branch from 31cb70c to f704d99 Compare November 17, 2021 19:25
@lpinca
Copy link
Member Author

lpinca commented Nov 17, 2021

Before:

$ ./tools/test.py test/parallel/test-http2-pipe-named-pipe.js 
[00:00|% 100|+   1|-   0]: Done                              
luigi@imac:node (master)$ ./tools/test.py -J --repeat=1000 test/parallel/test-http2-pipe-named-pipe.js 
=== release test-http2-pipe-named-pipe ===                   
Path: parallel/test-http2-pipe-named-pipe
node:assert:123
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

139837 !== 139244

    at WriteStream.<anonymous> (/Users/luigi/code/node/test/parallel/test-http2-pipe-named-pipe.js:26:12)
    at WriteStream.emit (node:events:402:35)
    at finish (node:internal/streams/writable:755:10)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 139837,
  expected: 139244,
  operator: 'strictEqual'
}

Node.js v18.0.0-pre
Command: out/Release/node /Users/luigi/code/node/test/parallel/test-http2-pipe-named-pipe.js

...

=== release test-http2-pipe-named-pipe ===                   
Path: parallel/test-http2-pipe-named-pipe
node:assert:123
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

139837 !== 139244

    at WriteStream.<anonymous> (/Users/luigi/code/node/test/parallel/test-http2-pipe-named-pipe.js:26:12)
    at WriteStream.emit (node:events:402:35)
    at finish (node:internal/streams/writable:755:10)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 139837,
  expected: 139244,
  operator: 'strictEqual'
}

Node.js v18.0.0-pre
Command: out/Release/node /Users/luigi/code/node/test/parallel/test-http2-pipe-named-pipe.js
[00:12|% 100|+ 995|-   5]: Done 

After:

$ ./tools/test.py -J --repeat=1000 test/parallel/test-http2-pipe-named-pipe.js 
[00:12|% 100|+ 1000|-   0]: Done

The first argument is the actual value, the second argument is the
expected value.
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Nov 17, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 21, 2021
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 21, 2021
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/40842
✔  Done loading data for nodejs/node/pull/40842
----------------------------------- PR info ------------------------------------
Title      test: deflake http2-pipe-named-pipe (#40842)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     lpinca:deflake/test-http2-pipe-named-pipe -> nodejs:master
Labels     test, needs-ci
Commits    3
 - test: use descriptive name for destination file
 - test: deflake http2-pipe-named-pipe
 - test: fix argument order in assertion
Committers 1
 - Luigi Pinca 
PR-URL: https://github.com/nodejs/node/pull/40842
Fixes: https://github.com/nodejs/node/issues/40277
Reviewed-By: Robert Nagy 
Reviewed-By: Adrian Estrada 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40842
Fixes: https://github.com/nodejs/node/issues/40277
Reviewed-By: Robert Nagy 
Reviewed-By: Adrian Estrada 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 17 Nov 2021 19:21:28 GMT
   ✔  Approvals: 2
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/40842#pullrequestreview-809927718
   ✔  - Adrian Estrada (@edsadr): https://github.com/nodejs/node/pull/40842#pullrequestreview-810388382
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2021-11-21T13:43:54Z: https://ci.nodejs.org/job/node-test-pull-request/41027/
- Querying data for job/node-test-pull-request/41027/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch                  master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 40842
From https://github.com/nodejs/node
 * branch                  refs/pull/40842/merge -> FETCH_HEAD
✔  Fetched commits as e31d1cb55da6..570123aa320d
--------------------------------------------------------------------------------
[master 4c98aaf796] test: use descriptive name for destination file
 Author: Luigi Pinca 
 Date: Wed Nov 17 20:09:37 2021 +0100
 1 file changed, 1 insertion(+), 1 deletion(-)
[master 9d5c54ea71] test: deflake http2-pipe-named-pipe
 Author: Luigi Pinca 
 Date: Wed Nov 17 20:18:10 2021 +0100
 1 file changed, 7 insertions(+), 4 deletions(-)
[master 8460bd5b2c] test: fix argument order in assertion
 Author: Luigi Pinca 
 Date: Wed Nov 17 20:31:03 2021 +0100
 1 file changed, 2 insertions(+), 2 deletions(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/6)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: use descriptive name for destination file

http2-url-tests.js is misleading. Use the same name of the source file
instead.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Adrian Estrada edsadr@gmail.com

[detached HEAD f2ca78a78e] test: use descriptive name for destination file
Author: Luigi Pinca luigipinca@gmail.com
Date: Wed Nov 17 20:09:37 2021 +0100
1 file changed, 1 insertion(+), 1 deletion(-)
Rebasing (3/6)
Rebasing (4/6)

Executing: git node land --amend --yes
⚠ Found Fixes: #40277, skipping..
--------------------------------- New Message ----------------------------------
test: deflake http2-pipe-named-pipe

Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

PR-URL: #40842
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Adrian Estrada edsadr@gmail.com

[detached HEAD a9e05fede3] test: deflake http2-pipe-named-pipe
Author: Luigi Pinca luigipinca@gmail.com
Date: Wed Nov 17 20:18:10 2021 +0100
1 file changed, 7 insertions(+), 4 deletions(-)
Rebasing (5/6)
Rebasing (6/6)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: fix argument order in assertion

The first argument is the actual value, the second argument is the
expected value.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Adrian Estrada edsadr@gmail.com

[detached HEAD 10de988bd0] test: fix argument order in assertion
Author: Luigi Pinca luigipinca@gmail.com
Date: Wed Nov 17 20:31:03 2021 +0100
1 file changed, 2 insertions(+), 2 deletions(-)

Successfully rebased and updated refs/heads/master.

ℹ Use --fixupAll option, squash the PR manually or land the PR from the command line.

https://github.com/nodejs/node/actions/runs/1487003296

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 21, 2021
@targos targos added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 21, 2021
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 21, 2021
@nodejs-github-bot
Copy link
Collaborator

Landed in e31d1cb...1dcba68

nodejs-github-bot pushed a commit that referenced this pull request Nov 21, 2021
`http2-url-tests.js` is misleading. Use the same name of the source file
instead.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 21, 2021
Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

PR-URL: #40842
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
nodejs-github-bot pushed a commit that referenced this pull request Nov 21, 2021
The first argument is the actual value, the second argument is the
expected value.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
targos pushed a commit that referenced this pull request Nov 21, 2021
`http2-url-tests.js` is misleading. Use the same name of the source file
instead.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
targos pushed a commit that referenced this pull request Nov 21, 2021
Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

PR-URL: #40842
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
targos pushed a commit that referenced this pull request Nov 21, 2021
The first argument is the actual value, the second argument is the
expected value.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
@lpinca lpinca deleted the deflake/test-http2-pipe-named-pipe branch November 21, 2021 18:34
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
`http2-url-tests.js` is misleading. Use the same name of the source file
instead.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

PR-URL: #40842
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 30, 2022
The first argument is the actual value, the second argument is the
expected value.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
`http2-url-tests.js` is misleading. Use the same name of the source file
instead.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
Wait for all data to be read before sending the response and closing
the client.

Fixes: #40277

PR-URL: #40842
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
The first argument is the actual value, the second argument is the
expected value.

PR-URL: #40842
Fixes: #40277
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Adrian Estrada <edsadr@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate flaky test test-http2-pipe-named-pipe on osx-arm
5 participants