From 81d3533447e97c685cf22b848cb65a41b5b3b831 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 11:55:05 +0200 Subject: [PATCH 1/7] Always remove node internals from stacktraces --- CHANGELOG.md | 3 +- integration_tests/utils.js | 10 +---- packages/jest-message-util/package.json | 3 +- .../__snapshots__/messages.test.js.snap | 17 ++++++++ .../src/__tests__/messages.test.js | 43 ++++++++++++++++++- packages/jest-message-util/src/index.js | 38 ++++++++++++++-- yarn.lock | 4 ++ 7 files changed, 103 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52de836d914e..8c998aa59ca0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,9 @@ * `[jest-editor-support]` Fix editor support test for node 4 ([#4640](https://github.com/facebook/jest/pull/4640)) * `[jest-mock]` Support mocking constructor in `mockImplementationOnce` ([#4599](https://github.com/facebook/jest/pull/4599)) * `[jest-runtime]` Fix manual user mocks not working with custom resolver ([#4489](https://github.com/facebook/jest/pull/4489)) -* `[jest-runtime]` Move `babel-core` to peer dependenies so it works with Babel 7 ([#4557](https://github.com/facebook/jest/pull/4557)) +* `[jest-runtime]` Move `babel-core` to peer dependencies so it works with Babel 7 ([#4557](https://github.com/facebook/jest/pull/4557)) * `[jest-util]` Fix `runOnlyPendingTimers` for `setTimeout` inside `setImmediate` ([#4608](https://github.com/facebook/jest/pull/4608)) +* `[jest-message-util]` Always remove node internals from stacktraces (TBD) ### Features * `[jest-environment-*]` [**BREAKING**] Add Async Test Environment APIs, dispose is now teardown ([#4506](https://github.com/facebook/jest/pull/4506)) diff --git a/integration_tests/utils.js b/integration_tests/utils.js index 04d1dec293bc..e90259f6949b 100644 --- a/integration_tests/utils.js +++ b/integration_tests/utils.js @@ -157,15 +157,7 @@ const extractSummary = (stdout: string) => { // different versions of Node print different stack traces. This function // unifies their output to make it possible to snapshot them. const cleanupStackTrace = (output: string) => { - return output - .replace(/\n.*at.*timers\.js.*$/gm, '') - .replace(/\n.*at.*assert\.js.*$/gm, '') - .replace(/\n.*at.*node\.js.*$/gm, '') - .replace(/\n.*at.*next_tick\.js.*$/gm, '') - .replace(/\n.*at (new )?Promise \(\).*$/gm, '') - .replace(/\n.*at .*$/gm, '') - .replace(/\n.*at Generator.next \(\).*$/gm, '') - .replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); + return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); }; module.exports = { diff --git a/packages/jest-message-util/package.json b/packages/jest-message-util/package.json index a69491dffe79..ff5a7c7018ab 100644 --- a/packages/jest-message-util/package.json +++ b/packages/jest-message-util/package.json @@ -10,6 +10,7 @@ "dependencies": { "chalk": "^2.0.1", "micromatch": "^2.3.11", - "slash": "^1.0.0" + "slash": "^1.0.0", + "stack-utils": "^1.0.1" } } diff --git a/packages/jest-message-util/src/__tests__/__snapshots__/messages.test.js.snap b/packages/jest-message-util/src/__tests__/__snapshots__/messages.test.js.snap index ddcbfd37f4fe..b4ebdf847fe3 100644 --- a/packages/jest-message-util/src/__tests__/__snapshots__/messages.test.js.snap +++ b/packages/jest-message-util/src/__tests__/__snapshots__/messages.test.js.snap @@ -7,6 +7,23 @@ exports[`.formatExecError() 1`] = ` " `; +exports[`formatStackTrace should strip node internals 1`] = ` +" ● Unix test + + + Expected value to be of type: + \\"number\\" + Received: + \\"\\" + type: + \\"string\\" + + + at Object.it (__tests__/test.js:8:14) + +" +`; + exports[`should exclude jasmine from stack trace for Unix paths. 1`] = ` " ● Unix test diff --git a/packages/jest-message-util/src/__tests__/messages.test.js b/packages/jest-message-util/src/__tests__/messages.test.js index d70f3573707d..98ab8beab662 100644 --- a/packages/jest-message-util/src/__tests__/messages.test.js +++ b/packages/jest-message-util/src/__tests__/messages.test.js @@ -8,7 +8,7 @@ 'use strict'; -const {formatResultsErrors, formatExecError} = require('../'); +const {formatResultsErrors, formatExecError} = require('..'); const unixStackTrace = ` ` + @@ -23,6 +23,27 @@ const unixStackTrace = at Object. (../jest-jasmine2/build/jasmine-pit.js:35:32) at attemptAsync (../jest-jasmine2/build/jasmine-2.4.1.js:1919:24)`; +const assertionStack = + ' ' + + ` + Expected value to be of type: + "number" + Received: + "" + type: + "string" + + at Object.it (__tests__/test.js:8:14) + at Object.asyncFn (node_modules/jest-jasmine2/build/jasmine_async.js:124:345) + at resolve (node_modules/jest-jasmine2/build/queue_runner.js:46:12) + at Promise () + at mapper (node_modules/jest-jasmine2/build/queue_runner.js:34:499) + at promise.then (node_modules/jest-jasmine2/build/queue_runner.js:74:39) + at + at process._tickCallback (internal/process/next_tick.js:188:7) + at internal/process/next_tick.js:188:7 +`; + it('should exclude jasmine from stack trace for Unix paths.', () => { const messages = formatResultsErrors( [ @@ -62,3 +83,23 @@ it('.formatExecError()', () => { expect(message).toMatchSnapshot(); }); + +it('formatStackTrace should strip node internals', () => { + const messages = formatResultsErrors( + [ + { + ancestorTitles: [], + failureMessages: [assertionStack], + title: 'Unix test', + }, + ], + { + rootDir: '', + }, + { + noStackTrace: false, + }, + ); + + expect(messages).toMatchSnapshot(); +}); diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index 6fa2df62b475..1569e2778ac4 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -14,6 +14,11 @@ import path from 'path'; import chalk from 'chalk'; import micromatch from 'micromatch'; import slash from 'slash'; +import StackUtils from 'stack-utils'; + +const nodeInternals = StackUtils.nodeInternals() + // Somehow we get a trace without the `process._tickCallback` part + .concat(new RegExp('internal/process/next_tick.js')); type StackTraceConfig = { rootDir: string, @@ -27,6 +32,9 @@ type StackTraceOptions = { // filter for noisy stack trace lines const JASMINE_IGNORE = /^\s+at(?:(?:.*?vendor\/|jasmine\-)|\s+jasmine\.buildExpectationResult)/; const STACK_TRACE_IGNORE = /^\s+at.*?jest(-.*?)?(\/|\\)(build|node_modules|packages)(\/|\\)/; +const ANONYMOUS_TRACE_IGNORE = /^\s+at .*$/; +const ANONYMOUS_PROMISE_IGNORE = /^\s+at (new )?Promise \(\).*$/; +const ANONYMOUS_GENERATOR_IGNORE = /^\s+at Generator.next \(\).*$/; const TITLE_INDENT = ' '; const MESSAGE_INDENT = ' '; const STACK_INDENT = ' '; @@ -106,10 +114,26 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => { let pathCounter = 0; return lines.filter(line => { - const isPath = STACK_PATH_REGEXP.test(line); - if (!isPath) { + if (ANONYMOUS_TRACE_IGNORE.test(line)) { + return false; + } + + if (ANONYMOUS_PROMISE_IGNORE.test(line)) { + return false; + } + + if (ANONYMOUS_GENERATOR_IGNORE.test(line)) { + return false; + } + + if (nodeInternals.some(internal => internal.test(line))) { + return false; + } + + if (!STACK_PATH_REGEXP.test(line)) { return true; } + if (JASMINE_IGNORE.test(line)) { return false; } @@ -118,7 +142,15 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => { return true; // always keep the first line even if it's from Jest } - return !(STACK_TRACE_IGNORE.test(line) || options.noStackTrace); + if (options.noStackTrace) { + return false; + } + + if (STACK_TRACE_IGNORE.test(line)) { + return false; + } + + return true; }); }; diff --git a/yarn.lock b/yarn.lock index 0e6ed826f959..21128d7a623e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5444,6 +5444,10 @@ sshpk@^1.7.0: jsbn "~0.1.0" tweetnacl "~0.14.0" +stack-utils@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/stack-utils/-/stack-utils-1.0.1.tgz#d4f33ab54e8e38778b0ca5cfd3b3afb12db68620" + state-toggle@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/state-toggle/-/state-toggle-1.0.0.tgz#d20f9a616bb4f0c3b98b91922d25b640aa2bc425" From 1596e89a2a55115bb361263dd2812e04a7f2c17c Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 11:56:03 +0200 Subject: [PATCH 2/7] Update link to PR in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c998aa59ca0..da217d7ac122 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ * `[jest-runtime]` Fix manual user mocks not working with custom resolver ([#4489](https://github.com/facebook/jest/pull/4489)) * `[jest-runtime]` Move `babel-core` to peer dependencies so it works with Babel 7 ([#4557](https://github.com/facebook/jest/pull/4557)) * `[jest-util]` Fix `runOnlyPendingTimers` for `setTimeout` inside `setImmediate` ([#4608](https://github.com/facebook/jest/pull/4608)) -* `[jest-message-util]` Always remove node internals from stacktraces (TBD) +* `[jest-message-util]` Always remove node internals from stacktraces ([#4695](https://github.com/facebook/jest/pull/4695)) ### Features * `[jest-environment-*]` [**BREAKING**] Add Async Test Environment APIs, dispose is now teardown ([#4506](https://github.com/facebook/jest/pull/4506)) From 4472f9b47c8fb4d6bc4332f3e2a00d4ca9646c20 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 12:08:08 +0200 Subject: [PATCH 3/7] Fix browser test --- packages/jest-message-util/src/index.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index 1569e2778ac4..f5ed0b58a925 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -16,9 +16,16 @@ import micromatch from 'micromatch'; import slash from 'slash'; import StackUtils from 'stack-utils'; -const nodeInternals = StackUtils.nodeInternals() - // Somehow we get a trace without the `process._tickCallback` part - .concat(new RegExp('internal/process/next_tick.js')); +let nodeInternals = []; + +try { + nodeInternals = StackUtils.nodeInternals() + // Somehow we get a trace without the `process._tickCallback` part + .concat(new RegExp('internal/process/next_tick.js')); +} catch (e) { + // `StackUtils.nodeInternals()` fails in browsers. We don't need to remove + // node internals in the browser though, so no issue. +} type StackTraceConfig = { rootDir: string, From f097ecebd8a71d72df37a6f0063a72631b7af92a Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 12:15:49 +0200 Subject: [PATCH 4/7] Remove weird next_tick trace just in integration tests --- integration_tests/utils.js | 5 +++-- packages/jest-message-util/src/__tests__/messages.test.js | 1 - packages/jest-message-util/src/index.js | 4 +--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/integration_tests/utils.js b/integration_tests/utils.js index e90259f6949b..e73faf55babd 100644 --- a/integration_tests/utils.js +++ b/integration_tests/utils.js @@ -157,12 +157,13 @@ const extractSummary = (stdout: string) => { // different versions of Node print different stack traces. This function // unifies their output to make it possible to snapshot them. const cleanupStackTrace = (output: string) => { - return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); + return output + .replace(/\n.*at.*next_tick\.js.*$/gm, '') + .replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); }; module.exports = { cleanup, - cleanupStackTrace, copyDir, createEmptyPackage, extractSummary, diff --git a/packages/jest-message-util/src/__tests__/messages.test.js b/packages/jest-message-util/src/__tests__/messages.test.js index 98ab8beab662..458a30ff23be 100644 --- a/packages/jest-message-util/src/__tests__/messages.test.js +++ b/packages/jest-message-util/src/__tests__/messages.test.js @@ -41,7 +41,6 @@ const assertionStack = at promise.then (node_modules/jest-jasmine2/build/queue_runner.js:74:39) at at process._tickCallback (internal/process/next_tick.js:188:7) - at internal/process/next_tick.js:188:7 `; it('should exclude jasmine from stack trace for Unix paths.', () => { diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index f5ed0b58a925..5eac740f8230 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -19,9 +19,7 @@ import StackUtils from 'stack-utils'; let nodeInternals = []; try { - nodeInternals = StackUtils.nodeInternals() - // Somehow we get a trace without the `process._tickCallback` part - .concat(new RegExp('internal/process/next_tick.js')); + nodeInternals = StackUtils.nodeInternals(); } catch (e) { // `StackUtils.nodeInternals()` fails in browsers. We don't need to remove // node internals in the browser though, so no issue. From 3c294ee62debf7515f0bac8b09a80fc5e56bcac7 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 13:03:06 +0200 Subject: [PATCH 5/7] Rename some regexpes --- packages/jest-message-util/src/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index 5eac740f8230..41749728f380 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -36,8 +36,8 @@ type StackTraceOptions = { // filter for noisy stack trace lines const JASMINE_IGNORE = /^\s+at(?:(?:.*?vendor\/|jasmine\-)|\s+jasmine\.buildExpectationResult)/; -const STACK_TRACE_IGNORE = /^\s+at.*?jest(-.*?)?(\/|\\)(build|node_modules|packages)(\/|\\)/; -const ANONYMOUS_TRACE_IGNORE = /^\s+at .*$/; +const JEST_INTERNALS_IGNORE = /^\s+at.*?jest(-.*?)?(\/|\\)(build|node_modules|packages)(\/|\\)/; +const ANONYMOUS_FN_IGNORE = /^\s+at .*$/; const ANONYMOUS_PROMISE_IGNORE = /^\s+at (new )?Promise \(\).*$/; const ANONYMOUS_GENERATOR_IGNORE = /^\s+at Generator.next \(\).*$/; const TITLE_INDENT = ' '; @@ -119,7 +119,7 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => { let pathCounter = 0; return lines.filter(line => { - if (ANONYMOUS_TRACE_IGNORE.test(line)) { + if (ANONYMOUS_FN_IGNORE.test(line)) { return false; } @@ -151,7 +151,7 @@ const removeInternalStackEntries = (lines, options: StackTraceOptions) => { return false; } - if (STACK_TRACE_IGNORE.test(line)) { + if (JEST_INTERNALS_IGNORE.test(line)) { return false; } From b4bfc534bac5674fe1e46b2828e7c7c0014672dd Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 13:09:12 +0200 Subject: [PATCH 6/7] Revert "Remove weird next_tick trace just in integration tests" This reverts commit f097ecebd8a71d72df37a6f0063a72631b7af92a. --- integration_tests/utils.js | 5 ++--- packages/jest-message-util/src/__tests__/messages.test.js | 1 + packages/jest-message-util/src/index.js | 4 +++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/integration_tests/utils.js b/integration_tests/utils.js index e73faf55babd..e90259f6949b 100644 --- a/integration_tests/utils.js +++ b/integration_tests/utils.js @@ -157,13 +157,12 @@ const extractSummary = (stdout: string) => { // different versions of Node print different stack traces. This function // unifies their output to make it possible to snapshot them. const cleanupStackTrace = (output: string) => { - return output - .replace(/\n.*at.*next_tick\.js.*$/gm, '') - .replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); + return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); }; module.exports = { cleanup, + cleanupStackTrace, copyDir, createEmptyPackage, extractSummary, diff --git a/packages/jest-message-util/src/__tests__/messages.test.js b/packages/jest-message-util/src/__tests__/messages.test.js index 458a30ff23be..98ab8beab662 100644 --- a/packages/jest-message-util/src/__tests__/messages.test.js +++ b/packages/jest-message-util/src/__tests__/messages.test.js @@ -41,6 +41,7 @@ const assertionStack = at promise.then (node_modules/jest-jasmine2/build/queue_runner.js:74:39) at at process._tickCallback (internal/process/next_tick.js:188:7) + at internal/process/next_tick.js:188:7 `; it('should exclude jasmine from stack trace for Unix paths.', () => { diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index 41749728f380..7505364a241b 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -19,7 +19,9 @@ import StackUtils from 'stack-utils'; let nodeInternals = []; try { - nodeInternals = StackUtils.nodeInternals(); + nodeInternals = StackUtils.nodeInternals() + // Somehow we get a trace without the `process._tickCallback` part + .concat(new RegExp('internal/process/next_tick.js')); } catch (e) { // `StackUtils.nodeInternals()` fails in browsers. We don't need to remove // node internals in the browser though, so no issue. From c68e6705a5dd44386756f2034afbc93819a425e3 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 14 Oct 2017 13:11:21 +0200 Subject: [PATCH 7/7] Note code that can be delete when we drop support fo node 4 --- integration_tests/utils.js | 1 + packages/jest-message-util/src/index.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/integration_tests/utils.js b/integration_tests/utils.js index e90259f6949b..87fa132ed1d0 100644 --- a/integration_tests/utils.js +++ b/integration_tests/utils.js @@ -156,6 +156,7 @@ const extractSummary = (stdout: string) => { // different versions of Node print different stack traces. This function // unifies their output to make it possible to snapshot them. +// TODO: Remove when we drop support for node 4 const cleanupStackTrace = (output: string) => { return output.replace(/^.*at.*[\s][\(]?(\S*\:\d*\:\d*).*$/gm, ' at $1'); }; diff --git a/packages/jest-message-util/src/index.js b/packages/jest-message-util/src/index.js index 7505364a241b..3ed544fdffbb 100644 --- a/packages/jest-message-util/src/index.js +++ b/packages/jest-message-util/src/index.js @@ -20,7 +20,8 @@ let nodeInternals = []; try { nodeInternals = StackUtils.nodeInternals() - // Somehow we get a trace without the `process._tickCallback` part + // this is to have the tests be the same in node 4 and node 6. + // TODO: Remove when we drop support for node 4 .concat(new RegExp('internal/process/next_tick.js')); } catch (e) { // `StackUtils.nodeInternals()` fails in browsers. We don't need to remove