-
Notifications
You must be signed in to change notification settings - Fork 225
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
tests: fix TAV test breakage with node 17 (hapi <=16, fastify <0.36.0) #2387
Conversation
This was broken in #2380. Versions of hapi <=16 did not support the 'host' option to `new Server`, but instead to `server.connection`.
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
^^^ I think TAV tests should have passed there in https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/PR-2387/2/pipeline/
|
That fastify install error with node 17 reproduced, and can be reproduced locally:
Installing [email protected] works:
But installing the next earlier version fails:
And after that, things are screwed up with this npm install:
and it takes manual intervention to get it working again:
proposalLet's drop support for fastify versions from 2017 at least:
The minimum change here would be to not test fastify versions before 0.36.0 with node 17. However, that seems overly cautious: adding complexity to .tav.yml for unnecessary use cases. No one should be running these old fastify versions. Options:
|
There is also a failure attempting to test/use some older fastify 1.x versions with node 17:
However, fastify doesn't support 1.x with node 17: https://www.fastify.io/docs/latest/LTS/
|
…stify.io/docs/latest/LTS/ says are supported for that major version
https://apm-ci.elastic.co/job/apm-agent-nodejs/job/apm-agent-nodejs-mbp/job/PR-2387/ build 5 is a TAV test run. |
This should get a changelog entry as well, but I'm waiting for the ES client v8 PR to go in first to avoid a merge conflict. |
This was broken in #2380. Versions of hapi <=16 did not support the
'host' option to
new Server
, but instead toserver.connection
.