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

Fix abort test timing to be consistent from node v12 through v16 #4239

Merged
merged 1 commit into from
Mar 20, 2021

Conversation

devinivy
Copy link
Member

This fixes a test failure surfaced by citgm against node v16 nightly nodejs/citgm#852. In the past we have seen this test be a bit flaky on OSX— hopefully this addresses that issue as well.

The issue is that the test relies on the relative timing of various node events. Previously three requests were made to hapi, and during the first one the requests are aborted by the client. For previous versions of node, the first two of those three requests were able to make it through before the server saw the client disconnect. In node v16 the disconnect occurs nearly immediately, and only the first of the three requests were able to make it through. To fix this, I ensure that the second and third requests experience the disconnect during the pre-handler.

@devinivy devinivy added the test Test or coverage label Mar 20, 2021
Copy link
Member

@nlf nlf left a comment

Choose a reason for hiding this comment

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

👍

@devinivy
Copy link
Member Author

Thanks for the speedy review!

@devinivy devinivy added this to the 20.1.2 milestone Mar 20, 2021
@devinivy devinivy self-assigned this Mar 20, 2021
@devinivy devinivy merged commit e95683a into master Mar 20, 2021
@devinivy devinivy deleted the fix-abort-test-timing-node-v16 branch March 20, 2021 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Test or coverage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants