Skip to content

Commit f0b2992

Browse files
Masashi Hiranotargos
authored andcommitted
test: fix ineffective error tests
Fix tests whether errors are thrown correctly because they are successful when error doesn't get thrown. PR-URL: #27333 Fixes: #26385 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent d84a6d0 commit f0b2992

File tree

6 files changed

+73
-72
lines changed

6 files changed

+73
-72
lines changed

test/addons/non-node-context/test-make-buffer.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@ const {
99
// Because the `Buffer` function and its protoype property only (currently)
1010
// exist in a Node.js instance’s main context, trying to create buffers from
1111
// another context throws an exception.
12+
assert.throws(
13+
() => makeBufferInNewContext(),
14+
(exception) => {
15+
assert.strictEqual(exception.constructor.name, 'Error');
16+
assert(!(exception.constructor instanceof Error));
1217

13-
try {
14-
makeBufferInNewContext();
15-
} catch (exception) {
16-
assert.strictEqual(exception.constructor.name, 'Error');
17-
assert(!(exception.constructor instanceof Error));
18-
19-
assert.strictEqual(exception.code, 'ERR_BUFFER_CONTEXT_NOT_AVAILABLE');
20-
assert.strictEqual(exception.message,
21-
'Buffer is not available for the current Context');
22-
}
18+
assert.strictEqual(exception.code, 'ERR_BUFFER_CONTEXT_NOT_AVAILABLE');
19+
assert.strictEqual(exception.message,
20+
'Buffer is not available for the current Context');
21+
return true;
22+
}
23+
);

test/es-module/test-esm-error-cache.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@ let error;
1818

1919
assert(error);
2020

21-
try {
22-
await import(file);
23-
} catch (e) {
24-
assert.strictEqual(error, e);
25-
}
21+
await assert.rejects(
22+
() => import(file),
23+
(e) => {
24+
assert.strictEqual(error, e);
25+
return true;
26+
}
27+
);
2628
})();

test/parallel/test-assert.js

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -297,23 +297,21 @@ testAssertionMessage({ a: NaN, b: Infinity, c: -Infinity },
297297
'{\n+ a: NaN,\n+ b: Infinity,\n+ c: -Infinity\n+ }');
298298

299299
// https://github.com/nodejs/node-v0.x-archive/issues/5292
300-
try {
301-
assert.strictEqual(1, 2);
302-
} catch (e) {
303-
assert.strictEqual(
304-
e.message,
305-
`${strictEqualMessageStart}\n1 !== 2\n`
306-
);
307-
assert.ok(e.generatedMessage, 'Message not marked as generated');
308-
}
300+
assert.throws(
301+
() => assert.strictEqual(1, 2),
302+
{
303+
message: `${strictEqualMessageStart}\n1 !== 2\n`,
304+
generatedMessage: true
305+
}
306+
);
309307

310-
try {
311-
assert.strictEqual(1, 2, 'oh no');
312-
} catch (e) {
313-
assert.strictEqual(e.message, 'oh no');
314-
// Message should not be marked as generated.
315-
assert.strictEqual(e.generatedMessage, false);
316-
}
308+
assert.throws(
309+
() => assert.strictEqual(1, 2, 'oh no'),
310+
{
311+
message: 'oh no',
312+
generatedMessage: false
313+
}
314+
);
317315

318316
{
319317
let threw = false;

test/parallel/test-util-inspect.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -538,11 +538,14 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
538538
].forEach((err) => {
539539
assert.strictEqual(util.inspect(err), err.stack);
540540
});
541-
try {
542-
undef(); // eslint-disable-line no-undef
543-
} catch (e) {
544-
assert.strictEqual(util.inspect(e), e.stack);
545-
}
541+
assert.throws(
542+
() => undef(), // eslint-disable-line no-undef
543+
(e) => {
544+
assert.strictEqual(util.inspect(e), e.stack);
545+
return true;
546+
}
547+
);
548+
546549
const ex = util.inspect(new Error('FAIL'), true);
547550
assert(ex.includes('Error: FAIL'));
548551
assert(ex.includes('[stack]'));

test/parallel/test-vm-codegen.js

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,6 @@ const { createContext, runInContext, runInNewContext } = require('vm');
88
const WASM_BYTES = Buffer.from(
99
[0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]);
1010

