From f8f564c55c0444baf8f7063550bb8efaecba3912 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Tue, 5 Sep 2017 15:27:21 -0400 Subject: [PATCH 1/4] Relocate test --- test/beautify-stack.js | 21 +++++++++++++++++++++ test/reporters/verbose.js | 21 --------------------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/test/beautify-stack.js b/test/beautify-stack.js index 0609d9335..6f5567e72 100644 --- a/test/beautify-stack.js +++ b/test/beautify-stack.js @@ -10,6 +10,14 @@ const beautifyStack = proxyquire('../lib/beautify-stack', { } }); +function fooFunc() { + barFunc(); +} + +function barFunc() { + throw new Error(); +} + test('does not strip ava internals and dependencies from stack trace with debug enabled', t => { const beautify = proxyquire('../lib/beautify-stack', { debug() { @@ -44,3 +52,16 @@ test('returns empty string without any arguments', t => { t.is(beautifyStack(), ''); t.end(); }); + +test('beautify stack - removes uninteresting lines', t => { + try { + fooFunc(); + } catch (err) { + const stack = beautifyStack(err.stack); + t.match(stack, /fooFunc/); + t.match(stack, /barFunc/); + t.match(err.stack, /Module._compile/); + t.notMatch(stack, /Module\._compile/); + t.end(); + } +}); diff --git a/test/reporters/verbose.js b/test/reporters/verbose.js index 228bab288..3c3f8d0af 100644 --- a/test/reporters/verbose.js +++ b/test/reporters/verbose.js @@ -46,14 +46,6 @@ function createRunStatus() { }; } -function fooFunc() { - barFunc(); -} - -function barFunc() { - throw new Error(); -} - function source(file, line) { return { file, @@ -63,19 +55,6 @@ function source(file, line) { }; } -test('beautify stack - removes uninteresting lines', t => { - try { - fooFunc(); - } catch (err) { - const stack = beautifyStack(err.stack); - t.match(stack, /fooFunc/); - t.match(stack, /barFunc/); - t.match(err.stack, /Module._compile/); - t.notMatch(stack, /Module\._compile/); - t.end(); - } -}); - test('start', t => { const reporter = createReporter(); From e24364531f954dbadf0837122de1f26fc1ded855 Mon Sep 17 00:00:00 2001 From: Kay Lovelace Date: Sun, 19 Mar 2017 15:16:40 +0000 Subject: [PATCH 2/4] Include anonymous functions in stack traces Some functions are more anonymous than others. Node does a pretty good job of naming functions like this: ```js const fn = () => { throw "whoops!" } // <-- gets named `fn` in stacktraces ``` But in some cases a function genuinely has no name, e.g. ```js const fn = () => () => { throw "whoops!" } // <-- inner function has no name! ``` This means we get stacktraces like this: ``` whoops! path/to/file.js:1:1 Test.t (test.js:1:1) ``` Before this commit, the regex in `extract-stack.js` was not anticipating stacktraces of this form, and would simply filter out the anonymous functions. This made my life a little more awkward, as I could only see the failing test line and the error, not the code that was actually erroring. This commit adds another regex to `extract-stack.js` which matches on anonymous function stacktrace lines too. --- lib/extract-stack.js | 2 +- test/cli.js | 8 ++++++++ test/extract-stack.js | 10 ++++++++++ test/fixture/error-in-anonymous-function.js | 9 +++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 test/fixture/error-in-anonymous-function.js diff --git a/lib/extract-stack.js b/lib/extract-stack.js index 64f63db1c..c6e0fea86 100644 --- a/lib/extract-stack.js +++ b/lib/extract-stack.js @@ -1,5 +1,5 @@ 'use strict'; -const stackLineRegex = /^.+ \(.+:[0-9]+:[0-9]+\)$/; +const stackLineRegex = /^.+( \(.+:[0-9]+:[0-9]+\)|:[0-9]+:[0-9]+)$/; module.exports = stack => { return stack diff --git a/test/cli.js b/test/cli.js index b924000e3..35ff3d83a 100644 --- a/test/cli.js +++ b/test/cli.js @@ -101,6 +101,14 @@ test('throwing a named function will report the to the console', t => { }); }); +test('include anonymous functions in error reports', t => { + execCli('fixture/error-in-anonymous-function.js', (err, stdout, stderr) => { + t.ok(err); + t.match(stderr, /test\/fixture\/error-in-anonymous-function\.js:4:8/); + t.end(); + }); +}); + test('improper use of t.throws will be reported to the console', t => { execCli('fixture/improper-t-throws/throws.js', (err, stdout, stderr) => { t.ok(err); diff --git a/test/extract-stack.js b/test/extract-stack.js index 362e825fa..d0fc8612c 100644 --- a/test/extract-stack.js +++ b/test/extract-stack.js @@ -34,3 +34,13 @@ test('strip beginning whitespace from stack', t => { t.is(extractStack(stack), 'Test.t (test.js:1:1)'); t.end(); }); + +test('includes anonymous function lines', t => { + const stack = [ + 'error message', + 'path/to/test.js:1:1' + ].join('\n'); + + t.is(extractStack(stack), 'path/to/test.js:1:1'); + t.end(); +}); diff --git a/test/fixture/error-in-anonymous-function.js b/test/fixture/error-in-anonymous-function.js new file mode 100644 index 000000000..db5f0d571 --- /dev/null +++ b/test/fixture/error-in-anonymous-function.js @@ -0,0 +1,9 @@ +import test from '../../'; + +const getAnonymousFn = () => () => { + throw Error(); +}; + +test(t => { + getAnonymousFn()(); +}); From d54a52b37ed7daee2087c3359e4ee111c317d0a0 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Mon, 4 Sep 2017 19:52:50 -0400 Subject: [PATCH 3/4] Re-factor error object processing Incorporate review feedback for the prior commit. Additionally, update test fixture file to satisfy the project's linting rules. --- lib/beautify-stack.js | 51 +++++++++++-- lib/extract-stack.js | 10 --- lib/reporters/mini.js | 21 +++-- lib/reporters/tap.js | 8 +- lib/reporters/verbose.js | 8 +- lib/serialize-error.js | 10 ++- test/extract-stack.js | 46 ----------- test/fixture/error-in-anonymous-function.js | 3 +- test/helper/error-from-worker.js | 23 ++++++ test/reporters/mini.js | 85 +++++++++------------ test/reporters/tap.js | 4 +- test/reporters/verbose.js | 73 ++++++++---------- test/serialize-error.js | 3 +- 13 files changed, 165 insertions(+), 180 deletions(-) delete mode 100644 lib/extract-stack.js delete mode 100644 test/extract-stack.js create mode 100644 test/helper/error-from-worker.js diff --git a/lib/beautify-stack.js b/lib/beautify-stack.js index 189ed0714..481bf7b62 100644 --- a/lib/beautify-stack.js +++ b/lib/beautify-stack.js @@ -8,6 +8,7 @@ let ignoreStackLines = []; const avaInternals = /\/ava\/(?:lib\/)?[\w-]+\.js:\d+:\d+\)?$/; const avaDependencies = /\/node_modules\/(?:bluebird|empower-core|(?:ava\/node_modules\/)?(?:babel-runtime|core-js))\//; +const stackFrameLine = /^.+( \(.+:[0-9]+:[0-9]+\)|:[0-9]+:[0-9]+)$/; if (!debug.enabled) { ignoreStackLines = StackUtils.nodeInternals(); @@ -17,21 +18,55 @@ if (!debug.enabled) { const stackUtils = new StackUtils({internals: ignoreStackLines}); +function extractFrames(stack) { + return stack + .split('\n') + .map(line => line.trim()) + .filter(line => stackFrameLine.test(line)) + .join('\n'); +} + +/** + * Given a string value of the format generated for the `stack` property of a + * V8 error object, return a string that contains only stack frame information + * for frames that have relevance to the consumer. + * + * For example, given the following string value: + * + * ``` + * Error + * at inner (/home/ava/ex.js:7:12) + * at /home/ava/ex.js:12:5 + * at outer (/home/ava/ex.js:13:4) + * at Object. (/home/ava/ex.js:14:3) + * at Module._compile (module.js:570:32) + * at Object.Module._extensions..js (module.js:579:10) + * at Module.load (module.js:487:32) + * at tryModuleLoad (module.js:446:12) + * at Function.Module._load (module.js:438:3) + * at Module.runMain (module.js:604:10) + * ``` + * + * ...this function returns the following string value: + * + * ``` + * inner (/home/ava/ex.js:7:12) + * /home/ava/ex.js:12:5 + * outer (/home/ava/ex.js:13:4) + * Object. (/home/ava/ex.js:14:3) + * ``` + */ module.exports = stack => { if (!stack) { return ''; } + stack = extractFrames(stack); // Workaround for https://github.com/tapjs/stack-utils/issues/14 // TODO: fix it in `stack-utils` stack = cleanStack(stack); - const title = stack.split('\n')[0]; - const lines = stackUtils - .clean(stack) - .split('\n') - .map(x => ` ${x}`) - .join('\n'); - - return `${title}\n${lines}`; + return stackUtils.clean(stack) + // Remove the trailing newline inserted by the `stack-utils` module + .trim(); }; diff --git a/lib/extract-stack.js b/lib/extract-stack.js deleted file mode 100644 index c6e0fea86..000000000 --- a/lib/extract-stack.js +++ /dev/null @@ -1,10 +0,0 @@ -'use strict'; -const stackLineRegex = /^.+( \(.+:[0-9]+:[0-9]+\)|:[0-9]+:[0-9]+)$/; - -module.exports = stack => { - return stack - .split('\n') - .filter(line => stackLineRegex.test(line)) - .map(line => line.trim()) - .join('\n'); -}; diff --git a/lib/reporters/mini.js b/lib/reporters/mini.js index 8f3b8b34d..14d372a4a 100644 --- a/lib/reporters/mini.js +++ b/lib/reporters/mini.js @@ -11,7 +11,6 @@ const cross = require('figures').cross; const indentString = require('indent-string'); const ansiEscapes = require('ansi-escapes'); const trimOffNewlines = require('trim-off-newlines'); -const extractStack = require('../extract-stack'); const codeExcerpt = require('../code-excerpt'); const colors = require('../colors'); const formatSerializedError = require('./format-serialized-error'); @@ -207,9 +206,9 @@ class MiniReporter { } if (test.error.stack) { - const extracted = extractStack(test.error.stack); - if (extracted.includes('\n')) { - status += '\n' + indentString(colors.errorStack(extracted), 2) + '\n'; + const stack = test.error.stack; + if (stack.includes('\n')) { + status += '\n' + indentString(colors.errorStack(stack), 2) + '\n'; } } @@ -228,14 +227,14 @@ class MiniReporter { status += ' ' + colors.error(cross + ' ' + err.message) + '\n\n'; } else { const title = err.type === 'rejection' ? 'Unhandled Rejection' : 'Uncaught Exception'; - let description = err.stack ? err.stack.trimRight() : JSON.stringify(err); - description = description.split('\n'); - const errorTitle = err.name ? description[0] : 'Threw non-error: ' + description[0]; - const errorStack = description.slice(1).join('\n'); - status += ' ' + colors.title(title) + '\n'; - status += ' ' + colors.stack(errorTitle) + '\n'; - status += colors.errorStack(errorStack) + '\n\n'; + + if (err.name) { + status += ' ' + colors.stack(err.summary) + '\n'; + status += colors.errorStack(err.stack) + '\n\n'; + } else { + status += ' Threw non-error: ' + err.summary + '\n'; + } } }); } diff --git a/lib/reporters/tap.js b/lib/reporters/tap.js index 976ae3083..1670df575 100644 --- a/lib/reporters/tap.js +++ b/lib/reporters/tap.js @@ -3,12 +3,6 @@ const format = require('util').format; const indentString = require('indent-string'); const stripAnsi = require('strip-ansi'); const yaml = require('js-yaml'); -const extractStack = require('../extract-stack'); - -// Parses stack trace and extracts original function name, file name and line -function getSourceFromStack(stack) { - return extractStack(stack).split('\n')[0]; -} function dumpError(error, includeMessage) { const obj = Object.assign({}, error.object); @@ -35,7 +29,7 @@ function dumpError(error, includeMessage) { } if (error.stack) { - obj.at = getSourceFromStack(error.stack); + obj.at = error.stack.split('\n')[0]; } return ` ---\n${indentString(yaml.safeDump(obj).trim(), 4)}\n ...`; diff --git a/lib/reporters/verbose.js b/lib/reporters/verbose.js index 20091573a..0d007f052 100644 --- a/lib/reporters/verbose.js +++ b/lib/reporters/verbose.js @@ -5,7 +5,6 @@ const figures = require('figures'); const chalk = require('chalk'); const plur = require('plur'); const trimOffNewlines = require('trim-off-newlines'); -const extractStack = require('../extract-stack'); const codeExcerpt = require('../code-excerpt'); const colors = require('../colors'); const formatSerializedError = require('./format-serialized-error'); @@ -70,6 +69,7 @@ class VerboseReporter { let output = colors.error(types[err.type] + ':', err.file) + '\n'; if (err.stack) { + output += ' ' + colors.stack(err.title || err.summary) + '\n'; output += ' ' + colors.stack(err.stack) + '\n'; } else { output += ' ' + colors.stack(JSON.stringify(err)) + '\n'; @@ -156,9 +156,9 @@ class VerboseReporter { } if (test.error.stack) { - const extracted = extractStack(test.error.stack); - if (extracted.includes('\n')) { - output += '\n' + indentString(colors.errorStack(extracted), 2) + '\n'; + const stack = test.error.stack; + if (stack.includes('\n')) { + output += '\n' + indentString(colors.errorStack(stack), 2) + '\n'; } } diff --git a/lib/serialize-error.js b/lib/serialize-error.js index 55717e161..aebc5254d 100644 --- a/lib/serialize-error.js +++ b/lib/serialize-error.js @@ -4,7 +4,6 @@ const cleanYamlObject = require('clean-yaml-object'); const StackUtils = require('stack-utils'); const assert = require('./assert'); const beautifyStack = require('./beautify-stack'); -const extractStack = require('./extract-stack'); function isAvaAssertionError(source) { return source instanceof assert.AssertionError; @@ -20,7 +19,7 @@ function extractSource(stack) { return null; } - const firstStackLine = extractStack(stack).split('\n')[0]; + const firstStackLine = stack.split('\n')[0]; return stackUtils.parseLine(firstStackLine); } function buildSource(source) { @@ -90,5 +89,12 @@ module.exports = error => { } } + if (typeof error.stack === 'string') { + retval.summary = error.stack.split('\n')[0]; + } else { + retval.summary = typeof error === 'function' ? + error.toString() : JSON.stringify(error); + } + return retval; }; diff --git a/test/extract-stack.js b/test/extract-stack.js deleted file mode 100644 index d0fc8612c..000000000 --- a/test/extract-stack.js +++ /dev/null @@ -1,46 +0,0 @@ -'use strict'; -const test = require('tap').test; -const extractStack = require('../lib/extract-stack'); - -test('strip error message', t => { - const stack = [ - 'error message', - 'Test.t (test.js:1:1)' - ].join('\n'); - - t.is(extractStack(stack), 'Test.t (test.js:1:1)'); - t.end(); -}); - -test('strip multiline error message', t => { - const stack = [ - 'error message', - 'with multiple', - 'lines', - '', - 'Test.t (test.js:1:1)' - ].join('\n'); - - t.is(extractStack(stack), 'Test.t (test.js:1:1)'); - t.end(); -}); - -test('strip beginning whitespace from stack', t => { - const stack = [ - 'error message', - ' Test.t (test.js:1:1)' - ].join('\n'); - - t.is(extractStack(stack), 'Test.t (test.js:1:1)'); - t.end(); -}); - -test('includes anonymous function lines', t => { - const stack = [ - 'error message', - 'path/to/test.js:1:1' - ].join('\n'); - - t.is(extractStack(stack), 'path/to/test.js:1:1'); - t.end(); -}); diff --git a/test/fixture/error-in-anonymous-function.js b/test/fixture/error-in-anonymous-function.js index db5f0d571..b78b2631b 100644 --- a/test/fixture/error-in-anonymous-function.js +++ b/test/fixture/error-in-anonymous-function.js @@ -1,9 +1,10 @@ import test from '../../'; const getAnonymousFn = () => () => { - throw Error(); + throw new Error(); }; test(t => { getAnonymousFn()(); + t.pass(); }); diff --git a/test/helper/error-from-worker.js b/test/helper/error-from-worker.js new file mode 100644 index 000000000..18c0be9b7 --- /dev/null +++ b/test/helper/error-from-worker.js @@ -0,0 +1,23 @@ +'use strict'; + +const serializeError = require('../../lib/serialize-error'); + +module.exports = function (err, options) { + options = Object.assign({}, options); + + if (options.stack) { + err.stack = options.stack; + } + + const serialized = serializeError(err); + + if (options.type) { + serialized.type = options.type; + } + + if (options.file) { + serialized.file = options.file; + } + + return serialized; +}; diff --git a/test/reporters/mini.js b/test/reporters/mini.js index 4daac4c01..19722a991 100644 --- a/test/reporters/mini.js +++ b/test/reporters/mini.js @@ -17,10 +17,11 @@ const test = require('tap').test; const cross = require('figures').cross; const lolex = require('lolex'); const AvaError = require('../../lib/ava-error'); +const AssertionError = require('../../lib/assert').AssertionError; const MiniReporter = require('../../lib/reporters/mini'); -const beautifyStack = require('../../lib/beautify-stack'); const colors = require('../helper/colors'); const compareLineOutput = require('../helper/compare-line-output'); +const errorFromWorker = require('../helper/error-from-worker'); const codeExcerpt = require('../../lib/code-excerpt'); const graySpinner = colors.dimGray(process.platform === 'win32' ? '-' : '⠋'); @@ -295,12 +296,11 @@ test('results with passing tests and rejections', t => { reporter.passCount = 1; reporter.rejectionCount = 1; - const err1 = new Error('failure one'); - err1.type = 'rejection'; - err1.stack = beautifyStack(err1.stack); - const err2 = new Error('failure two'); - err2.type = 'rejection'; - err2.stack = 'stack line with trailing whitespace\t\n'; + const err1 = errorFromWorker(new Error('failure one'), {type: 'rejection'}); + const err2 = errorFromWorker(new Error('failure two'), { + type: 'rejection', + stack: 'Error: failure two\n at trailingWhitespace (test.js:1:1)\r\n' + }); const runStatus = { errors: [err1, err2] @@ -313,12 +313,13 @@ test('results with passing tests and rejections', t => { ' ' + colors.red('1 rejection'), '', ' ' + colors.boldWhite('Unhandled Rejection'), - /Error: failure/, + /Error: failure one/, /test\/reporters\/mini\.js/, compareLineOutput.SKIP_UNTIL_EMPTY_LINE, '', ' ' + colors.boldWhite('Unhandled Rejection'), - ' ' + colors.red('stack line with trailing whitespace'), + /Error: failure two/, + /trailingWhitespace/, '' ]); t.end(); @@ -329,12 +330,9 @@ test('results with passing tests and exceptions', t => { reporter.passCount = 1; reporter.exceptionCount = 2; - const err = new Error('failure'); - err.type = 'exception'; - err.stack = beautifyStack(err.stack); + const err = errorFromWorker(new Error('failure'), {type: 'exception'}); - const avaErr = new AvaError('A futuristic test runner'); - avaErr.type = 'exception'; + const avaErr = errorFromWorker(new AvaError('A futuristic test runner'), {type: 'exception'}); const runStatus = { errors: [err, avaErr] @@ -358,33 +356,31 @@ test('results with passing tests and exceptions', t => { }); test('results with errors', t => { - const err1 = new Error('failure one'); - err1.stack = beautifyStack(err1.stack); + const err1 = errorFromWorker(new AssertionError({message: 'failure one'})); const err1Path = tempWrite.sync('a();'); err1.source = source(err1Path); - err1.avaAssertionError = true; err1.statements = []; err1.values = [ {label: 'actual:', formatted: JSON.stringify('abc') + '\n'}, {label: 'expected:', formatted: JSON.stringify('abd') + '\n'} ]; - const err2 = new Error('failure two'); - err2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const err2 = errorFromWorker(new AssertionError({message: 'failure two'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err2Path = tempWrite.sync('b();'); err2.source = source(err2Path); - err2.avaAssertionError = true; err2.statements = []; err2.values = [ {label: 'actual:', formatted: JSON.stringify([1]) + '\n'}, {label: 'expected:', formatted: JSON.stringify([2]) + '\n'} ]; - const err3 = new Error('failure three'); - err3.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const err3 = errorFromWorker(new AssertionError({message: 'failure three'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err3Path = tempWrite.sync('c();'); err3.source = source(err3Path); - err3.avaAssertionError = true; err3.statements = []; err3.values = [ {label: 'failure three:', formatted: JSON.stringify([1]) + '\n'} @@ -461,20 +457,19 @@ test('results with errors', t => { }); test('results with errors and disabled code excerpts', t => { - const err1 = new Error('failure one'); - err1.stack = beautifyStack(err1.stack); - err1.avaAssertionError = true; + const err1 = errorFromWorker(new AssertionError({message: 'failure one'})); + delete err1.source; err1.statements = []; err1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, {label: 'expected:', formatted: JSON.stringify('abd')} ]; - const err2 = new Error('failure two'); - err2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const err2 = errorFromWorker(new AssertionError({message: 'failure two'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err2Path = tempWrite.sync('b();'); err2.source = source(err2Path); - err2.avaAssertionError = true; err2.statements = []; err2.values = [ {label: 'actual:', formatted: JSON.stringify([1])}, @@ -535,22 +530,20 @@ test('results with errors and disabled code excerpts', t => { }); test('results with errors and broken code excerpts', t => { - const err1 = new Error('failure one'); - err1.stack = beautifyStack(err1.stack); + const err1 = errorFromWorker(new AssertionError({message: 'failure one'})); const err1Path = tempWrite.sync('a();'); err1.source = source(err1Path, 10); - err1.avaAssertionError = true; err1.statements = []; err1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, {label: 'expected:', formatted: JSON.stringify('abd')} ]; - const err2 = new Error('failure two'); - err2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const err2 = errorFromWorker(new AssertionError({message: 'failure two'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err2Path = tempWrite.sync('b();'); err2.source = source(err2Path); - err2.avaAssertionError = true; err2.statements = []; err2.values = [ {label: 'actual:', formatted: JSON.stringify([1])}, @@ -615,8 +608,8 @@ test('results with unhandled errors', t => { const reporter = miniReporter(); reporter.failCount = 2; - const err = new Error('failure one'); - err.stack = beautifyStack(err.stack); + const err = errorFromWorker(new Error('failure one')); + delete err.source; const runStatus = { errors: [ @@ -826,18 +819,18 @@ test('does not handle errors with body in rejections', t => { test('returns description based on error itself if no stack available', t => { const reporter = miniReporter(); reporter.exceptionCount = 1; - const err1 = new Error('failure one'); + const thrownValue = {message: 'failure one'}; + const err1 = errorFromWorker(thrownValue); const runStatus = { - errors: [{ - error: err1 - }] + errors: [err1] }; const actualOutput = reporter.finish(runStatus); + console.log(actualOutput); const expectedOutput = [ '\n ' + colors.red('1 exception'), '\n', '\n ' + colors.boldWhite('Uncaught Exception'), - '\n ' + colors.red('Threw non-error: ' + JSON.stringify({error: err1})), + '\n Threw non-error: ' + JSON.stringify(thrownValue), '\n' ].join(''); t.is(actualOutput, expectedOutput); @@ -847,7 +840,7 @@ test('returns description based on error itself if no stack available', t => { test('shows "non-error" hint for invalid throws', t => { const reporter = miniReporter(); reporter.exceptionCount = 1; - const err = {type: 'exception', message: 'function fooFn() {}', stack: 'function fooFn() {}'}; + const err = errorFromWorker({type: 'exception', message: 'function fooFn() {}', stack: 'function fooFn() {}'}); const runStatus = { errors: [err] }; @@ -856,7 +849,7 @@ test('shows "non-error" hint for invalid throws', t => { '\n ' + colors.red('1 exception'), '\n', '\n ' + colors.boldWhite('Uncaught Exception'), - '\n ' + colors.red('Threw non-error: function fooFn() {}'), + '\n Threw non-error: function fooFn() {}', '\n' ].join(''); t.is(actualOutput, expectedOutput); @@ -948,11 +941,9 @@ test('result when no-color flag is set', t => { }); test('results with errors and logs', t => { - const err1 = new Error('failure one'); - err1.stack = beautifyStack(err1.stack); + const err1 = errorFromWorker(new AssertionError({message: 'failure one'})); const err1Path = tempWrite.sync('a();'); err1.source = source(err1Path); - err1.avaAssertionError = true; err1.statements = []; err1.values = [ {label: 'actual:', formatted: JSON.stringify('abc') + '\n'}, diff --git a/test/reporters/tap.js b/test/reporters/tap.js index 6c973dfef..6305b91fc 100644 --- a/test/reporters/tap.js +++ b/test/reporters/tap.js @@ -40,7 +40,7 @@ test('failing test', t => { assertion: 'true', operator: '==', values: [{label: 'expected:', formatted: 'true'}, {label: 'actual:', formatted: 'false'}], - stack: ['', 'Test.fn (test.js:1:2)'].join('\n') + stack: 'Test.fn (test.js:1:2)' } }); @@ -113,7 +113,7 @@ test('unhandled error', t => { const actualOutput = reporter.unhandledError({ message: 'unhandled', name: 'TypeError', - stack: ['', 'Test.fn (test.js:1:2)'].join('\n') + stack: 'Test.fn (test.js:1:2)' }); const expectedOutput = `# unhandled diff --git a/test/reporters/verbose.js b/test/reporters/verbose.js index 3c3f8d0af..e369c8f74 100644 --- a/test/reporters/verbose.js +++ b/test/reporters/verbose.js @@ -15,10 +15,11 @@ const figures = require('figures'); const sinon = require('sinon'); const test = require('tap').test; const lolex = require('lolex'); -const beautifyStack = require('../../lib/beautify-stack'); +const AssertionError = require('../../lib/assert').AssertionError; const colors = require('../helper/colors'); const VerboseReporter = require('../../lib/reporters/verbose'); const compareLineOutput = require('../helper/compare-line-output'); +const errorFromWorker = require('../helper/error-from-worker'); const codeExcerpt = require('../../lib/code-excerpt'); const stackLineRegex = /.+ \(.+:[0-9]+:[0-9]+\)/; @@ -163,13 +164,12 @@ test('todo test', t => { test('uncaught exception', t => { const reporter = createReporter(); - const error = new Error('Unexpected token'); - - const output = reporter.unhandledError({ + const error = errorFromWorker(new Error('Unexpected token'), { type: 'exception', - file: 'test.js', - stack: beautifyStack(error.stack) - }, createRunStatus()).split('\n'); + file: 'test.js' + }); + + const output = reporter.unhandledError(error, createRunStatus()).split('\n'); t.is(output[0], colors.red('Uncaught Exception: test.js')); t.match(output[1], /Error: Unexpected token/); @@ -194,13 +194,12 @@ test('ava error', t => { test('unhandled rejection', t => { const reporter = createReporter(); - const error = new Error('Unexpected token'); - - const output = reporter.unhandledError({ - type: 'rejection', + const error = errorFromWorker(new Error('Unexpected token'), { file: 'test.js', - stack: beautifyStack(error.stack) - }, createRunStatus()).split('\n'); + type: 'rejection' + }); + + const output = reporter.unhandledError(error, createRunStatus()).split('\n'); t.is(output[0], colors.red('Unhandled Rejection: test.js')); t.match(output[1], /Error: Unexpected token/); @@ -211,11 +210,10 @@ test('unhandled rejection', t => { test('unhandled error without stack', t => { const reporter = createReporter(); - const err = { - type: 'exception', + const err = errorFromWorker({message: 'test'}, { file: 'test.js', - message: 'test' - }; + type: 'exception' + }); const output = reporter.unhandledError(err, createRunStatus()).split('\n'); @@ -358,33 +356,31 @@ test('results with passing tests, rejections and exceptions', t => { }); test('results with errors', t => { - const error1 = new Error('error one message'); - error1.stack = beautifyStack(error1.stack); + const error1 = errorFromWorker(new AssertionError({message: 'error one message'})); const err1Path = tempWrite.sync('a()'); error1.source = source(err1Path); - error1.avaAssertionError = true; error1.statements = []; error1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, {label: 'expected:', formatted: JSON.stringify('abd')} ]; - const error2 = new Error('error two message'); - error2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const error2 = errorFromWorker(new AssertionError({message: 'error two message'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err2Path = tempWrite.sync('b()'); error2.source = source(err2Path); - error2.avaAssertionError = true; error2.statements = []; error2.values = [ {label: 'actual:', formatted: JSON.stringify([1])}, {label: 'expected:', formatted: JSON.stringify([2])} ]; - const error3 = new Error('error three message'); - error3.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const error3 = errorFromWorker(new AssertionError({message: 'error three message'}), { + stack: 'error message\nTest.fn (test.js:1:1)' + }); const err3Path = tempWrite.sync('b()'); error3.source = source(err3Path); - error3.avaAssertionError = true; error3.statements = []; error3.values = [ {label: 'error three message:', formatted: JSON.stringify([1])} @@ -459,20 +455,19 @@ test('results with errors', t => { }); test('results with errors and disabled code excerpts', t => { - const error1 = new Error('error one message'); - error1.stack = beautifyStack(error1.stack); - error1.avaAssertionError = true; + const error1 = errorFromWorker(new AssertionError({message: 'error one message'})); + delete error1.source; error1.statements = []; error1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, {label: 'expected:', formatted: JSON.stringify('abd')} ]; - const error2 = new Error('error two message'); - error2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const error2 = errorFromWorker(new AssertionError({message: 'error two message'}), { + stack: 'error message\nTest.fn (test.js:1:1)\n' + }); const err2Path = tempWrite.sync('b()'); error2.source = source(err2Path); - error2.avaAssertionError = true; error2.statements = []; error2.values = [ {label: 'actual:', formatted: JSON.stringify([1])}, @@ -531,22 +526,20 @@ test('results with errors and disabled code excerpts', t => { }); test('results with errors and disabled code excerpts', t => { - const error1 = new Error('error one message'); - error1.stack = beautifyStack(error1.stack); + const error1 = errorFromWorker(new AssertionError({message: 'error one message'})); const err1Path = tempWrite.sync('a();'); error1.source = source(err1Path, 10); - error1.avaAssertionError = true; error1.statements = []; error1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, {label: 'expected:', formatted: JSON.stringify('abd')} ]; - const error2 = new Error('error two message'); - error2.stack = 'error message\nTest.fn (test.js:1:1)\n'; + const error2 = errorFromWorker(new AssertionError({message: 'error two message'}), { + stack: 'error message\nTest.fn (test.js:1:1)\n' + }); const err2Path = tempWrite.sync('b()'); error2.source = source(err2Path); - error2.avaAssertionError = true; error2.statements = []; error2.values = [ {label: 'actual:', formatted: JSON.stringify([1])}, @@ -878,11 +871,9 @@ test('failed test with logs', t => { }); test('results with errors and logs', t => { - const error1 = new Error('error one message'); - error1.stack = beautifyStack(error1.stack); + const error1 = errorFromWorker(new AssertionError({message: 'error one message'})); const err1Path = tempWrite.sync('a()'); error1.source = source(err1Path); - error1.avaAssertionError = true; error1.statements = []; error1.values = [ {label: 'actual:', formatted: JSON.stringify('abc')}, diff --git a/test/serialize-error.js b/test/serialize-error.js index ef117a12c..f3c8a8865 100644 --- a/test/serialize-error.js +++ b/test/serialize-error.js @@ -18,12 +18,13 @@ test('serialize standard props', t => { const err = new Error('Hello'); const serializedErr = serialize(err); - t.is(Object.keys(serializedErr).length, 6); + t.is(Object.keys(serializedErr).length, 7); t.is(serializedErr.avaAssertionError, false); t.deepEqual(serializedErr.object, {}); t.is(serializedErr.name, 'Error'); t.is(serializedErr.stack, beautifyStack(err.stack)); t.is(serializedErr.message, 'Hello'); + t.is(serializedErr.summary, 'Error: Hello'); t.is(typeof serializedErr.source.isDependency, 'boolean'); t.is(typeof serializedErr.source.isWithinProject, 'boolean'); t.is(typeof serializedErr.source.file, 'string'); From 91123461590dd5246e47136c56f2748e5b95d879 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Sun, 1 Oct 2017 11:00:53 -0400 Subject: [PATCH 4/4] Simplify reported value When the value of an uncaught exception is a function, AVA previously displayed the source of that function in the error report. During the review process for an unrelated change, this condition was found to be too rare to justify the maintenance overhead. Do not attempt to display the source of function values in these cases. --- lib/serialize-error.js | 3 +-- test/cli.js | 20 -------------------- test/fixture/throw-anonymous-function.js | 8 -------- test/fixture/throw-named-function.js | 10 ---------- 4 files changed, 1 insertion(+), 40 deletions(-) delete mode 100644 test/fixture/throw-anonymous-function.js delete mode 100644 test/fixture/throw-named-function.js diff --git a/lib/serialize-error.js b/lib/serialize-error.js index aebc5254d..13146ff42 100644 --- a/lib/serialize-error.js +++ b/lib/serialize-error.js @@ -92,8 +92,7 @@ module.exports = error => { if (typeof error.stack === 'string') { retval.summary = error.stack.split('\n')[0]; } else { - retval.summary = typeof error === 'function' ? - error.toString() : JSON.stringify(error); + retval.summary = JSON.stringify(error); } return retval; diff --git a/test/cli.js b/test/cli.js index 35ff3d83a..c598b50e6 100644 --- a/test/cli.js +++ b/test/cli.js @@ -91,16 +91,6 @@ test('timeout', t => { }); }); -test('throwing a named function will report the to the console', t => { - execCli('fixture/throw-named-function.js', (err, stdout, stderr) => { - t.ok(err); - t.match(stderr, /function fooFn\(\) \{\}/); - // TODO(jamestalmage) - // t.ok(/1 uncaught exception[^s]/.test(stdout)); - t.end(); - }); -}); - test('include anonymous functions in error reports', t => { execCli('fixture/error-in-anonymous-function.js', (err, stdout, stderr) => { t.ok(err); @@ -210,16 +200,6 @@ test('babel require hook only does not apply to source files', t => { }); }); -test('throwing a anonymous function will report the function to the console', t => { - execCli('fixture/throw-anonymous-function.js', (err, stdout, stderr) => { - t.ok(err); - t.match(stderr, /\(\) => \{\}/); - // TODO(jamestalmage) - // t.ok(/1 uncaught exception[^s]/.test(stdout)); - t.end(); - }); -}); - test('pkg-conf: defaults', t => { execCli([], {dirname: 'fixture/pkg-conf/defaults'}, err => { t.ifError(err); diff --git a/test/fixture/throw-anonymous-function.js b/test/fixture/throw-anonymous-function.js deleted file mode 100644 index c2ba78206..000000000 --- a/test/fixture/throw-anonymous-function.js +++ /dev/null @@ -1,8 +0,0 @@ -import test from '../../'; - -test('throw an uncaught exception', t => { - setImmediate(() => { - throw () => {}; // eslint-disable-line no-throw-literal - }); - t.pass(); -}); diff --git a/test/fixture/throw-named-function.js b/test/fixture/throw-named-function.js deleted file mode 100644 index 4e4b79b16..000000000 --- a/test/fixture/throw-named-function.js +++ /dev/null @@ -1,10 +0,0 @@ -import test from '../../'; - -function fooFn() {} - -test('throw an uncaught exception', t => { - setImmediate(() => { - throw fooFn; - }); - t.pass(); -});