test: refactor common.expectWarning()

The current API is somewhat confusing at times and simpler usage is possible. This overloads the arguments further to accept objects with deprecation codes as property keys. It also adds documentation for the different possible styles. Besides that it is now going to validate for the code being present in case of deprecations but not for other cases. The former validation was not consistent as it only validated some cases and accepted undefined instead of `common.noWarnCode`. This check is removed due to the lack of consistency. `common.noWarnCode` is completely removed due to just being sugar for `undefined`. This also verifies that the warning order is identical to the order in which they are triggered. PR-URL: #25251 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Author: BridgeAR

test/common/README.md

@@ -109,14 +109,51 @@ Indicates if there is more than 1gb of total memory.
  returned function has not been called exactly `exact` number of times when the
  test is complete, then the test will fail.
 
-### expectWarning(name, expected, code)
-* `name` [<string>]
-* `expected` [<string>] | [<Array>]
+### expectWarning(name[, expected[, code]])
+* `name` [<string>] | [<Object>]
+* `expected` [<string>] | [<Array>] | [<Object>]
 * `code` [<string>]
 
-Tests whether `name`, `expected`, and `code` are part of a raised warning. If
-an expected warning does not have a code then `common.noWarnCode` can be used
-to indicate this.
+Tests whether `name`, `expected`, and `code` are part of a raised warning.
+
+The code is required in case the name is set to `'DeprecationWarning'`.
+
+Examples:
+
+```js
+const { expectWarning } = require('../common');
+
+expectWarning('Warning', 'Foobar is really bad');
+
+expectWarning('DeprecationWarning', 'Foobar is deprecated', 'DEP0XXX');
+
+expectWarning('DeprecationWarning', [
+ 'Foobar is deprecated', 'DEP0XXX'
+]);
+
+expectWarning('DeprecationWarning', [
+ ['Foobar is deprecated', 'DEP0XXX'],
+ ['Baz is also deprecated', 'DEP0XX2']
+]);
+
+expectWarning('DeprecationWarning', {
+ DEP0XXX: 'Foobar is deprecated',
+ DEP0XX2: 'Baz is also deprecated'
+});
+
+expectWarning({
+ DeprecationWarning: {
+ DEP0XXX: 'Foobar is deprecated',
+ DEP0XX1: 'Baz is also deprecated'
+ },
+ Warning: [
+ ['Multiple array entries are fine', 'SpecialWarningCode'],
+ ['No code is also fine']
+ ],
+ SingleEntry: ['This will also work', 'WarningCode'],
+ SingleString: 'Single string entries without code will also work'
+});
+```
 
 ### getArrayBufferViews(buf)
 * `buf` [<Buffer>]
@@ -262,9 +299,6 @@ Returns `true` if the exit code `exitCode` and/or signal name `signal` represent
 the exit code and/or signal name of a node process that aborted, `false`
 otherwise.
 
-### noWarnCode
-See `common.expectWarning()` for usage.
-
 ### opensslCli
 * [<boolean>]
 

test/common/index.js

@@ -497,54 +497,43 @@ function isAlive(pid) {
  }
 }
 
