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

[v11.x] test: fix test by removing node-inspect/lib/_inspect #26619

Closed

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Mar 12, 2019

If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.

Fixes: #26480

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. v11.x labels Mar 12, 2019
@richardlau
Copy link
Member

Fixes: #26480

FWIW I dug a little and the behaviour regressed in 11.10.0 by commit b280d90 (prior to that commit only one deprecation warning was emitted).

@BridgeAR
Copy link
Member Author

I would like to fast track this. @ collaborators please +1 if you are fine with that.

@BridgeAR
Copy link
Member Author

This PR is specifically for v11 and no backport @nodejs/releasers

@BridgeAR
Copy link
Member Author

Landed in b581bb0

@targos said it's fine to land it on staging no matter the waiting time (otherwise the CI would be red again).

@BridgeAR BridgeAR closed this Mar 13, 2019
BridgeAR added a commit that referenced this pull request Mar 14, 2019
If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.

PR-URL: #26619
Fixes: #26480
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 14, 2019
If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.

PR-URL: nodejs#26619
Fixes: nodejs#26480
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit that referenced this pull request Mar 14, 2019
If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.

PR-URL: #26619
Fixes: #26480
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs pushed a commit that referenced this pull request Apr 16, 2019
If this file is loaded with Node.js build without ssl, it will load
'node-inspect/lib/internal/inspect_client' as well. Due to that two
deprecation warnings will be emitted instead of one and the test
fails.

It should be safe to just test all other cases and to ignore this
specific file.

PR-URL: #26619
Fixes: #26480
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BethGriggs BethGriggs mentioned this pull request May 1, 2019
@BridgeAR BridgeAR deleted the fix-deprecated-require-test branch January 20, 2020 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants