From 9de2e159c4e83bea80c1d96abdabc6a69207344c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Fri, 7 Apr 2017 16:15:46 +0200 Subject: [PATCH 1/2] test: add second argument to assert.throws This adds RegExp or error constructor arguments to the remaining places where it is missing in preparation for the commit that will enforce the presence of at least two arguments. PR-URL: https://github.com/nodejs/node/pull/12270 Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- test/addons-napi/test_constructor/test.js | 6 ++-- test/addons-napi/test_properties/test.js | 6 ++-- test/parallel/test-assert-checktag.js | 6 ++-- test/parallel/test-assert-deep.js | 36 ++++++++++++------- test/parallel/test-assert.js | 11 ++++-- test/parallel/test-repl-context.js | 2 +- .../test-vm-new-script-this-context.js | 2 +- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/test/addons-napi/test_constructor/test.js b/test/addons-napi/test_constructor/test.js index b1762c2253b9..92440bf49e74 100644 --- a/test/addons-napi/test_constructor/test.js +++ b/test/addons-napi/test_constructor/test.js @@ -13,7 +13,7 @@ assert.strictEqual(test_object.readwriteValue, 1); test_object.readwriteValue = 2; assert.strictEqual(test_object.readwriteValue, 2); -assert.throws(() => { test_object.readonlyValue = 3; }); +assert.throws(() => { test_object.readonlyValue = 3; }, TypeError); assert.ok(test_object.hiddenValue); @@ -35,8 +35,8 @@ assert.ok(propertyNames.indexOf('readonlyAccessor2') < 0); test_object.readwriteAccessor1 = 1; assert.strictEqual(test_object.readwriteAccessor1, 1); assert.strictEqual(test_object.readonlyAccessor1, 1); -assert.throws(() => { test_object.readonlyAccessor1 = 3; }); +assert.throws(() => { test_object.readonlyAccessor1 = 3; }, TypeError); test_object.readwriteAccessor2 = 2; assert.strictEqual(test_object.readwriteAccessor2, 2); assert.strictEqual(test_object.readonlyAccessor2, 2); -assert.throws(() => { test_object.readonlyAccessor2 = 3; }); +assert.throws(() => { test_object.readonlyAccessor2 = 3; }, TypeError); diff --git a/test/addons-napi/test_properties/test.js b/test/addons-napi/test_properties/test.js index fb0c19ceedb0..868c603879f1 100644 --- a/test/addons-napi/test_properties/test.js +++ b/test/addons-napi/test_properties/test.js @@ -12,7 +12,7 @@ assert.strictEqual(test_object.readwriteValue, 1); test_object.readwriteValue = 2; assert.strictEqual(test_object.readwriteValue, 2); -assert.throws(() => { test_object.readonlyValue = 3; }); +assert.throws(() => { test_object.readonlyValue = 3; }, TypeError); assert.ok(test_object.hiddenValue); @@ -34,8 +34,8 @@ assert.ok(propertyNames.indexOf('readonlyAccessor2') < 0); test_object.readwriteAccessor1 = 1; assert.strictEqual(test_object.readwriteAccessor1, 1); assert.strictEqual(test_object.readonlyAccessor1, 1); -assert.throws(() => { test_object.readonlyAccessor1 = 3; }); +assert.throws(() => { test_object.readonlyAccessor1 = 3; }, TypeError); test_object.readwriteAccessor2 = 2; assert.strictEqual(test_object.readwriteAccessor2, 2); assert.strictEqual(test_object.readonlyAccessor2, 2); -assert.throws(() => { test_object.readonlyAccessor2 = 3; }); +assert.throws(() => { test_object.readonlyAccessor2 = 3; }, TypeError); diff --git a/test/parallel/test-assert-checktag.js b/test/parallel/test-assert-checktag.js index ae409c725035..c050beeed6f6 100644 --- a/test/parallel/test-assert-checktag.js +++ b/test/parallel/test-assert-checktag.js @@ -45,7 +45,8 @@ function re(literals, ...values) { } assert.doesNotThrow(() => assert.deepEqual(fakeGlobal, global)); // Message will be truncated anyway, don't validate - assert.throws(() => assert.deepStrictEqual(fakeGlobal, global)); + assert.throws(() => assert.deepStrictEqual(fakeGlobal, global), + assert.AssertionError); } { // At the moment process has its own type tag @@ -56,6 +57,7 @@ function re(literals, ...values) { } assert.doesNotThrow(() => assert.deepEqual(fakeProcess, process)); // Message will be truncated anyway, don't validate - assert.throws(() => assert.deepStrictEqual(fakeProcess, process)); + assert.throws(() => assert.deepStrictEqual(fakeProcess, process), + assert.AssertionError); } /* eslint-enable */ diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index 46fec2a512f5..70f538d91912 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -26,18 +26,21 @@ function re(literals, ...values) { const arr = new Uint8Array([120, 121, 122, 10]); const buf = Buffer.from(arr); // They have different [[Prototype]] -assert.throws(() => assert.deepStrictEqual(arr, buf)); +assert.throws(() => assert.deepStrictEqual(arr, buf), + re`${arr} deepStrictEqual ${buf}`); assert.doesNotThrow(() => assert.deepEqual(arr, buf)); const buf2 = Buffer.from(arr); buf2.prop = 1; -assert.throws(() => assert.deepStrictEqual(buf2, buf)); +assert.throws(() => assert.deepStrictEqual(buf2, buf), + re`${buf2} deepStrictEqual ${buf}`); assert.doesNotThrow(() => assert.deepEqual(buf2, buf)); const arr2 = new Uint8Array([120, 121, 122, 10]); arr2.prop = 5; -assert.throws(() => assert.deepStrictEqual(arr, arr2)); +assert.throws(() => assert.deepStrictEqual(arr, arr2), + re`${arr} deepStrictEqual ${arr2}`); assert.doesNotThrow(() => assert.deepEqual(arr, arr2)); const date = new Date('2016'); @@ -121,19 +124,23 @@ function assertDeepAndStrictEqual(a, b) { } function assertNotDeepOrStrict(a, b) { - assert.throws(() => assert.deepEqual(a, b)); - assert.throws(() => assert.deepStrictEqual(a, b)); + assert.throws(() => assert.deepEqual(a, b), re`${a} deepEqual ${b}`); + assert.throws(() => assert.deepStrictEqual(a, b), + re`${a} deepStrictEqual ${b}`); - assert.throws(() => assert.deepEqual(b, a)); - assert.throws(() => assert.deepStrictEqual(b, a)); + assert.throws(() => assert.deepEqual(b, a), re`${b} deepEqual ${a}`); + assert.throws(() => assert.deepStrictEqual(b, a), + re`${b} deepStrictEqual ${a}`); } function assertOnlyDeepEqual(a, b) { assert.doesNotThrow(() => assert.deepEqual(a, b)); - assert.throws(() => assert.deepStrictEqual(a, b)); + assert.throws(() => assert.deepStrictEqual(a, b), + re`${a} deepStrictEqual ${b}`); assert.doesNotThrow(() => assert.deepEqual(b, a)); - assert.throws(() => assert.deepStrictEqual(b, a)); + assert.throws(() => assert.deepStrictEqual(b, a), + re`${b} deepStrictEqual ${a}`); } // es6 Maps and Sets @@ -232,10 +239,13 @@ assertDeepAndStrictEqual( assertNotDeepOrStrict(m1, m2); } -assert.deepEqual(new Map([[1, 1]]), new Map([[1, '1']])); -assert.throws(() => - assert.deepStrictEqual(new Map([[1, 1]]), new Map([[1, '1']])) -); +{ + const map1 = new Map([[1, 1]]); + const map2 = new Map([[1, '1']]); + assert.deepEqual(map1, map2); + assert.throws(() => assert.deepStrictEqual(map1, map2), + re`${map1} deepStrictEqual ${map2}`); +} { // Two equivalent sets / maps with different key/values applied shouldn't be diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index c84d59623272..89a54df81a70 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -274,10 +274,12 @@ assert.doesNotThrow(makeBlock(a.deepStrictEqual, {a: 4}, {a: 4})); assert.doesNotThrow(makeBlock(a.deepStrictEqual, {a: 4, b: '2'}, {a: 4, b: '2'})); -assert.throws(makeBlock(a.deepStrictEqual, [4], ['4'])); +assert.throws(makeBlock(a.deepStrictEqual, [4], ['4']), + /^AssertionError: \[ 4 ] deepStrictEqual \[ '4' ]$/); assert.throws(makeBlock(a.deepStrictEqual, {a: 4}, {a: 4, b: true}), - a.AssertionError); -assert.throws(makeBlock(a.deepStrictEqual, ['a'], {0: 'a'})); + /^AssertionError: { a: 4 } deepStrictEqual { a: 4, b: true }$/); +assert.throws(makeBlock(a.deepStrictEqual, ['a'], {0: 'a'}), + /^AssertionError: \[ 'a' ] deepStrictEqual { '0': 'a' }$/); //(although not necessarily the same order), assert.doesNotThrow(makeBlock(a.deepStrictEqual, {a: 4, b: '1'}, @@ -354,9 +356,11 @@ function thrower(errorConstructor) { assert.throws(makeBlock(thrower, a.AssertionError), a.AssertionError, 'message'); assert.throws(makeBlock(thrower, a.AssertionError), a.AssertionError); +// eslint-disable-next-line assert-throws-arguments assert.throws(makeBlock(thrower, a.AssertionError)); // if not passing an error, catch all. +// eslint-disable-next-line assert-throws-arguments assert.throws(makeBlock(thrower, TypeError)); // when passing a type, only catch errors of the appropriate type @@ -565,6 +569,7 @@ testAssertionMessage({a: NaN, b: Infinity, c: -Infinity}, { let threw = false; try { + // eslint-disable-next-line assert-throws-arguments assert.throws(function() { assert.ifError(null); }); diff --git a/test/parallel/test-repl-context.js b/test/parallel/test-repl-context.js index 2287a247d81b..a2f04214311c 100644 --- a/test/parallel/test-repl-context.js +++ b/test/parallel/test-repl-context.js @@ -22,5 +22,5 @@ function testContext(repl) { assert.strictEqual(context.global, context); // ensure that the repl console instance does not have a setter - assert.throws(() => context.console = 'foo'); + assert.throws(() => context.console = 'foo', TypeError); } diff --git a/test/parallel/test-vm-new-script-this-context.js b/test/parallel/test-vm-new-script-this-context.js index 7ddb2110085e..dfa9b1130b1f 100644 --- a/test/parallel/test-vm-new-script-this-context.js +++ b/test/parallel/test-vm-new-script-this-context.js @@ -35,7 +35,7 @@ console.error('thrown error'); script = new Script('throw new Error(\'test\');'); assert.throws(function() { script.runInThisContext(script); -}); +}, /^Error: test$/); global.hello = 5; script = new Script('hello = 2'); From a4b9c585b33bfc0921a405fc54a8cb9783046b6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Fri, 7 Apr 2017 16:18:02 +0200 Subject: [PATCH 2/2] tools: enforce two arguments in assert.throws Enables the requireTwo option of our custom rule. PR-URL: https://github.com/nodejs/node/pull/12270 Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- .eslintrc.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 630fd19688c4..7468a234382d 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -142,7 +142,7 @@ rules: # Custom rules in tools/eslint-rules align-multiline-assignment: 2 - assert-throws-arguments: [2, { requireTwo: false }] + assert-throws-arguments: [2, { requireTwo: true }] no-unescaped-regexp-dot: 2 # Global scoped method and vars