FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: refactor `common.expectWarning()` · nodejs/node@201a8d9 · GitHub

/ node Public

Commit 201a8d9

Browse files
BridgeAR
authored and
addaleax
committed
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>
1 parent f0202a7 commit 201a8d9

17 files changed

Lines changed: 119 additions & 108 deletions

‎test/common/README.md‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,51 @@ Indicates if there is more than 1gb of total memory.
109109
returned function has not been called exactly `exact` number of times when the
110110
test is complete, then the test will fail.
111111

112-
### expectWarning(name, expected, code)
113-
* `name` [&lt;string>]
114-
* `expected` [&lt;string>] | [&lt;Array>]
112+
### expectWarning(name[, expected[, code]])
113+
* `name` [&lt;string>] | [&lt;Object>]
114+
* `expected` [&lt;string>] | [&lt;Array>] | [&lt;Object>]
115115
* `code` [&lt;string>]
116116

117-
Tests whether `name`, `expected`, and `code` are part of a raised warning. If
118-
an expected warning does not have a code then `common.noWarnCode` can be used
119-
to indicate this.
117+
Tests whether `name`, `expected`, and `code` are part of a raised warning.
118+
119+
The code is required in case the name is set to `'DeprecationWarning'`.
120+
121+
Examples:
122+
123+
```js
124+
const { expectWarning } = require('../common');
125+
126+
expectWarning('Warning', 'Foobar is really bad');
127+
128+
expectWarning('DeprecationWarning', 'Foobar is deprecated', 'DEP0XXX');
129+
130+
expectWarning('DeprecationWarning', [
131+
'Foobar is deprecated', 'DEP0XXX'
132+
]);
133+
134+
expectWarning('DeprecationWarning', [
135+
['Foobar is deprecated', 'DEP0XXX'],
136+
['Baz is also deprecated', 'DEP0XX2']
137+
]);
138+
139+
expectWarning('DeprecationWarning', {
140+
DEP0XXX: 'Foobar is deprecated',
141+
DEP0XX2: 'Baz is also deprecated'
142+
});
143+
144+
expectWarning({
145+
DeprecationWarning: {
146+
DEP0XXX: 'Foobar is deprecated',
147+
DEP0XX1: 'Baz is also deprecated'
148+
},
149+
Warning: [
150+
['Multiple array entries are fine', 'SpecialWarningCode'],
151+
['No code is also fine']
152+
],
153+
SingleEntry: ['This will also work', 'WarningCode'],
154+
SingleString: 'Single string entries without code will also work'
155+
});
156+
```
120157

121158
### getArrayBufferViews(buf)
122159
* `buf` [&lt;Buffer>]
@@ -262,9 +299,6 @@ Returns `true` if the exit code `exitCode` and/or signal name `signal` represent
262299
the exit code and/or signal name of a node process that aborted, `false`
263300
otherwise.
264301

265-
### noWarnCode
266-
See `common.expectWarning()` for usage.
267-
268302
### opensslCli
269303
* [&lt;boolean>]
270304

‎test/common/index.js‎

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -508,54 +508,43 @@ function isAlive(pid) {
508508
}
509509
}
510510