11-
12-
function expectsError(fn, type) {
13-
try {
14-
fn();
15-
assert.fail('expected fn to error');
16-
} catch (err) {
17-
if (typeof type === 'string')
18-
assert.strictEqual(err.name, type);
19-
else
20-
assert(err instanceof type);
21-
}
22-
}
23-
2411
{
2512
const ctx = createContext({ WASM_BYTES });
2613
const test = 'eval(""); new WebAssembly.Module(WASM_BYTES);';
@@ -39,7 +26,7 @@ function expectsError(fn, type) {
3926
});
4027

4128
const EvalError = runInContext('EvalError', ctx);
42-
expectsError(() => {
29+
assert.throws(() => {
4330
runInContext('eval("x")', ctx);
4431
}, EvalError);
4532
}
@@ -52,26 +39,30 @@ function expectsError(fn, type) {
5239
});
5340

5441
const CompileError = runInContext('WebAssembly.CompileError', ctx);
55-
expectsError(() => {
42+
assert.throws(() => {
5643
runInContext('new WebAssembly.Module(WASM_BYTES)', ctx);
5744
}, CompileError);
5845
}
5946

60-
expectsError(() => {
47+
assert.throws(() => {
6148
runInNewContext('eval("x")', {}, {
6249
contextCodeGeneration: {
6350
strings: false,
6451
},
6552
});
66-
}, 'EvalError');
53+
}, {
54+
name: 'EvalError'
55+
});
6756

68-
expectsError(() => {
57+
assert.throws(() => {
6958
runInNewContext('new WebAssembly.Module(WASM_BYTES)', { WASM_BYTES }, {
7059
contextCodeGeneration: {
7160
wasm: false,
7261
},
7362
});
74-
}, 'CompileError');
63+
}, {
64+
name: 'CompileError'
65+
});
7566

7667
common.expectsError(() => {
7768
createContext({}, {

test/sequential/test-module-loading.js

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -201,27 +201,33 @@ assert.throws(
201201

202202
assert.strictEqual(require(`${loadOrder}file1`).file1, 'file1');
203203
assert.strictEqual(require(`${loadOrder}file2`).file2, 'file2.js');
204-
try {
205-
require(`${loadOrder}file3`);
206-
} catch (e) {
207-
// Not a real .node module, but we know we require'd the right thing.
208-
if (common.isOpenBSD) // OpenBSD errors with non-ELF object error
209-
assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
210-
else
211-
assert.ok(/file3\.node/.test(e.message.replace(backslash, '/')));
212-
}
204+
assert.throws(
205+
() => require(`${loadOrder}file3`),
206+
(e) => {
207+
// Not a real .node module, but we know we require'd the right thing.
208+
if (common.isOpenBSD) { // OpenBSD errors with non-ELF object error
209+
assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
210+
} else {
211+
assert.ok(/file3\.node/.test(e.message.replace(backslash, '/')));
212+
}
213+
return true;
214+
}
215+
);
213216

214217
assert.strictEqual(require(`${loadOrder}file4`).file4, 'file4.reg');
215218
assert.strictEqual(require(`${loadOrder}file5`).file5, 'file5.reg2');
216219
assert.strictEqual(require(`${loadOrder}file6`).file6, 'file6/index.js');
217-
try {
218-
require(`${loadOrder}file7`);
219-
} catch (e) {
220-
if (common.isOpenBSD)
221-
assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
222-
else
223-
assert.ok(/file7\/index\.node/.test(e.message.replace(backslash, '/')));
224-
}
220+
assert.throws(
221+
() => require(`${loadOrder}file7`),
222+
(e) => {
223+
if (common.isOpenBSD) {
224+
assert.ok(/File not an ELF object/.test(e.message.replace(backslash, '/')));
225+
} else {
226+
assert.ok(/file7\/index\.node/.test(e.message.replace(backslash, '/')));
227+
}
228+
return true;
229+
}
230+
);
225231

226232
assert.strictEqual(require(`${loadOrder}file8`).file8, 'file8/index.reg');
227233
assert.strictEqual(require(`${loadOrder}file9`).file9, 'file9/index.reg2');

0 commit comments

Comments
 (0)