-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Node 8 fixes #3507
Node 8 fixes #3507
Conversation
Node core has a testing utility named Canary in the Gold Mine (CITGM) that is used to detect npm ecosystem breakage. It runs the regression tests of many popular modules, but hapi is no longer one of them. @hueniverse, you might want to work with the CITGM team to get hapi tested again. Sorry to hijack this PR. |
@kanongil For my own curiosity can you point me in the direction of the tests that are expecting the If we have a part causing these tests to fail with Node 8 instead of just removing the |
@DavidTPate Don't worry about that part. The tests exercises code that drains streams with no destroy method. I fixed it by removing the destroy method from the test stream. I could also have done it by using an old streams2 Readable. |
it looks like hapi has the same errors on node 6 https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-nobuild/150/#showFailuresLink |
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions. |
Node 8 changes the
stream.Readable
implementation a bit. Some of the tests expects the.destroy()
method to be absent, which I fixed by always clearing it.The more serious issue, is
server.inject()
, which can return incorrect headers. This is not a test issue, but rather a bug, which is fixed by updating to the latestshot
release.Closes #3510