From 1cade4a9d99384f0358748131d4eb3b61e4dac56 Mon Sep 17 00:00:00 2001 From: Kinnan Kwok Date: Fri, 6 Oct 2017 12:23:58 -0700 Subject: [PATCH 1/4] test: fix flakey race condition bug in unit test --- test/addons-napi/test_promise/test.js | 42 +++++++++++++++------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js index 4c2a2e5e76c9fb..366c741a85f854 100644 --- a/test/addons-napi/test_promise/test.js +++ b/test/addons-napi/test_promise/test.js @@ -4,29 +4,33 @@ const common = require('../../common'); const test_promise = require(`./build/${common.buildType}/test_promise`); const assert = require('assert'); -let expected_result, promise; +let promise; // A resolution -expected_result = 42; -promise = test_promise.createPromise(); -promise.then( - common.mustCall(function(result) { - assert.strictEqual(result, expected_result, - 'promise resolved as expected'); - }), - common.mustNotCall()); -test_promise.concludeCurrentPromise(expected_result, true); +{ + let expected_result = 42; + promise = test_promise.createPromise(); + promise.then( + common.mustCall(function(result) { + assert.strictEqual(result, expected_result, + `promise resolved as expected, received ${result}`); + }), + common.mustNotCall()); + test_promise.concludeCurrentPromise(expected_result, true); +} // A rejection -expected_result = 'It\'s not you, it\'s me.'; -promise = test_promise.createPromise(); -promise.then( - common.mustNotCall(), - common.mustCall(function(result) { - assert.strictEqual(result, expected_result, - 'promise rejected as expected'); - })); -test_promise.concludeCurrentPromise(expected_result, false); +{ + let expected_result = 'It\'s not you, it\'s me.'; + promise = test_promise.createPromise(); + promise.then( + common.mustNotCall(), + common.mustCall(function(result) { + assert.strictEqual(result, expected_result, + `promise rejected as expected, received ${result}`); + })); + test_promise.concludeCurrentPromise(expected_result, false); +} // Chaining promise = test_promise.createPromise(); From d5c44ef74abc6b15ff1a0f63c54a0de2d96cb6e5 Mon Sep 17 00:00:00 2001 From: Kinnan Kwok Date: Fri, 6 Oct 2017 12:25:59 -0700 Subject: [PATCH 2/4] test: remove template message to display assertion values --- test/addons-napi/test_promise/test.js | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js index 366c741a85f854..49d5f696b23a01 100644 --- a/test/addons-napi/test_promise/test.js +++ b/test/addons-napi/test_promise/test.js @@ -42,23 +42,16 @@ promise.then( common.mustNotCall()); test_promise.concludeCurrentPromise(Promise.resolve('chained answer'), true); -assert.strictEqual(test_promise.isPromise(promise), true, - 'natively created promise is recognized as a promise'); +assert.strictEqual(test_promise.isPromise(promise), true); -assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true, - 'Promise created with JS is recognized as a promise'); +assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true); -assert.strictEqual(test_promise.isPromise(2.4), false, - 'Number is recognized as not a promise'); +assert.strictEqual(test_promise.isPromise(2.4), false); -assert.strictEqual(test_promise.isPromise('I promise!'), false, - 'String is recognized as not a promise'); +assert.strictEqual(test_promise.isPromise('I promise!'), false); -assert.strictEqual(test_promise.isPromise(undefined), false, - 'undefined is recognized as not a promise'); +assert.strictEqual(test_promise.isPromise(undefined), false); -assert.strictEqual(test_promise.isPromise(null), false, - 'null is recognized as not a promise'); +assert.strictEqual(test_promise.isPromise(null), false); -assert.strictEqual(test_promise.isPromise({}), false, - 'an object is recognized as not a promise'); +assert.strictEqual(test_promise.isPromise({}), false); From e51dc37ea8a5b343d338f7704c8e8b45885ead01 Mon Sep 17 00:00:00 2001 From: Kinnan Kwok Date: Sat, 7 Oct 2017 11:18:43 -0700 Subject: [PATCH 3/4] test: remove empty lines after single line assertions --- test/addons-napi/test_promise/test.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js index 49d5f696b23a01..0dc5bc9715d9bc 100644 --- a/test/addons-napi/test_promise/test.js +++ b/test/addons-napi/test_promise/test.js @@ -43,15 +43,9 @@ promise.then( test_promise.concludeCurrentPromise(Promise.resolve('chained answer'), true); assert.strictEqual(test_promise.isPromise(promise), true); - assert.strictEqual(test_promise.isPromise(Promise.reject(-1)), true); - assert.strictEqual(test_promise.isPromise(2.4), false); - assert.strictEqual(test_promise.isPromise('I promise!'), false); - assert.strictEqual(test_promise.isPromise(undefined), false); - assert.strictEqual(test_promise.isPromise(null), false); - assert.strictEqual(test_promise.isPromise({}), false); From f8b986ab7450f54e25f2cc93cea11d6ac6509c43 Mon Sep 17 00:00:00 2001 From: Kinnan Kwok Date: Sat, 7 Oct 2017 11:31:10 -0700 Subject: [PATCH 4/4] test: use const where possible --- test/addons-napi/test_promise/test.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/addons-napi/test_promise/test.js b/test/addons-napi/test_promise/test.js index 0dc5bc9715d9bc..e0c8a1a7575bdd 100644 --- a/test/addons-napi/test_promise/test.js +++ b/test/addons-napi/test_promise/test.js @@ -4,12 +4,10 @@ const common = require('../../common'); const test_promise = require(`./build/${common.buildType}/test_promise`); const assert = require('assert'); -let promise; - // A resolution { - let expected_result = 42; - promise = test_promise.createPromise(); + const expected_result = 42; + const promise = test_promise.createPromise(); promise.then( common.mustCall(function(result) { assert.strictEqual(result, expected_result, @@ -21,8 +19,8 @@ let promise; // A rejection { - let expected_result = 'It\'s not you, it\'s me.'; - promise = test_promise.createPromise(); + const expected_result = 'It\'s not you, it\'s me.'; + const promise = test_promise.createPromise(); promise.then( common.mustNotCall(), common.mustCall(function(result) { @@ -33,7 +31,7 @@ let promise; } // Chaining -promise = test_promise.createPromise(); +const promise = test_promise.createPromise(); promise.then( common.mustCall(function(result) { assert.strictEqual(result, 'chained answer',