From 83ec4b5cdc2004a88d608137a46d01897e2e7f6d Mon Sep 17 00:00:00 2001 From: Ilya Shaisultanov Date: Sun, 16 Aug 2015 20:35:38 -0700 Subject: [PATCH 1/4] assert: respect assert.doesNotThrow message. Addresses #2385. Special handling to detect when user has supplied a custom message. Added a test for user message. When testing if `actual` value is an error use `util.isError` instead of `instanceof`. --- lib/assert.js | 6 +++++- test/parallel/test-assert.js | 7 +++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/assert.js b/lib/assert.js index b4de551e28019f..d00c892c9106d6 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -330,7 +330,11 @@ function _throws(shouldThrow, block, expected, message) { fail(actual, expected, 'Missing expected exception' + message); } - if (!shouldThrow && expectedException(actual, expected)) { + if (!shouldThrow && + actual instanceof Error && + typeof message === 'string' && + expectedException(actual, expected) || + !shouldThrow && actual && !expected) { fail(actual, expected, 'Got unwanted exception' + message); } diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 2f4b757f4ec43e..f9bb3af409fe39 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -321,6 +321,13 @@ assert.throws(function() {assert.ifError(new Error('test error'));}); assert.doesNotThrow(function() {assert.ifError(null);}); assert.doesNotThrow(function() {assert.ifError();}); +try { + assert.doesNotThrow(makeBlock(thrower, Error), 'user message'); +} catch(e) { + assert.equal(e.message, 'Got unwanted exception. user message', + 'a.doesNotThrow ignores user message'); +} + // make sure that validating using constructor really works threw = false; try { From d741140c1979ba82512e21072287d2e830430810 Mon Sep 17 00:00:00 2001 From: Ilya Shaisultanov Date: Tue, 5 Apr 2016 19:24:11 -0700 Subject: [PATCH 2/4] assert: clean up conditional for _throws, fix docs. Addresses #2407. Break out conditional into named variables for readability. Explain the purpose of `message` in throws/doesNotThrow assertions. --- lib/assert.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index d00c892c9106d6..90a88653abba77 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -330,11 +330,14 @@ function _throws(shouldThrow, block, expected, message) { fail(actual, expected, 'Missing expected exception' + message); } - if (!shouldThrow && - actual instanceof Error && - typeof message === 'string' && - expectedException(actual, expected) || - !shouldThrow && actual && !expected) { + var userProvidedMessage = typeof message === 'string'; + var isUnwantedException = !shouldThrow && util.isError(actual); + var isUnexpectedException = !shouldThrow && actual && !expected; + + if ((isUnwantedException && + userProvidedMessage && + expectedException(actual, expected)) || + isUnexpectedException) { fail(actual, expected, 'Got unwanted exception' + message); } From f3eac76f60ff199d9c4fea058c585b3ee55fd139 Mon Sep 17 00:00:00 2001 From: Ilya Shaisultanov Date: Wed, 13 Apr 2016 16:39:33 -0700 Subject: [PATCH 3/4] assert: reformat docs, use const, refactor a test Addresses #2407. Wrap long lines in assert.markdown to be < 80 chars. Use const instead of var in assert.js. Refactor assert.doesNotThrow test to use assert.throws. --- lib/assert.js | 6 +++--- test/parallel/test-assert.js | 7 ++----- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/assert.js b/lib/assert.js index 90a88653abba77..7fc9e0c342646a 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -330,9 +330,9 @@ function _throws(shouldThrow, block, expected, message) { fail(actual, expected, 'Missing expected exception' + message); } - var userProvidedMessage = typeof message === 'string'; - var isUnwantedException = !shouldThrow && util.isError(actual); - var isUnexpectedException = !shouldThrow && actual && !expected; + const userProvidedMessage = typeof message === 'string'; + const isUnwantedException = !shouldThrow && util.isError(actual); + const isUnexpectedException = !shouldThrow && actual && !expected; if ((isUnwantedException && userProvidedMessage && diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index f9bb3af409fe39..035b9c7ce158f5 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -321,12 +321,9 @@ assert.throws(function() {assert.ifError(new Error('test error'));}); assert.doesNotThrow(function() {assert.ifError(null);}); assert.doesNotThrow(function() {assert.ifError();}); -try { +assert.throws(function() { assert.doesNotThrow(makeBlock(thrower, Error), 'user message'); -} catch(e) { - assert.equal(e.message, 'Got unwanted exception. user message', - 'a.doesNotThrow ignores user message'); -} +}, /Got unwanted exception. user message/, 'a.doesNotThrow ignores user message'); // make sure that validating using constructor really works threw = false; From 610a38ea7f71223b940273aaebc30cfee340dfc8 Mon Sep 17 00:00:00 2001 From: Ilya Shaisultanov Date: Fri, 15 Apr 2016 13:05:31 -0700 Subject: [PATCH 4/4] assert: fix linter error in test-assert.js Addresses #2407. Wrap the line that exceeded 80 chars. --- test/parallel/test-assert.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index 035b9c7ce158f5..e537bc3c1d747e 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -321,9 +321,10 @@ assert.throws(function() {assert.ifError(new Error('test error'));}); assert.doesNotThrow(function() {assert.ifError(null);}); assert.doesNotThrow(function() {assert.ifError();}); -assert.throws(function() { +assert.throws(() => { assert.doesNotThrow(makeBlock(thrower, Error), 'user message'); -}, /Got unwanted exception. user message/, 'a.doesNotThrow ignores user message'); +}, /Got unwanted exception. user message/, + 'a.doesNotThrow ignores user message'); // make sure that validating using constructor really works threw = false;