-function _expectWarning(name, expected) {
- const map = new Map(expected);
+function _expectWarning(name, expected, code) {
+ if (typeof expected === 'string') {
+ expected = [[expected, code]];
+ } else if (!Array.isArray(expected)) {
+ expected = Object.entries(expected).map(([a, b]) => [b, a]);
+ } else if (!(Array.isArray(expected[0]))) {
+ expected = [[expected[0], expected[1]]];
+ }
+ // Deprecation codes are mandatory, everything else is not.
+ if (name === 'DeprecationWarning') {
+ expected.forEach(([_, code]) => assert(code, expected));
+ }
  return mustCall((warning) => {
+ const [ message, code ] = expected.shift();
  assert.strictEqual(warning.name, name);
- assert.ok(map.has(warning.message),
- `unexpected error message: "${warning.message}"`);
- const code = map.get(warning.message);
+ assert.strictEqual(warning.message, message);
  assert.strictEqual(warning.code, code);
- // Remove a warning message after it is seen so that we guarantee that we
- // get each message only once.
- map.delete(expected);
  }, expected.length);
 }
 
-function expectWarningByName(name, expected, code) {
- if (typeof expected === 'string') {
- expected = [[expected, code]];
- }
- process.on('warning', _expectWarning(name, expected));
-}
+let catchWarning;
 
-function expectWarningByMap(warningMap) {
- const catchWarning = {};
- Object.keys(warningMap).forEach((name) => {
- let expected = warningMap[name];
- if (!Array.isArray(expected)) {
- throw new Error('warningMap entries must be arrays consisting of two ' +
- 'entries: [message, warningCode]');
- }
- if (!(Array.isArray(expected[0]))) {
- if (expected.length === 0) {
- return;
- }
- expected = [[expected[0], expected[1]]];
- }
- catchWarning[name] = _expectWarning(name, expected);
- });
- process.on('warning', (warning) => catchWarning[warning.name](warning));
-}
-
-// Accepts a warning name and description or array of descriptions or a map
-// of warning names to description(s)
-// ensures a warning is generated for each name/description pair
+// Accepts a warning name and description or array of descriptions or a map of
+// warning names to description(s) ensures a warning is generated for each
+// name/description pair.
+// The expected messages have to be unique per `expectWarning()` call.
 function expectWarning(nameOrMap, expected, code) {
+ if (catchWarning === undefined) {
+ catchWarning = {};
+ process.on('warning', (warning) => catchWarning[warning.name](warning));
+ }
  if (typeof nameOrMap === 'string') {
- expectWarningByName(nameOrMap, expected, code);
+ catchWarning[nameOrMap] = _expectWarning(nameOrMap, expected, code);
  } else {
- expectWarningByMap(nameOrMap);
+ Object.keys(nameOrMap).forEach((name) => {
+ catchWarning[name] = _expectWarning(name, nameOrMap[name]);
+ });
  }
 }
 
@@ -758,7 +747,6 @@ module.exports = {
  mustCallAtLeast,
  mustNotCall,
  nodeProcessAborted,
- noWarnCode: undefined,
  PIPE,
  platformTimeout,
  printSkipMessage,

test/common/index.mjs

@@ -37,7 +37,6 @@ const {
  nodeProcessAborted,
  busyLoop,
  isAlive,
- noWarnCode,
  expectWarning,
  expectsError,
  skipIfInspectorDisabled,
@@ -84,7 +83,6 @@ export {
  nodeProcessAborted,
  busyLoop,
  isAlive,
- noWarnCode,
  expectWarning,
  expectsError,
  skipIfInspectorDisabled,

test/parallel/test-atomics-notify.js

@@ -1,6 +1,6 @@
 'use strict';
 
-const { expectWarning, noWarnCode } = require('../common');
+const { expectWarning } = require('../common');
 
 const assert = require('assert');
 const { runInNewContext } = require('vm');
@@ -14,6 +14,6 @@ assert.strictEqual(runInNewContext('typeof Atomics.notify'), 'function');
 expectWarning(
  'Atomics',
  'Atomics.wake will be removed in a future version, ' +
- 'use Atomics.notify instead.', noWarnCode);
+ 'use Atomics.notify instead.');
 
 Atomics.wake(new Int32Array(new SharedArrayBuffer(4)), 0, 0);

test/parallel/test-console.js

@@ -42,14 +42,14 @@ if (common.isMainThread) {
 common.expectWarning(
  'Warning',
  [
- ['Count for \'noLabel\' does not exist', common.noWarnCode],
- ['No such label \'noLabel\' for console.timeLog()', common.noWarnCode],
- ['No such label \'noLabel\' for console.timeEnd()', common.noWarnCode],
- ['Count for \'default\' does not exist', common.noWarnCode],
- ['No such label \'default\' for console.timeLog()', common.noWarnCode],
- ['No such label \'default\' for console.timeEnd()', common.noWarnCode],
- ['Label \'default\' already exists for console.time()', common.noWarnCode],
- ['Label \'test\' already exists for console.time()', common.noWarnCode]
+ ['Count for \'noLabel\' does not exist'],
+ ['No such label \'noLabel\' for console.timeLog()'],
+ ['No such label \'noLabel\' for console.timeEnd()'],
+ ['Count for \'default\' does not exist'],
+ ['No such label \'default\' for console.timeLog()'],
+ ['No such label \'default\' for console.timeEnd()'],
+ ['Label \'default\' already exists for console.time()'],
+ ['Label \'test\' already exists for console.time()']
  ]
 );
 

test/parallel/test-crypto-authenticated.js

@@ -50,25 +50,25 @@ const ciphers = crypto.getCiphers();
 
 const expectedWarnings = common.hasFipsCrypto ?
  [] : [
- ['Use Cipheriv for counter mode of aes-192-gcm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-192-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-192-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
- ['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode]
+ ['Use Cipheriv for counter mode of aes-192-gcm'],
+ ['Use Cipheriv for counter mode of aes-192-ccm'],
+ ['Use Cipheriv for counter mode of aes-192-ccm'],
+ ['Use Cipheriv for counter mode of aes-128-ccm'],
+ ['Use Cipheriv for counter mode of aes-128-ccm'],
+ ['Use Cipheriv for counter mode of aes-128-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm'],
+ ['Use Cipheriv for counter mode of aes-256-ccm']
  ];
 
 const expectedDeprecationWarnings = [

test/parallel/test-crypto-cipher-decipher.js

@@ -12,7 +12,7 @@ const assert = require('assert');
 
 common.expectWarning({
  Warning: [
- ['Use Cipheriv for counter mode of aes-256-gcm', common.noWarnCode]
+ ['Use Cipheriv for counter mode of aes-256-gcm']
  ],
  DeprecationWarning: [
  ['crypto.createCipher is deprecated.', 'DEP0106']

test/parallel/test-dns-lookup.js

@@ -21,21 +21,19 @@ cares.getaddrinfo = () => internalBinding('uv').UV_ENOENT;
  common.expectsError(() => dnsPromises.lookup(1, {}), err);
 }
 
+// This also verifies different expectWarning notations.
 common.expectWarning({
  // For 'internal/test/binding' module.
  'internal/test/binding': [
  'These APIs are for internal testing only. Do not use them.'
  ],
  // For dns.promises.
- 'ExperimentalWarning': [
- 'The dns.promises API is experimental'
- ],
+ 'ExperimentalWarning': 'The dns.promises API is experimental',
  // For calling `dns.lookup` with falsy `hostname`.
- 'DeprecationWarning': [
- 'The provided hostname "false" is not a valid ' +
- 'hostname, and is supported in the dns module solely for compatibility.',
- 'DEP0118',
- ],
+ 'DeprecationWarning': {
+ DEP0118: 'The provided hostname "false" is not a valid ' +
+ 'hostname, and is supported in the dns module solely for compatibility.'
+ }
 });
 
 common.expectsError(() => {

test/parallel/test-fs-filehandle.js

@@ -21,12 +21,10 @@ let fdnum;
 
 common.expectWarning({
  'internal/test/binding': [
- 'These APIs are for internal testing only. Do not use them.',
- common.noWarnCode
+ 'These APIs are for internal testing only. Do not use them.'
  ],
  'Warning': [
- `Closing file descriptor ${fdnum} on garbage collection`,
- common.noWarnCode
+ `Closing file descriptor ${fdnum} on garbage collection`
  ]
 });
 

test/parallel/test-https-strict.js

@@ -32,8 +32,7 @@ common.expectWarning(
  'Warning',
  'Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to \'0\' ' +
  'makes TLS connections and HTTPS requests insecure by disabling ' +
- 'certificate verification.',
- common.noWarnCode
+ 'certificate verification.'
 );
 
 const assert = require('assert');