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: Removed server.start in grpc tests as it is deprecated and no longer needed #2372

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

bizob2828
Copy link
Member

@bizob2828 bizob2828 commented Jul 17, 2024

Description

I noticed when adding support for Node 22 we had some deprecation warnings.

(node:98461) DeprecationWarning: Calling start() is no longer necessary. It can be safely omitted.
(Use `node --trace-deprecation ...` to show where the warning was created)

When adding --trace-deprecation to the entire integration run I discovered

(node:99014) DeprecationWarning: Calling start() is no longer necessary. It can be safely omitted.
    at /Users/revans/code/pull-requests/node-newrelic/test/integration/grpc/reconnect.tap.js:221:16
    at /Users/revans/code/pull-requests/node-newrelic/node_modules/@grpc/grpc-js/src/server.ts:869:11
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

It also existed in another grpc test.

How to Test

node test/integration/grpc/reconnect.tap.js

The deprecation warning is gone.

Additional Information

There is still this warning

(node:20389) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)

But it is safe because if/when Node.js removes punycode the unicode-length package will use its 3rd party dep of punycode based on this copy

⚠️ Note that userland modules don't hide core modules. For example, require('punycode') still imports the deprecated core module even if you executed npm install punycode. Use require('punycode/') to import userland modules rather than core modules.

@bizob2828 bizob2828 added the dev:tests Indicates only changes to tests label Jul 17, 2024
Copy link
Contributor

@jsumners-nr jsumners-nr left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev:tests Indicates only changes to tests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants