Skip to content

Better throws helper #1315

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

Merged
merged 5 commits into from
Mar 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class AssertionError extends Error {
this.name = 'AssertionError';

this.assertion = opts.assertion;
this.fixedSource = opts.fixedSource;
this.improperUsage = opts.improperUsage || false;
this.operator = opts.operator;
this.values = opts.values || [];

Expand Down Expand Up @@ -129,6 +131,7 @@ function wrapAssertions(callbacks) {
} else if (typeof fn !== 'function') {
fail(this, new AssertionError({
assertion: 'throws',
improperUsage: true,
message: '`t.throws()` must be called with a function, Promise, or Observable',
values: [formatAssertError.formatWithLabel('Called with:', fn)]
}));
Expand Down Expand Up @@ -199,6 +202,7 @@ function wrapAssertions(callbacks) {
} else if (typeof fn !== 'function') {
fail(this, new AssertionError({
assertion: 'notThrows',
improperUsage: true,
message: '`t.notThrows()` must be called with a function, Promise, or Observable',
values: [formatAssertError.formatWithLabel('Called with:', fn)]
}));
Expand Down Expand Up @@ -314,13 +318,15 @@ function wrapAssertions(callbacks) {
if (typeof string !== 'string') {
throw new AssertionError({
assertion: 'regex',
improperUsage: true,
message: '`t.regex()` must be called with a string',
values: [formatAssertError.formatWithLabel('Called with:', string)]
});
}
if (!(regex instanceof RegExp)) {
throw new AssertionError({
assertion: 'regex',
improperUsage: true,
message: '`t.regex()` must be called with a regular expression',
values: [formatAssertError.formatWithLabel('Called with:', regex)]
});
Expand All @@ -342,13 +348,15 @@ function wrapAssertions(callbacks) {
if (typeof string !== 'string') {
throw new AssertionError({
assertion: 'notRegex',
improperUsage: true,
message: '`t.notRegex()` must be called with a string',
values: [formatAssertError.formatWithLabel('Called with:', string)]
});
}
if (!(regex instanceof RegExp)) {
throw new AssertionError({
assertion: 'notRegex',
improperUsage: true,
message: '`t.notRegex()` must be called with a regular expression',
values: [formatAssertError.formatWithLabel('Called with:', regex)]
});
Expand Down
3 changes: 1 addition & 2 deletions lib/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ const runner = new Runner({
match: opts.match
});

// Note that test files have require('ava')
worker.avaRequired = true;
worker.setRunner(runner);

// If fail-fast is enabled, use this variable to detect
// that no more tests should be logged
Expand Down
21 changes: 21 additions & 0 deletions lib/reporters/improper-usage-messages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';
const chalk = require('chalk');

exports.forError = error => {
if (!error.improperUsage) {
return null;
}

const assertion = error.assertion;
if (assertion !== 'throws' || !assertion === 'notThrows') {
return null;
}

return `Try wrapping the first argument to \`t.${assertion}()\` in a function:

${chalk.cyan(`t.${assertion}(() => { `)}${chalk.grey('/* your code here */')}${chalk.cyan(' })')}

Visit the following URL for more details:

${chalk.blue.underline('https://github.com/avajs/ava#throwsfunctionpromise-error-message')}`;
};
12 changes: 9 additions & 3 deletions lib/reporters/mini.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const formatAssertError = require('../format-assert-error');
const extractStack = require('../extract-stack');
const codeExcerpt = require('../code-excerpt');
const colors = require('../colors');
const improperUsageMessages = require('./improper-usage-messages');

// TODO(@jamestalamge): This should be fixed in log-update and ansi-escapes once we are confident it's a good solution.
const CSI = '\u001B[';
Expand Down Expand Up @@ -191,15 +192,20 @@ class MiniReporter {
}
}

if (test.error.message) {
status += '\n' + indentString(test.error.message, 2) + '\n';
}

if (test.error.avaAssertionError) {
const formatted = formatAssertError.formatSerializedError(test.error);
if (formatted) {
status += '\n' + indentString(formatted, 2);
}
}

if (test.error.message) {
status += '\n' + indentString(test.error.message, 2) + '\n';
const message = improperUsageMessages.forError(test.error);
if (message) {
status += '\n' + indentString(message, 2) + '\n';
}
}

if (test.error.stack) {
Expand Down
12 changes: 9 additions & 3 deletions lib/reporters/verbose.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const formatAssertError = require('../format-assert-error');
const extractStack = require('../extract-stack');
const codeExcerpt = require('../code-excerpt');
const colors = require('../colors');
const improperUsageMessages = require('./improper-usage-messages');

class VerboseReporter {
constructor(options) {
Expand Down Expand Up @@ -111,15 +112,20 @@ class VerboseReporter {
}
}

if (test.error.message) {
output += '\n' + indentString(test.error.message, 2) + '\n';
}

if (test.error.avaAssertionError) {
const formatted = formatAssertError.formatSerializedError(test.error);
if (formatted) {
output += '\n' + indentString(formatted, 2);
}
}

if (test.error.message) {
output += '\n' + indentString(test.error.message, 2) + '\n';
const message = improperUsageMessages.forError(test.error);
if (message) {
output += '\n' + indentString(message, 2) + '\n';
}
}

if (test.error.stack) {
Expand Down
3 changes: 3 additions & 0 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ class Runner extends EventEmitter {

return Promise.resolve(this.tests.build().run()).then(this._buildStats);
}
attributeLeakedError(err) {
return this.tests.attributeLeakedError(err);
}
}

optionChain(chainableMethods, function (opts, args) {
Expand Down
18 changes: 14 additions & 4 deletions lib/serialize-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ function filter(propertyName, isRoot) {
}

const stackUtils = new StackUtils();
function buildSource(stack) {
function extractSource(stack) {
if (!stack) {
return null;
}

const firstStackLine = extractStack(stack).split('\n')[0];
const source = stackUtils.parseLine(firstStackLine);
return stackUtils.parseLine(firstStackLine);
}
function buildSource(source) {
if (!source) {
return null;
}
Expand Down Expand Up @@ -49,21 +51,29 @@ module.exports = error => {
const stack = typeof error.stack === 'string' ?
beautifyStack(error.stack) :
null;
const source = buildSource(stack);

const retval = {
avaAssertionError: isAvaAssertionError(error),
source
source: buildSource(extractSource(stack))
};
if (stack) {
retval.stack = stack;
}

if (retval.avaAssertionError) {
retval.improperUsage = error.improperUsage;
retval.message = error.message;
retval.name = error.name;
retval.statements = error.statements;
retval.values = error.values;

if (error.fixedSource) {
const source = buildSource(error.fixedSource);
if (source) {
retval.source = source;
}
}

if (error.assertion) {
retval.assertion = error.assertion;
}
Expand Down
23 changes: 19 additions & 4 deletions lib/test-collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class TestCollection extends EventEmitter {
afterEachAlways: []
};

this.pendingTestInstances = new Set();

this._emitTestResult = this._emitTestResult.bind(this);
}
add(test) {
Expand Down Expand Up @@ -100,8 +102,9 @@ class TestCollection extends EventEmitter {
}
};
}
_emitTestResult(test) {
this.emit('test', test);
_emitTestResult(result) {
this.pendingTestInstances.delete(result.result);
this.emit('test', result);
}
_buildHooks(hooks, testTitle, context) {
return hooks.map(hook => {
Expand All @@ -125,28 +128,32 @@ class TestCollection extends EventEmitter {
contextRef = null;
}

return new Test({
const test = new Test({
contextRef,
failWithoutAssertions: false,
fn: hook.fn,
metadata: hook.metadata,
onResult: this._emitTestResult,
title
});
this.pendingTestInstances.add(test);
return test;
}
_buildTest(test, contextRef) {
if (!contextRef) {
contextRef = null;
}

return new Test({
test = new Test({
contextRef,
failWithoutAssertions: this.failWithoutAssertions,
fn: test.fn,
metadata: test.metadata,
onResult: this._emitTestResult,
title: test.title
});
this.pendingTestInstances.add(test);
return test;
}
_buildTestWithHooks(test) {
if (test.metadata.skipped || test.metadata.todo) {
Expand Down Expand Up @@ -183,6 +190,14 @@ class TestCollection extends EventEmitter {
}
return finalTests;
}
attributeLeakedError(err) {
for (const test of this.pendingTestInstances) {
if (test.attributeLeakedError(err)) {
return true;
}
}
return false;
}
}

module.exports = TestCollection;
32 changes: 25 additions & 7 deletions lib/test-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const isObj = require('is-obj');
const adapter = require('./process-adapter');
const globals = require('./globals');
const serializeError = require('./serialize-error');
const throwsHelper = require('./throws-helper');

const opts = adapter.opts;
const testPath = opts.file;
Expand All @@ -41,20 +40,38 @@ adapter.installPrecompilerHook();
const dependencies = [];
adapter.installDependencyTracking(dependencies, testPath);

// Check if test files required ava and show error, when they didn't
exports.avaRequired = false;
// Set when main.js is required (since test files should have `require('ava')`).
let runner = null;
exports.setRunner = newRunner => {
runner = newRunner;
};

require(testPath); // eslint-disable-line import/no-dynamic-require

// If AVA was not required, show an error
if (!exports.avaRequired) {
if (!runner) {
adapter.send('no-tests', {avaRequired: false});
}

process.on('unhandledRejection', throwsHelper);
function attributeLeakedError(err) {
if (!runner) {
return false;
}

return runner.attributeLeakedError(err);
}

const attributedRejections = new Set();
process.on('unhandledRejection', (reason, promise) => {
if (attributeLeakedError(reason)) {
attributedRejections.add(promise);
}
});

process.on('uncaughtException', exception => {
throwsHelper(exception);
if (attributeLeakedError(exception)) {
return;
}

let serialized;
try {
Expand Down Expand Up @@ -85,7 +102,8 @@ process.on('ava-teardown', () => {
}
tearingDown = true;

let rejections = currentlyUnhandled();
let rejections = currentlyUnhandled()
.filter(rejection => !attributedRejections.has(rejection.promise));

if (rejections.length > 0) {
rejections = rejections.map(rejection => {
Expand Down
Loading