test: coverage for internal util.emitExperimentalWarning #17635
Conversation
|
|
||
| // This test ensures that the emitExperimentalWarning in internal/util emits a | ||
| // warning when passed a unsupported feature and that it simply returns when | ||
| // passed a supported feature. |
There was a problem hiding this comment.
The description isn't correct. The second part "and that it simply returns when passed a supported feature." should say "and that it simply returns when passed the same feature multiple times."
Can this also be moved after the require below.
There was a problem hiding this comment.
@apapirovski : Thanks for the feedback. I've updated the PR. Kindly review now.
| assert(/is an experimental feature/.test(warning.message)); | ||
| }, 2)); | ||
| emitExperimentalWarning('feature1'); | ||
| emitExperimentalWarning('feature1'); |
There was a problem hiding this comment.
Could you add, at the end of the line, // should not warn
There was a problem hiding this comment.
@apapirovski : Thanks for the feedback. I've updated the PR. Kindly review now.
|
|
||
| process.on('warning', common.mustCall((warning) => { | ||
| assert(/is an experimental feature/.test(warning.message)); | ||
| }, 2)); |
There was a problem hiding this comment.
Can you please add an empty line in between the process.on('warning') and emitExperimentalWarning calls
There was a problem hiding this comment.
@maclover7 : Thanks for the feedback. I've updated the PR. Kindly review now.
|
|
||
| emitExperimentalWarning('feature1'); | ||
| emitExperimentalWarning('feature1'); | ||
| emitExperimentalWarning('feature2'); // should not warn |
There was a problem hiding this comment.
Seems like the comment is on the wrong line / the numbers got mixed up?
There was a problem hiding this comment.
@BridgeAR : You're right.. my bad. I've fixed it now. Kindly review. Thanks !
| const { emitExperimentalWarning } = require('internal/util'); | ||
|
|
||
| // This test ensures that the emitExperimentalWarning in internal/util emits a | ||
| // warning when passed a unsupported feature and that it simply returns |
There was a problem hiding this comment.
@lpinca : Good catch. I've updated it. Thanks !
|
Landed in 3a53f7c. |
PR-URL: #17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #17635 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Added missing coverage for method
emitExperimentalWarningininternal/utils.js.Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test