Code excerpt fixes (#1271)

* Resolve absolute source file path when serializing errors

Fixes #1270.

* Bail excerpting code if file cannot be read

* temp-write@^3.1.0

* Restrict code excerpts to tests/helpers/sources

Don't show code excerpts for dependencies or code outside the project
directory. Determine where the source lives when serializing the error.
Then, when excerpting the code, bail if it lives outside the project
or is a dependency.

lib/cli.js

@@ -129,9 +129,9 @@ exports.run = () => {
 	if (cli.flags.tap && !cli.flags.watch) {
 		reporter = new TapReporter();
 	} else if (cli.flags.verbose || isCi) {
-		reporter = new VerboseReporter({color: cli.flags.color, basePath: projectDir});
+		reporter = new VerboseReporter({color: cli.flags.color});
 	} else {
-		reporter = new MiniReporter({color: cli.flags.color, watching: cli.flags.watch, basePath: projectDir});
+		reporter = new MiniReporter({color: cli.flags.color, watching: cli.flags.watch});
 	}
 
 	reporter.api = api;

lib/code-excerpt.js

@@ -8,12 +8,25 @@ const chalk = require('chalk');
 const formatLineNumber = (lineNumber, maxLineNumber) =>
 	' '.repeat(Math.max(0, String(maxLineNumber).length - String(lineNumber).length)) + lineNumber;
 
-module.exports = (file, line, options) => {
-	options = options || {};
+module.exports = (source, options) => {
+	if (!source.isWithinProject || source.isDependency) {
+		return null;
+	}
 
+	const file = source.file;
+	const line = source.line;
+
+	options = options || {};
 	const maxWidth = options.maxWidth || 80;
-	const source = fs.readFileSync(file, 'utf8');
-	const excerpt = codeExcerpt(source, line, {around: 1});
+
+	let contents;
+	try {
+		contents = fs.readFileSync(file, 'utf8');
+	} catch (err) {
+		return null;
+	}
+
+	const excerpt = codeExcerpt(contents, line, {around: 1});
 	if (!excerpt) {
 		return null;
 	}

lib/reporters/mini.js

@@ -1,6 +1,5 @@
 'use strict';
 const StringDecoder = require('string_decoder').StringDecoder;
-const path = require('path');
 const cliCursor = require('cli-cursor');
 const lastLineTracker = require('last-line-stream/tracker');
 const plur = require('plur');
@@ -187,8 +186,7 @@ class MiniReporter {
 				if (test.error.source) {
 					status += '  ' + colors.errorSource(test.error.source.file + ':' + test.error.source.line) + '\n';
 
-					const errorPath = path.join(this.options.basePath, test.error.source.file);
-					const excerpt = codeExcerpt(errorPath, test.error.source.line, {maxWidth: process.stdout.columns});
+					const excerpt = codeExcerpt(test.error.source, {maxWidth: process.stdout.columns});
 					if (excerpt) {
 						status += '\n' + indentString(excerpt, 2) + '\n';
 					}

lib/reporters/verbose.js

@@ -1,5 +1,4 @@
 'use strict';
-const path = require('path');
 const indentString = require('indent-string');
 const prettyMs = require('pretty-ms');
 const figures = require('figures');
@@ -106,8 +105,7 @@ class VerboseReporter {
 				if (test.error.source) {
 					output += '  ' + colors.errorSource(test.error.source.file + ':' + test.error.source.line) + '\n';
 
-					const errorPath = path.join(this.options.basePath, test.error.source.file);
-					const excerpt = codeExcerpt(errorPath, test.error.source.line, {maxWidth: process.stdout.columns});
+					const excerpt = codeExcerpt(test.error.source, {maxWidth: process.stdout.columns});
 					if (excerpt) {
 						output += '\n' + indentString(excerpt, 2) + '\n';
 					}

lib/serialize-error.js

@@ -1,4 +1,5 @@
 'use strict';
+const path = require('path');
 const cleanYamlObject = require('clean-yaml-object');
 const StackUtils = require('stack-utils');
 const prettyFormat = require('@ava/pretty-format');
@@ -58,8 +59,21 @@ module.exports = error => {
 		const firstStackLine = extractStack(err.stack).split('\n')[0];
 		const source = stackUtils.parseLine(firstStackLine);
 		if (source) {
+			// Assume the CWD is the project directory. This holds since this function
+			// is only called in test workers, which are created with their working
+			// directory set to the project directory.
+			const projectDir = process.cwd();
+
+			const file = path.resolve(projectDir, source.file.trim());
+			const rel = path.relative(projectDir, file);
+
+			const isWithinProject = rel.split(path.sep)[0] !== '..';
+			const isDependency = isWithinProject && path.dirname(rel).split(path.sep).indexOf('node_modules') > -1;
+
 			err.source = {
-				file: source.file.trim(),
+				isDependency,
+				isWithinProject,
+				file,
 				line: source.line
 			};
 		}

package.json

@@ -183,7 +183,7 @@
     "sinon": "^1.17.2",
     "source-map-fixtures": "^2.1.0",
     "tap": "^10.0.0",
-    "temp-write": "^3.0.0",
+    "temp-write": "^3.1.0",
     "touch": "^1.0.0",
     "xo": "^0.17.0",
     "zen-observable": "^0.4.0"

test/code-excerpt.js

@@ -1,4 +1,5 @@
 'use strict';
+const fs = require('fs');
 const tempWrite = require('temp-write');
 const chalk = require('chalk');
 const test = require('tap').test;
@@ -7,13 +8,13 @@ const codeExcerpt = require('../lib/code-excerpt');
 chalk.enabled = true;
 
 test('read code excerpt', t => {
-	const path = tempWrite.sync([
+	const file = tempWrite.sync([
 		'function a() {',
 		'\talert();',
 		'}'
 	].join('\n'));
 
-	const excerpt = codeExcerpt(path, 2);
+	const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false});
 	const expected = [
 		` ${chalk.grey('1:')} function a() {`,
 		chalk.bgRed(` 2:   alert();    `),
@@ -25,13 +26,13 @@ test('read code excerpt', t => {
 });
 
 test('truncate lines', t => {
-	const path = tempWrite.sync([
+	const file = tempWrite.sync([
 		'function a() {',
 		'\talert();',
 		'}'
 	].join('\n'));
 
-	const excerpt = codeExcerpt(path, 2, {maxWidth: 14});
+	const excerpt = codeExcerpt({file, line: 2, isWithinProject: true, isDependency: false}, {maxWidth: 14});
 	const expected = [
 		` ${chalk.grey('1:')} functio…`,
 		chalk.bgRed(` 2:   alert…`),
@@ -43,14 +44,14 @@ test('truncate lines', t => {
 });
 
 test('format line numbers', t => {
-	const path = tempWrite.sync([
+	const file = tempWrite.sync([
 		'', '', '', '', '', '', '', '',
 		'function a() {',
 		'\talert();',
 		'}'
 	].join('\n'));
 
-	const excerpt = codeExcerpt(path, 10);
+	const excerpt = codeExcerpt({file, line: 10, isWithinProject: true, isDependency: false});
 	const expected = [
 		` ${chalk.grey(' 9:')} function a() {`,
 		chalk.bgRed(` 10:   alert();    `),
@@ -60,3 +61,24 @@ test('format line numbers', t => {
 	t.is(excerpt, expected);
 	t.end();
 });
+
+test('noop if file cannot be read', t => {
+	const file = tempWrite.sync('');
+	fs.unlinkSync(file);
+
+	const excerpt = codeExcerpt({file, line: 10, isWithinProject: true, isDependency: false});
+	t.is(excerpt, null);
+	t.end();
+});
+
+test('noop if file is not within project', t => {
+	const excerpt = codeExcerpt({isWithinProject: false, file: __filename, line: 1});
+	t.is(excerpt, null);
+	t.end();
+});
+
+test('noop if file is a dependency', t => {
+	const excerpt = codeExcerpt({isWithinProject: true, isDependency: true, file: __filename, line: 1});
+	t.is(excerpt, null);
+	t.end();
+});

test/reporters/mini.js

@@ -1,5 +1,4 @@
 'use strict';
-const path = require('path');
 const indentString = require('indent-string');
 const tempWrite = require('temp-write');
 const flatten = require('arr-flatten');
@@ -35,6 +34,15 @@ function miniReporter(options) {
 	return reporter;
 }
 
+function source(file, line) {
+	return {
+		file,
+		line: line || 1,
+		isWithinProject: true,
+		isDependency: false
+	};
+}
+
 process.stderr.setMaxListeners(50);
 
 test('start', t => {
@@ -354,7 +362,7 @@ test('results with errors', t => {
 	const err1 = new Error('failure one');
 	err1.stack = beautifyStack(err1.stack);
 	const err1Path = tempWrite.sync('a();');
-	err1.source = {file: path.basename(err1Path), line: 1};
+	err1.source = source(err1Path);
 	err1.showOutput = true;
 	err1.actual = JSON.stringify('abc');
 	err1.actualType = 'string';
@@ -364,14 +372,14 @@ test('results with errors', t => {
 	const err2 = new Error('failure two');
 	err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
 	const err2Path = tempWrite.sync('b();');
-	err2.source = {file: path.basename(err2Path), line: 1};
+	err2.source = source(err2Path);
 	err2.showOutput = true;
 	err2.actual = JSON.stringify([1]);
 	err2.actualType = 'array';
 	err2.expected = JSON.stringify([2]);
 	err2.expectedType = 'array';
 
-	const reporter = miniReporter({basePath: path.dirname(err1Path)});
+	const reporter = miniReporter();
 	reporter.failCount = 1;
 
 	const runStatus = {
@@ -393,7 +401,7 @@ test('results with errors', t => {
 		'  ' + chalk.bold.white('failed one'),
 		'  ' + chalk.grey(`${err1.source.file}:${err1.source.line}`),
 		'',
-		indentString(codeExcerpt(err1Path, err1.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err1.source), 2).split('\n'),
 		'',
 		indentString(formatAssertError(err1), 2).split('\n'),
 		/failure one/,
@@ -406,7 +414,7 @@ test('results with errors', t => {
 		'  ' + chalk.bold.white('failed two'),
 		'  ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
 		'',
-		indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err2.source), 2).split('\n'),
 		'',
 		indentString(formatAssertError(err2), 2).split('\n'),
 		/failure two/
@@ -426,14 +434,14 @@ test('results with errors and disabled code excerpts', t => {
 	const err2 = new Error('failure two');
 	err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
 	const err2Path = tempWrite.sync('b();');
-	err2.source = {file: path.basename(err2Path), line: 1};
+	err2.source = source(err2Path);
 	err2.showOutput = true;
 	err2.actual = JSON.stringify([1]);
 	err2.actualType = 'array';
 	err2.expected = JSON.stringify([2]);
 	err2.expectedType = 'array';
 
-	const reporter = miniReporter({color: true, basePath: path.dirname(err2Path)});
+	const reporter = miniReporter({color: true});
 	reporter.failCount = 1;
 
 	const runStatus = {
@@ -465,7 +473,7 @@ test('results with errors and disabled code excerpts', t => {
 		'  ' + chalk.bold.white('failed two'),
 		'  ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
 		'',
-		indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err2.source), 2).split('\n'),
 		'',
 		indentString(formatAssertError(err2), 2).split('\n'),
 		/failure two/
@@ -477,7 +485,7 @@ test('results with errors and broken code excerpts', t => {
 	const err1 = new Error('failure one');
 	err1.stack = beautifyStack(err1.stack);
 	const err1Path = tempWrite.sync('a();');
-	err1.source = {file: path.basename(err1Path), line: 10};
+	err1.source = source(err1Path, 10);
 	err1.showOutput = true;
 	err1.actual = JSON.stringify('abc');
 	err1.actualType = 'string';
@@ -487,14 +495,14 @@ test('results with errors and broken code excerpts', t => {
 	const err2 = new Error('failure two');
 	err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
 	const err2Path = tempWrite.sync('b();');
-	err2.source = {file: path.basename(err2Path), line: 1};
+	err2.source = source(err2Path);
 	err2.showOutput = true;
 	err2.actual = JSON.stringify([1]);
 	err2.actualType = 'array';
 	err2.expected = JSON.stringify([2]);
 	err2.expectedType = 'array';
 
-	const reporter = miniReporter({color: true, basePath: path.dirname(err2Path)});
+	const reporter = miniReporter({color: true});
 	reporter.failCount = 1;
 
 	const runStatus = {
@@ -527,7 +535,7 @@ test('results with errors and broken code excerpts', t => {
 		'  ' + chalk.bold.white('failed two'),
 		'  ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
 		'',
-		indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err2.source), 2).split('\n'),
 		'',
 		indentString(formatAssertError(err2), 2).split('\n'),
 		/failure two/
@@ -539,7 +547,7 @@ test('results with errors and disabled assert output', t => {
 	const err1 = new Error('failure one');
 	err1.stack = beautifyStack(err1.stack);
 	const err1Path = tempWrite.sync('a();');
-	err1.source = {file: path.basename(err1Path), line: 1};
+	err1.source = source(err1Path);
 	err1.showOutput = false;
 	err1.actual = JSON.stringify('abc');
 	err1.actualType = 'string';
@@ -549,14 +557,14 @@ test('results with errors and disabled assert output', t => {
 	const err2 = new Error('failure two');
 	err2.stack = 'error message\nTest.fn (test.js:1:1)\n';
 	const err2Path = tempWrite.sync('b();');
-	err2.source = {file: path.basename(err2Path), line: 1};
+	err2.source = source(err2Path);
 	err2.showOutput = true;
 	err2.actual = JSON.stringify([1]);
 	err2.actualType = 'array';
 	err2.expected = JSON.stringify([2]);
 	err2.expectedType = 'array';
 
-	const reporter = miniReporter({color: true, basePath: path.dirname(err1Path)});
+	const reporter = miniReporter({color: true});
 	reporter.failCount = 1;
 
 	const runStatus = {
@@ -578,7 +586,7 @@ test('results with errors and disabled assert output', t => {
 		'  ' + chalk.bold.white('failed one'),
 		'  ' + chalk.grey(`${err1.source.file}:${err1.source.line}`),
 		'',
-		indentString(codeExcerpt(err1Path, err1.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err1.source), 2).split('\n'),
 		'',
 		/failure one/,
 		'',
@@ -590,7 +598,7 @@ test('results with errors and disabled assert output', t => {
 		'  ' + chalk.bold.white('failed two'),
 		'  ' + chalk.grey(`${err2.source.file}:${err2.source.line}`),
 		'',
-		indentString(codeExcerpt(err2Path, err2.source.line), 2).split('\n'),
+		indentString(codeExcerpt(err2.source), 2).split('\n'),
 		'',
 		indentString(formatAssertError(err2), 2).split('\n'),
 		/failure two/