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

Run tests with --throw-deprecations #2844

Closed
ofrobots opened this issue Jul 3, 2018 · 5 comments
Closed

Run tests with --throw-deprecations #2844

ofrobots opened this issue Jul 3, 2018 · 5 comments
Assignees
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@ofrobots
Copy link
Contributor

ofrobots commented Jul 3, 2018

Similar to #2839, it would be good for Google Cloud libraries to not use deprecated Node.js APIs. Deprecation messages printed to the console, however, can be easy to miss.

We should run tests with --throw-deprecations to ensure that use of deprecated APIs is caught by tests.

The same deprecation mechanism would work for our own deprecations, e.g. googleapis/google-auth-library-nodejs#402.

@ofrobots ofrobots added the type: cleanup An internal cleanup or hygiene concern. label Jul 3, 2018
@JustinBeckwith
Copy link
Contributor

Should we run all of our tests with this turned on, or just the system-tests and samples tests? I ask because we have unit tests that verify the warning behavior for deprecated APIs :)

@ofrobots
Copy link
Contributor Author

ofrobots commented Jul 3, 2018

We have unit tests that verify the warning behavior of all deprecations, including Node.js? Can you point me to how this is done?

@JustinBeckwith
Copy link
Contributor

I was referring to this:
https://github.com/google/google-auth-library-nodejs/pull/402/files#diff-86629eb1fa03b5146081c8eed046735bR1407

When you run that unit test, it will emit the deprecation warning to the console.

@ofrobots
Copy link
Contributor Author

ofrobots commented Jul 3, 2018

Oh gotcha! In that test could you install a mock on process.emitWarning instead of installing a listener and expecting it to be called?

@JustinBeckwith
Copy link
Contributor

All of the PRs for this have been submitted. Just waiting on fix in gax-nodejs to land the stragglers :)

@JustinBeckwith JustinBeckwith self-assigned this Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

2 participants