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 tests to work with restify@9 #3031

Merged
merged 3 commits into from
Nov 17, 2022
Merged

test: fix tests to work with restify@9 #3031

merged 3 commits into from
Nov 17, 2022

Conversation

trentm
Copy link
Member

@trentm trentm commented Nov 17, 2022

restify@9 was recently released. It only works with node >=v14.18.0
because it added use of https://nodejs.org/api/modules.html#core-modules
This updates restify testing to skip out when there is an incompat
(restify, node) version pair.

Note: It looks like the intent may have been for restify@9 to support
node >=10.0.0, so this may be refined later.


In this TAV CI test run the restify tests broke for node v8, v10, v12. Fine for node v14.
https://apm-ci.elastic.co/blue/organizations/jenkins/apm-agent-nodejs%2Fapm-agent-nodejs-mbp/detail/main/496/pipeline

[2022-11-16T06:55:02.428Z] node_tests_1  | -- installing ["[email protected]"]
[2022-11-16T06:55:10.253Z] node_tests_1  | -- running test "node test/instrumentation/modules/restify/basic.test.js" with restify
[2022-11-16T06:55:10.655Z] node_tests_1  | internal/modules/cjs/loader.js:638
[2022-11-16T06:55:10.655Z] node_tests_1  |     throw err;
[2022-11-16T06:55:10.655Z] node_tests_1  |     ^
[2022-11-16T06:55:10.655Z] node_tests_1  |
[2022-11-16T06:55:10.655Z] node_tests_1  | Error: Cannot find module 'node:process'
[2022-11-16T06:55:10.655Z] node_tests_1  |     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)

The breakage is in this restify file:

lib/request.js
5:const { emitWarning } = require('node:process');

The "node:" prefix is called "Core modules" (https://nodejs.org/api/modules.html#core-modules). This was added in v16.0.0, v14.18.0.

Restify@9's package.json says "engines": { "node": ">=10.0.0" } so the intent might have been to support node v10. I'll open an issue on restify asking about that.

restify@9 was recently released. It only works with node >=v14.18.0
because it added use of https://nodejs.org/api/modules.html#core-modules
This updates restify testing to skip out when there is an incompat
(restify, node) version pair.

Note: It looks like the intent may have been for restify@9 to support
node >=10.0.0, so this may be refined later.
@trentm trentm self-assigned this Nov 17, 2022
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Nov 17, 2022
@elastic-apm-tech elastic-apm-tech added this to In Progress in APM-Agents (OLD) Nov 17, 2022
@apmmachine
Copy link
Collaborator

apmmachine commented Nov 17, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-11-17T22:36:27.997+0000

  • Duration: 37 min 49 sec

Test stats 🧪

Test Results
Failed 0
Passed 318848
Skipped 0
Total 318848

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@trentm
Copy link
Member Author

trentm commented Nov 17, 2022

I've opened restify/node-restify#1932 about the "engines" mismatch.

@trentm
Copy link
Member Author

trentm commented Nov 17, 2022

run module tests for restify

@trentm trentm merged commit 4d52d72 into main Nov 17, 2022
APM-Agents (OLD) automation moved this from In Progress to Done Nov 17, 2022
@trentm trentm deleted the trentm/restify-9 branch November 17, 2022 23:17
trentm added a commit that referenced this pull request Nov 18, 2022
This is a follow-on to #3031 to handle testing of Node.js nightly
builds. These have a prerelease version, e.g.
"v20.0.0-nightly202211186f9175deaa", and therefore need the
includePrelease semver option for gte comparison.

Refs: #3031
trentm added a commit that referenced this pull request Nov 18, 2022
#3033)

This is a follow-on to #3031 to handle testing of Node.js nightly
builds. These have a prerelease version, e.g.
"v20.0.0-nightly202211186f9175deaa", and therefore need the
includePrelease semver option for gte comparison.

Refs: #3031
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants