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.

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

Jump to bottom

test: move test-http-keepalive-maxsockets to sequential #16777

Closed
wants to merge 1 commit into from
Closed

test: move test-http-keepalive-maxsockets to sequential #16777

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 5, 2017

test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test http

@Trott
test: move test-http-keepalive-maxsockets to sequential
3e5c824 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Nov 5, 2017
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Nov 5, 2017
@mscdex
Copy link
Contributor

mscdex commented Nov 5, 2017 •
edited
Loading

Why does it fail? It's not using a shared port? If it's the setTimeout(), perhaps we should be using the platform timeout variable there?

@Trott
Copy link
Member Author

Trott commented Nov 5, 2017

Why does it fail?

=== release test-http-keepalive-maxsockets ===                    
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 1 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:54:14)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
[00:06|% 100|+  31|-   1]: Done                                  
$ tools/test.py -j 32 --repeat 32 test/parallel/test-http-keepalive-maxsockets.js 
=== release test-http-keepalive-maxsockets ===                    
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 0 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:46:12)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
=== release test-http-keepalive-maxsockets ===                   
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 1 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:46:12)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
=== release test-http-keepalive-maxsockets ===                   
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 0 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:46:12)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
=== release test-http-keepalive-maxsockets ===                   
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 1 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:54:14)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
=== release test-http-keepalive-maxsockets ===                   
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 0 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:46:12)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
=== release test-http-keepalive-maxsockets ===                   
Path: parallel/test-http-keepalive-maxsockets
assert.js:42
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 1 === 2
    at Timeout._onTimeout (/Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js:54:14)
    at ontimeout (timers.js:478:11)
    at tryOnTimeout (timers.js:302:5)
    at Timer.listOnTimeout (timers.js:262:5)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-http-keepalive-maxsockets.js
[00:06|% 100|+  26|-   6]: Done  
$

perhaps we should be using the platform timeout variable there?

IMO anything that needs platformTimeout() should probably be in sequential anyway. (And anything in sequential should be refactored to not have race conditions if possible, of course.)

@Trott
Copy link
Member Author

Trott commented Nov 7, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/11266/

Trott added a commit to Trott/io.js that referenced this pull request Nov 7, 2017
@Trott
test: move test-http-keepalive-maxsockets to sequential
6af68d0 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: nodejs#16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member Author

Trott commented Nov 7, 2017

Landed in 6af68d0

@Trott Trott closed this Nov 7, 2017
evanlucas pushed a commit that referenced this pull request Nov 13, 2017
@Trott @evanlucas
test: move test-http-keepalive-maxsockets to sequential
be266bd 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: #16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@evanlucas evanlucas mentioned this pull request Nov 13, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
@Trott @MylesBorins
test: move test-http-keepalive-maxsockets to sequential
4fcb03c 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: #16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 17, 2017
@Trott @MylesBorins
test: move test-http-keepalive-maxsockets to sequential
b458cb1 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: #16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@gibfahn gibfahn mentioned this pull request Nov 21, 2017
Merged
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@Trott @MylesBorins
test: move test-http-keepalive-maxsockets to sequential
71ac537 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: #16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@Trott @MylesBorins
test: move test-http-keepalive-maxsockets to sequential
cedf8a1 
test-http-keepalive-maxsockets.js will fail if sufficient copies are run
at once. Move to sequential.

PR-URL: #16777
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@apapirovski apapirovski mentioned this pull request Sep 14, 2018
Closed
3 tasks
@Trott Trott deleted the 4 branch November 26, 2021 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@hiroppy hiroppy hiroppy approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

@apapirovski apapirovski apapirovski approved these changes

Assignees
No one assigned
Labels
http Issues or PRs related to the http subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@Trott @mscdex @apapirovski @refack @jasnell @cjihrig @hiroppy @gireeshpunathil @MylesBorins @nodejs-github-bot