From 4d3a0b175d23b401379e52ab3c3c17c131e52e6b Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Wed, 22 Nov 2017 13:49:50 -0500 Subject: [PATCH 1/8] test: make common.mustNotCall show file:linenumber When a test fails via `common.mustNotCall` it is sometimes hard to determine exactly what was called. This modification stores the caller's file and line number by using the V8 Error API to capture a stack at the time `common.mustNotCall()` is called. In the event of failure, this information is printed. --- test/common/index.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/common/index.js b/test/common/index.js index a78f90087793c1..6e60c54d0e9016 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -566,9 +566,24 @@ exports.canCreateSymLink = function() { return true; }; +function getCallSite(level = 2) { + const originalStackFormatter = Error.prepareStackTrace; + Error.prepareStackTrace = (err, stack) => + `${stack[level].getFileName()}:${stack[level].getLineNumber()}`; + const err = new Error(); + Error.captureStackTrace(err, level); + // the way V8 Error API works, the stack is not + // formatted until it is accessed + err.stack; + Error.prepareStackTrace = originalStackFormatter; + err.stack; +} + exports.mustNotCall = function(msg) { + const callSite = getCallSite(); return function mustNotCall() { - assert.fail(msg || 'function should not have been called'); + assert.fail( + `${msg || 'function should not have been called'} at ${callSite}`); }; }; From c54e1e14df1bf8bf2a65c0347971bbe70e8d5450 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Wed, 22 Nov 2017 15:45:38 -0500 Subject: [PATCH 2/8] test: add test for common.mustNotCall --- test/common/index.js | 2 +- test/parallel/test-common-must-not-call.js | 23 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-common-must-not-call.js diff --git a/test/common/index.js b/test/common/index.js index 6e60c54d0e9016..b9b243fa3a7435 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -576,7 +576,7 @@ function getCallSite(level = 2) { // formatted until it is accessed err.stack; Error.prepareStackTrace = originalStackFormatter; - err.stack; + return err.stack; } exports.mustNotCall = function(msg) { diff --git a/test/parallel/test-common-must-not-call.js b/test/parallel/test-common-must-not-call.js new file mode 100644 index 00000000000000..e0a1b87b517723 --- /dev/null +++ b/test/parallel/test-common-must-not-call.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); + +const message = 'message'; +const testFunction = common.mustNotCall(message); + +const validateError = common.mustCall((e) => { + const prefix = `${message} at `; + assert.ok(e.message.startsWith(prefix)); + const [ fileName, lineNumber ] = e.message + .substring(prefix.length).split(':'); + assert.strictEqual(path.basename(fileName), 'test-common-must-not-call.js'); + assert.strictEqual(lineNumber, '8'); +}); + +try { + testFunction(); +} catch (e) { + validateError(e); +} From b9402b749f354b99b136dc9db53443aba9f6e370 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Thu, 23 Nov 2017 09:39:14 -0500 Subject: [PATCH 3/8] test: export commmon.getCallSite --- test/common/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index b9b243fa3a7435..7d329b25c4f561 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -566,21 +566,21 @@ exports.canCreateSymLink = function() { return true; }; -function getCallSite(level = 2) { +exports.getCallSite = function getCallSite(level = 2) { const originalStackFormatter = Error.prepareStackTrace; Error.prepareStackTrace = (err, stack) => `${stack[level].getFileName()}:${stack[level].getLineNumber()}`; const err = new Error(); - Error.captureStackTrace(err, level); + Error.captureStackTrace(err); // the way V8 Error API works, the stack is not // formatted until it is accessed err.stack; Error.prepareStackTrace = originalStackFormatter; return err.stack; -} +}; exports.mustNotCall = function(msg) { - const callSite = getCallSite(); + const callSite = exports.getCallSite(); return function mustNotCall() { assert.fail( `${msg || 'function should not have been called'} at ${callSite}`); From 9e0363d778536b5c0865ad697fd146ff0347e39f Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Thu, 23 Nov 2017 09:48:52 -0500 Subject: [PATCH 4/8] test: change common.getCallSite to take a function Instead of indexing into the call stack to find the right level to store, just use the `Error.captureStackTrace()` function's second parameter - a function pointer. --- test/common/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index 7d329b25c4f561..0a7f45624e2a6c 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -566,12 +566,12 @@ exports.canCreateSymLink = function() { return true; }; -exports.getCallSite = function getCallSite(level = 2) { +exports.getCallSite = function getCallSite(top) { const originalStackFormatter = Error.prepareStackTrace; Error.prepareStackTrace = (err, stack) => - `${stack[level].getFileName()}:${stack[level].getLineNumber()}`; + `${stack[0].getFileName()}:${stack[0].getLineNumber()}`; const err = new Error(); - Error.captureStackTrace(err); + Error.captureStackTrace(err, top); // the way V8 Error API works, the stack is not // formatted until it is accessed err.stack; @@ -580,7 +580,7 @@ exports.getCallSite = function getCallSite(level = 2) { }; exports.mustNotCall = function(msg) { - const callSite = exports.getCallSite(); + const callSite = exports.getCallSite(exports.mustNotCall); return function mustNotCall() { assert.fail( `${msg || 'function should not have been called'} at ${callSite}`); From 451ea32d8937db1b6d68c6a5b00d368d9aa246a6 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Fri, 24 Nov 2017 11:57:47 -0500 Subject: [PATCH 5/8] test: fix comment formatting --- test/common/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index 0a7f45624e2a6c..2cf188abe5ffc9 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -572,8 +572,7 @@ exports.getCallSite = function getCallSite(top) { `${stack[0].getFileName()}:${stack[0].getLineNumber()}`; const err = new Error(); Error.captureStackTrace(err, top); - // the way V8 Error API works, the stack is not - // formatted until it is accessed + // with the V8 Error API, the stack is not formatted until it is accessed err.stack; Error.prepareStackTrace = originalStackFormatter; return err.stack; From ae47bcefb90443ad226d320ea7fde207a0c9147c Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Fri, 24 Nov 2017 12:02:02 -0500 Subject: [PATCH 6/8] test: document common.getCallSite() --- test/common/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/common/README.md b/test/common/README.md index 7fa47677ca8ce2..53be30bfd7c0b8 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -133,6 +133,12 @@ a reason otherwise. Returns an instance of all possible `ArrayBufferView`s of the provided Buffer. +### getCallSite(func) +* `func` [<Function] +* return [<String>] + +Returns the file name and line number for the provided Function. + ### globalCheck * [<Boolean>] From 9e23ec226ba99464b3eccbb9562c676fbe4ab68a Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Tue, 28 Nov 2017 12:29:57 -0500 Subject: [PATCH 7/8] test: add check for win32 and modify message Because of the way this test splits up the message string on ':' we need to ensure that the initial drive letter does not affect the string manipulation. Shortening it by 2 characters solves this. --- test/parallel/test-common-must-not-call.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/parallel/test-common-must-not-call.js b/test/parallel/test-common-must-not-call.js index e0a1b87b517723..d70daabf0a4bd0 100644 --- a/test/parallel/test-common-must-not-call.js +++ b/test/parallel/test-common-must-not-call.js @@ -10,6 +10,9 @@ const testFunction = common.mustNotCall(message); const validateError = common.mustCall((e) => { const prefix = `${message} at `; assert.ok(e.message.startsWith(prefix)); + if (process.platform === 'win32') { + e.message = e.message.substring(2); // remove 'C:' + } const [ fileName, lineNumber ] = e.message .substring(prefix.length).split(':'); assert.strictEqual(path.basename(fileName), 'test-common-must-not-call.js'); From 8e7589c8053b2a1c6b6c347dc590d02edadb52b4 Mon Sep 17 00:00:00 2001 From: Lance Ball Date: Fri, 1 Dec 2017 08:49:34 -0500 Subject: [PATCH 8/8] test: fix typo in common.getCallSite doc --- test/common/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 53be30bfd7c0b8..f48215836ecae5 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -134,7 +134,7 @@ a reason otherwise. Returns an instance of all possible `ArrayBufferView`s of the provided Buffer. ### getCallSite(func) -* `func` [<Function] +* `func` [<Function>] * return [<String>] Returns the file name and line number for the provided Function.