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: fix test-net-keepalive for AIX #3458

Closed
wants to merge 1 commit into from
Closed

test: fix test-net-keepalive for AIX #3458

wants to merge 1 commit into from

Conversation

imran-iq
Copy link
Contributor

Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.

Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.
@mhdawson
Copy link
Member

lgtm

While the failures were seen on AIX, there is no reason the same failure could not occur on other platforms with the original test so fixing for all make sense.

@mhdawson mhdawson self-assigned this Oct 20, 2015
@mhdawson mhdawson added the test Issues and PRs related to the tests. label Oct 20, 2015
@mhdawson
Copy link
Member

@mscdex mscdex added the net Issues and PRs related to the net subsystem. label Oct 20, 2015
@jasnell
Copy link
Member

jasnell commented Oct 21, 2015

LGTM

@jasnell
Copy link
Member

jasnell commented Oct 21, 2015

@mhdawson ... there were failures on ppc ... likely unrelated but can you verify before I land?

@mhdawson
Copy link
Member

Those are known failures on PPC covered by existing defects so we are good to land

jasnell pushed a commit that referenced this pull request Oct 21, 2015
Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.

Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: #3458
@jasnell
Copy link
Member

jasnell commented Oct 21, 2015

Landed in 87d42b0

@jasnell jasnell closed this Oct 21, 2015
rvagg pushed a commit that referenced this pull request Oct 22, 2015
Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.

Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: #3458
jasnell pushed a commit that referenced this pull request Oct 26, 2015
Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.

Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: #3458
@jasnell
Copy link
Member

jasnell commented Oct 26, 2015

Landed in v4.x-staging in e99823b

jasnell pushed a commit that referenced this pull request Oct 29, 2015
Fixed an intermittent issue on AIX where the 100ms timeout was reached
before the 'connection' event was fired. This resulted in a failure as
serverConnection would be undefined and the assert.equal would throw an
error. Changed the flow of the test so that the timeout is only set
after a connection has been made.

Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: #3458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants