Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve syntax error reporting; revert babel-runtime change. #787

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Improved responsiveness of the system while using `--watch`.
* Fixed `jest -o` issue when no files were changed.
* Improved code coverage reporting when using `babel-jest`.
* Improved error reporting when a syntax error occurs.

## 0.9.1

Expand Down
1 change: 0 additions & 1 deletion packages/babel-jest/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"main": "src/index.js",
"dependencies": {
"babel-core": "^6.0.0",
"babel-plugin-transform-runtime": "^6.0.0",
"babel-preset-jest": "^1.0.0"
}
}
3 changes: 1 addition & 2 deletions packages/babel-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@ module.exports = {
process(src, filename) {
if (babel.util.canCompile(filename)) {
return babel.transform(src, {
auxiliaryCommentBefore: 'istanbul ignore next',
filename,
presets: [jestPreset],
auxiliaryCommentBefore: 'istanbul ignore next',
plugins: ['transform-runtime'],
retainLines: true,
}).code;
}
Expand Down
35 changes: 32 additions & 3 deletions src/HasteModuleLoader/HasteModuleLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,7 @@ class Loader {
moduleObj.parent = mockParentModule;
moduleObj.require = this._createRequireImplementation(filename);

const evalResultVariable = 'Object.<anonymous>';
const wrapper = '({ "' + evalResultVariable + '": function(module, exports, require, __dirname, __filename, global, jest, $JEST_COVERAGE_DATA) {' + moduleContent + '\n}});';
const wrapperFunc = this._environment.runSourceText(wrapper, filename)[evalResultVariable];
const wrapperFunc = this._runSourceText(moduleContent, filename);
wrapperFunc.call(
moduleObj.exports, // module context
moduleObj, // module object
Expand All @@ -321,6 +319,37 @@ class Loader {
this._currentlyExecutingModulePath = lastExecutingModulePath;
}

_runSourceText(moduleContent, filename) {
const config = this._config;
const relative = filePath => path.relative(config.rootDir, filePath);
const env = this._environment;
const evalResultVariable = 'Object.<anonymous>';
const wrapper = '({ "' + evalResultVariable + '": function(module, exports, require, __dirname, __filename, global, jest, $JEST_COVERAGE_DATA) {' + moduleContent + '\n}});';
try {
return env.runSourceText(wrapper, filename)[evalResultVariable];
} catch (e) {
if (e.constructor.name === 'SyntaxError') {
const hasPreprocessor = config.scriptPreprocessor;
const preprocessorInfo = hasPreprocessor
? relative(config.scriptPreprocessor)
: `No preprocessor specified, consider installing 'babel-jest'`;
const babelInfo = config.usesBabelJest
? `Make sure your '.babelrc' is set up correctly, ` +
`for example it should include the 'es2015' preset.\n`
: '';
throw new SyntaxError(
`${e.message} in file '${relative(filename)}'.\n\n` +
`Make sure your preprocessor is set up correctly and ensure ` +
`your 'preprocessorIgnorePatterns' configuration is correct: http://facebook.github.io/jest/docs/api.html#config-preprocessorignorepatterns-array-string\n` +
'If you are currently setting up Jest or modifying your preprocessor, try `jest --no-cache`.\n' +
`Preprocessor: ${preprocessorInfo}.\n${babelInfo}` +
`Jest tried to the execute the following ${hasPreprocessor ? 'preprocessed ' : ''}code:\n${moduleContent}\n`
);
}
throw e;
}
}

_generateMock(currPath, moduleName) {
const modulePath = this._resolveModuleName(currPath, moduleName);

Expand Down
92 changes: 42 additions & 50 deletions src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,75 +319,67 @@ function cleanStackTrace(stackTrace) {
const keepFirstLines = () => (lines++ < KEEP_TRACE_LINES);
return stackTrace.split('\n').filter(line => (
keepFirstLines() ||
!/jest(-cli)?\/(vendor|src|node_modules)\//.test(line)
!/^\s+at.*?jest(-cli)?\/(vendor|src|node_modules)\//.test(line)
)).join('\n');
}

/**
* Given a test result, return a human readable string representing the
* failures.
*
* @param {Object} testResult
* @param {Object} config Containing the following keys:
* `rootPath` - Root directory (for making stack trace paths relative).
* `useColor` - True if message should include color flags.
* @return {String}
*/
function formatFailureMessage(testResult, config) {
const rootPath = config.rootPath;
const useColor = config.useColor;
const rootDir = config.rootDir;

const localChalk = new chalk.constructor({enabled: !!useColor});
const ancestrySeparator = ' \u203A ';
const descBullet = localChalk.bold('\u25cf ');
const descBullet = config.verbose ? '' : chalk.bold('\u25cf ');
const msgBullet = ' - ';
const msgIndent = msgBullet.replace(/./g, ' ');

if (testResult.testExecError) {
const error = testResult.testExecError;
return (
descBullet + localChalk.bold('Runtime Error') + '\n' +
descBullet +
(config.verbose ? 'Runtime Error' : chalk.bold('Runtime Error')) + '\n' +
(error.stack ? cleanStackTrace(error.stack) : error.message)
);
}

return testResult.testResults.filter(function(result) {
return result.failureMessages.length !== 0;
}).map(function(result) {
const failureMessages = result.failureMessages.map(function(errorMsg) {
errorMsg = errorMsg.split('\n').map(function(line) {
// Extract the file path from the trace line.
let matches = line.match(/(^\s+at .*?\()([^()]+)(:[0-9]+:[0-9]+\).*$)/);
if (!matches) {
matches = line.match(/(^\s+at )([^()]+)(:[0-9]+:[0-9]+.*$)/);
if (!matches) {
return line;
}
}
var filePath = matches[2];
// Filter out noisy and unhelpful lines from the stack trace.
if (STACK_TRACE_LINE_IGNORE_RE.test(filePath)) {
return null;
}
return (
matches[1] +
path.relative(rootPath, filePath) +
matches[3]
);
}).filter(function(line) {
return line !== null;
return testResult.testResults
.filter(result => result.failureMessages.length !== 0)
.map(result => {
const failureMessages = result.failureMessages.map(errorMsg => {
errorMsg = errorMsg.split('\n')
.map(line => {
// Extract the file path from the trace line.
let matches =
line.match(/(^\s+at .*?\()([^()]+)(:[0-9]+:[0-9]+\).*$)/);
if (!matches) {
matches = line.match(/(^\s+at )([^()]+)(:[0-9]+:[0-9]+.*$)/);
if (!matches) {
return line;
}
}
var filePath = matches[2];
// Filter out noisy and unhelpful lines from the stack trace.
if (STACK_TRACE_LINE_IGNORE_RE.test(filePath)) {
return null;
}
return (
matches[1] +
path.relative(rootDir, filePath) +
matches[3]
);
})
.filter(line => line !== null)
.join('\n');

return msgBullet + errorMsg.replace(/\n/g, '\n' + msgIndent);
}).join('\n');

return msgBullet + errorMsg.replace(/\n/g, '\n' + msgIndent);
}).join('\n');

const testTitleAncestry = result.ancestorTitles.map(function(title) {
return localChalk.bold(title);
}).join(ancestrySeparator) + ancestrySeparator;
const testTitleAncestry = result.ancestorTitles.map(
title => chalk.bold(title)
).join(ancestrySeparator) + ancestrySeparator;

return descBullet + testTitleAncestry + result.title + '\n' +
failureMessages;
}).join('\n');
return descBullet + testTitleAncestry + result.title + '\n' +
failureMessages;
})
.join('\n');
}

function deepCopy(obj) {
Expand Down
35 changes: 12 additions & 23 deletions src/reporters/DefaultTestReporter.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,34 +73,23 @@ class DefaultTestReporter {
`${allTestsPassed ? PASS : FAIL} ${TEST_NAME_COLOR(pathStr)}` +
(testDetail.length ? ` (${testDetail.join(', ')})` : '');

/*
if (config.collectCoverage) {
// TODO: Find a nice pretty way to print this out
}
*/

this.log(resultHeader);
if (config.verbose) {
this.verboseLogger.logTestResults(testResult.testResults);
if (config.verbose && !testResult.testExecError) {
this.verboseLogger.logTestResults(
testResult.testResults
);
}

if (!allTestsPassed) {
const failureMessage = formatFailureMessage(testResult, {
rootPath: config.rootDir,
useColor: !config.noHighlight,
});
if (config.verbose) {
results.postSuiteHeaders.push(resultHeader, failureMessage);
} else {
// If we write more than one character at a time it is possible that
// node exits in the middle of printing the result.
// If you are reading this and you are from the future, this might not
// be true any more.
for (let i = 0; i < failureMessage.length; i++) {
this._process.stdout.write(failureMessage.charAt(i));
}
this._process.stdout.write('\n');
const failureMessage = formatFailureMessage(testResult, config);
// If we write more than one character at a time it is possible that
// node exits in the middle of printing the result.
// If you are reading this and you are from the future, this might not
// be true any more.
for (let i = 0; i < failureMessage.length; i++) {
this._process.stdout.write(failureMessage.charAt(i));
}
this._process.stdout.write('\n');

if (config.bail) {
this.onRunComplete(config, results);
Expand Down
2 changes: 0 additions & 2 deletions src/resolvers/HasteResolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
* of patent rights can be found in the PATENTS file in the same directory.
*/

/* eslint-disable fb-www/object-create-only-one-param */

'use strict';

const DependencyGraph = require('node-haste');
Expand Down