511-
function _expectWarning(name, expected) {
512-
const map = new Map(expected);
511+
function _expectWarning(name, expected, code) {
512+
if (typeof expected === 'string') {
513+
expected = [[expected, code]];
514+
} else if (!Array.isArray(expected)) {
515+
expected = Object.entries(expected).map(([a, b]) => [b, a]);
516+
} else if (!(Array.isArray(expected[0]))) {
517+
expected = [[expected[0], expected[1]]];
518+
}
519+
// Deprecation codes are mandatory, everything else is not.
520+
if (name === 'DeprecationWarning') {
521+
expected.forEach(([_, code]) => assert(code, expected));
522+
}
513523
return mustCall((warning) => {
524+
const [ message, code ] = expected.shift();
514525
assert.strictEqual(warning.name, name);
515-
assert.ok(map.has(warning.message),
516-
`unexpected error message: "${warning.message}"`);
517-
const code = map.get(warning.message);
526+
assert.strictEqual(warning.message, message);
518527
assert.strictEqual(warning.code, code);
519-
// Remove a warning message after it is seen so that we guarantee that we
520-
// get each message only once.
521-
map.delete(expected);
522528
}, expected.length);
523529
}
524530

525-
function expectWarningByName(name, expected, code) {
526-
if (typeof expected === 'string') {
527-
expected = [[expected, code]];
528-
}
529-
process.on('warning', _expectWarning(name, expected));
530-
}
531+
let catchWarning;
531532

532-
function expectWarningByMap(warningMap) {
533-
const catchWarning = {};
534-
Object.keys(warningMap).forEach((name) => {
535-
let expected = warningMap[name];
536-
if (!Array.isArray(expected)) {
537-
throw new Error('warningMap entries must be arrays consisting of two ' +
538-
'entries: [message, warningCode]');
539-
}
540-
if (!(Array.isArray(expected[0]))) {
541-
if (expected.length === 0) {
542-
return;
543-
}
544-
expected = [[expected[0], expected[1]]];
545-
}
546-
catchWarning[name] = _expectWarning(name, expected);
547-
});
548-
process.on('warning', (warning) => catchWarning[warning.name](warning));
549-
}
550-
551-
// Accepts a warning name and description or array of descriptions or a map
552-
// of warning names to description(s)
553-
// ensures a warning is generated for each name/description pair
533+
// Accepts a warning name and description or array of descriptions or a map of
534+
// warning names to description(s) ensures a warning is generated for each
535+
// name/description pair.
536+
// The expected messages have to be unique per `expectWarning()` call.
554537
function expectWarning(nameOrMap, expected, code) {
538+
if (catchWarning === undefined) {
539+
catchWarning = {};
540+
process.on('warning', (warning) => catchWarning[warning.name](warning));
541+
}
555542
if (typeof nameOrMap === 'string') {
556-
expectWarningByName(nameOrMap, expected, code);
543+
catchWarning[nameOrMap] = _expectWarning(nameOrMap, expected, code);
557544
} else {
558-
expectWarningByMap(nameOrMap);
545+
Object.keys(nameOrMap).forEach((name) => {
546+
catchWarning[name] = _expectWarning(name, nameOrMap[name]);
547+
});
559548
}
560549
}
561550

@@ -769,7 +758,6 @@ module.exports = {
769758
mustCallAtLeast,
770759
mustNotCall,
771760
nodeProcessAborted,
772-
noWarnCode: undefined,
773761
PIPE,
774762
platformTimeout,
775763
printSkipMessage,

‎test/common/index.mjs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ const {
3737
nodeProcessAborted,
3838
busyLoop,
3939
isAlive,
40-
noWarnCode,
4140
expectWarning,
4241
expectsError,
4342
skipIfInspectorDisabled,
@@ -84,7 +83,6 @@ export {
8483
nodeProcessAborted,
8584
busyLoop,
8685
isAlive,
87-
noWarnCode,
8886
expectWarning,
8987
expectsError,
9088
skipIfInspectorDisabled,

‎test/parallel/test-atomics-notify.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
const { expectWarning, noWarnCode } = require('../common');
3+
const { expectWarning } = require('../common');
44

55
const assert = require('assert');
66
const { runInNewContext } = require('vm');
@@ -14,6 +14,6 @@ assert.strictEqual(runInNewContext('typeof Atomics.notify'), 'function');
1414
expectWarning(
1515
'Atomics',
1616
'Atomics.wake will be removed in a future version, ' +
17-
'use Atomics.notify instead.', noWarnCode);
17+
'use Atomics.notify instead.');
1818

1919
Atomics.wake(new Int32Array(new SharedArrayBuffer(4)), 0, 0);

‎test/parallel/test-console.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,14 @@ if (common.isMainThread) {
4242
common.expectWarning(
4343
'Warning',
4444
[
45-
['Count for \'noLabel\' does not exist', common.noWarnCode],
46-
['No such label \'noLabel\' for console.timeLog()', common.noWarnCode],
47-
['No such label \'noLabel\' for console.timeEnd()', common.noWarnCode],
48-
['Count for \'default\' does not exist', common.noWarnCode],
49-
['No such label \'default\' for console.timeLog()', common.noWarnCode],
50-
['No such label \'default\' for console.timeEnd()', common.noWarnCode],
51-
['Label \'default\' already exists for console.time()', common.noWarnCode],
52-
['Label \'test\' already exists for console.time()', common.noWarnCode]
45+
['Count for \'noLabel\' does not exist'],
46+
['No such label \'noLabel\' for console.timeLog()'],
47+
['No such label \'noLabel\' for console.timeEnd()'],
48+
['Count for \'default\' does not exist'],
49+
['No such label \'default\' for console.timeLog()'],
50+
['No such label \'default\' for console.timeEnd()'],
51+
['Label \'default\' already exists for console.time()'],
52+
['Label \'test\' already exists for console.time()']
5353
]
5454
);
5555

‎test/parallel/test-crypto-authenticated.js‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -50,25 +50,25 @@ const ciphers = crypto.getCiphers();
5050

5151
const expectedWarnings = common.hasFipsCrypto ?
5252
[] : [
53-
['Use Cipheriv for counter mode of aes-192-gcm', common.noWarnCode],
54-
['Use Cipheriv for counter mode of aes-192-ccm', common.noWarnCode],
55-
['Use Cipheriv for counter mode of aes-192-ccm', common.noWarnCode],
56-
['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
57-
['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
58-
['Use Cipheriv for counter mode of aes-128-ccm', common.noWarnCode],
59-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
60-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
61-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
62-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
63-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
64-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
65-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
66-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
67-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
68-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
69-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
70-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode],
71-
['Use Cipheriv for counter mode of aes-256-ccm', common.noWarnCode]
53+
['Use Cipheriv for counter mode of aes-192-gcm'],
54+
['Use Cipheriv for counter mode of aes-192-ccm'],
55+
['Use Cipheriv for counter mode of aes-192-ccm'],
56+
['Use Cipheriv for counter mode of aes-128-ccm'],
57+
['Use Cipheriv for counter mode of aes-128-ccm'],
58+
['Use Cipheriv for counter mode of aes-128-ccm'],
59+
['Use Cipheriv for counter mode of aes-256-ccm'],
60+
['Use Cipheriv for counter mode of aes-256-ccm'],
61+
['Use Cipheriv for counter mode of aes-256-ccm'],
62+
['Use Cipheriv for counter mode of aes-256-ccm'],
63+
['Use Cipheriv for counter mode of aes-256-ccm'],
64+
['Use Cipheriv for counter mode of aes-256-ccm'],
65+
['Use Cipheriv for counter mode of aes-256-ccm'],
66+
['Use Cipheriv for counter mode of aes-256-ccm'],
67+
['Use Cipheriv for counter mode of aes-256-ccm'],
68+
['Use Cipheriv for counter mode of aes-256-ccm'],
69+
['Use Cipheriv for counter mode of aes-256-ccm'],
70+
['Use Cipheriv for counter mode of aes-256-ccm'],
71+
['Use Cipheriv for counter mode of aes-256-ccm']
7272
];
7373

7474
const expectedDeprecationWarnings = [

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ const assert = require('assert');
1212

1313
common.expectWarning({
1414
Warning: [
15-
['Use Cipheriv for counter mode of aes-256-gcm', common.noWarnCode]
15+
['Use Cipheriv for counter mode of aes-256-gcm']
1616
],
1717
DeprecationWarning: [
1818
['crypto.createCipher is deprecated.', 'DEP0106']

‎test/parallel/test-dns-lookup.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,19 @@ cares.getaddrinfo = () => internalBinding('uv').UV_ENOENT;
2121
common.expectsError(() => dnsPromises.lookup(1, {}), err);
2222
}
2323

24+
// This also verifies different expectWarning notations.
2425
common.expectWarning({
2526
// For 'internal/test/binding' module.
2627
'internal/test/binding': [
2728
'These APIs are for internal testing only. Do not use them.'
2829
],
2930
// For dns.promises.
30-
'ExperimentalWarning': [
31-
'The dns.promises API is experimental'
32-
],
31+
'ExperimentalWarning': 'The dns.promises API is experimental',
3332
// For calling `dns.lookup` with falsy `hostname`.
34-
'DeprecationWarning': [
35-
'The provided hostname "false" is not a valid ' +
36-
'hostname, and is supported in the dns module solely for compatibility.',
37-
'DEP0118',
38-
],
33+
'DeprecationWarning': {
34+
DEP0118: 'The provided hostname "false" is not a valid ' +
35+
'hostname, and is supported in the dns module solely for compatibility.'
36+
}
3937
});
4038

4139
common.expectsError(() => {

‎test/parallel/test-fs-filehandle.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,10 @@ let fdnum;
2121

2222
common.expectWarning({
2323
'internal/test/binding': [
24-
'These APIs are for internal testing only. Do not use them.',
25-
common.noWarnCode
24+
'These APIs are for internal testing only. Do not use them.'
2625
],
2726
'Warning': [
28-
`Closing file descriptor ${fdnum} on garbage collection`,
29-
common.noWarnCode
27+
`Closing file descriptor ${fdnum} on garbage collection`
3028
]
3129
});
3230

‎test/parallel/test-https-strict.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@ common.expectWarning(
3232
'Warning',
3333
'Setting the NODE_TLS_REJECT_UNAUTHORIZED environment variable to \'0\' ' +
3434
'makes TLS connections and HTTPS requests insecure by disabling ' +
35-
'certificate verification.',
36-
common.noWarnCode
35+
'certificate verification.'
3736
);
3837

3938
const assert = require('assert');

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL