Skip to content

Commit 827cc72

Browse files
novemberbornsindresorhus
authored andcommitted
Always fail tests with an assertion error
Wrap callback errors, rejected promises or synchronously thrown values in an `AssertionError`. Rely on the `actual` value being printed by the reporters, which means it's still shown to the user. Remove now obsolete `Error: ` prefix added to non-assertion errors in `run-status.js`. Instead more helpful messages from `test.js` can be shown. Try to reuse the stack trace from any original error, so magic assert can show context. Fail `t.throws()` / `t.notThrows()` with an AssertionError if called with an improper argument, rather than throwing a TypeError.
1 parent 22c93ed commit 827cc72

File tree

6 files changed

+53
-45
lines changed

6 files changed

+53
-45
lines changed

lib/assert.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ function wrapAssertions(callbacks) {
122122
} else if (isObservable(fn)) {
123123
promise = observableToPromise(fn);
124124
} else if (typeof fn !== 'function') {
125-
throw new TypeError('t.throws must be called with a function, Promise, or Observable');
125+
fail(this, new AssertionError({
126+
actual: fn,
127+
message: '`t.throws()` must be called with a function, Promise, or Observable'
128+
}));
129+
return;
126130
}
127131

128132
let coreAssertThrowsErrorArg;
@@ -177,7 +181,11 @@ function wrapAssertions(callbacks) {
177181
} else if (isObservable(fn)) {
178182
promise = observableToPromise(fn);
179183
} else if (typeof fn !== 'function') {
180-
throw new TypeError('t.notThrows must be called with a function, Promise, or Observable');
184+
fail(this, new AssertionError({
185+
actual: fn,
186+
message: '`t.notThrows()` must be called with a function, Promise, or Observable'
187+
}));
188+
return;
181189
}
182190

183191
const test = fn => {

lib/run-status.js

-4
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,6 @@ class RunStatus extends EventEmitter {
8787
test.title = this.prefixTitle(test.file) + test.title;
8888

8989
if (test.error) {
90-
if (test.error.name !== 'AssertionError') {
91-
test.error.message = `Error: ${test.error.message}`;
92-
}
93-
9490
this.errors.push(test);
9591
}
9692

lib/test.js

+15-16
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
'use strict';
2-
const inspect = require('util').inspect;
32
const isGeneratorFn = require('is-generator-fn');
43
const maxTimeout = require('max-timeout');
54
const Promise = require('bluebird');
@@ -129,8 +128,6 @@ class Test {
129128
this.assertCount++;
130129
}
131130
_setAssertError(err) {
132-
throwsHelper(err);
133-
134131
if (this.assertError !== undefined) {
135132
return;
136133
}
@@ -155,16 +152,17 @@ class Test {
155152
ret = this.fn(this._createExecutionContext());
156153
} catch (err) {
157154
this.threwSync = true;
155+
throwsHelper(err);
158156

159-
if (err instanceof Error) {
160-
this._setAssertError(err);
161-
} else {
162-
this._setAssertError(new assert.AssertionError({
157+
let error = err;
158+
if (!(err instanceof assert.AssertionError)) {
159+
error = new assert.AssertionError({
163160
actual: err,
164-
message: `Non-error thrown with value: ${inspect(err, {depth: null})}`,
165-
operator: 'catch'
166-
}));
161+
message: `Error thrown in test`,
162+
stack: err instanceof Error && err.stack
163+
});
167164
}
165+
this._setAssertError(error);
168166
}
169167

170168
return ret;
@@ -214,11 +212,13 @@ class Test {
214212
return Promise.resolve(ret).then(
215213
() => this.exit(),
216214
err => {
217-
if (!(err instanceof Error)) {
215+
throwsHelper(err);
216+
217+
if (!(err instanceof assert.AssertionError)) {
218218
err = new assert.AssertionError({
219219
actual: err,
220-
message: `Promise rejected with: ${inspect(err, {depth: null})}`,
221-
operator: 'promise'
220+
message: 'Rejected promise returned by test',
221+
stack: err instanceof Error && err.stack
222222
});
223223
}
224224

@@ -263,11 +263,10 @@ class Test {
263263
}
264264
_end(err) {
265265
if (err) {
266-
if (!(err instanceof Error)) {
266+
if (!(err instanceof assert.AssertionError)) {
267267
err = new assert.AssertionError({
268268
actual: err,
269-
message: 'Callback called with an error: ' + inspect(err, {depth: null}),
270-
operator: 'callback'
269+
message: 'Callback called with an error'
271270
});
272271
}
273272

test/assert.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,8 @@ test('.throws should throw if passed a bad value', t => {
412412
t.throws(() => {
413413
assertions.throws('not a function');
414414
}, {
415-
name: 'TypeError',
416-
message: /t\.throws must be called with a function, Promise, or Observable/
415+
name: 'AssertionError',
416+
message: /`t\.throws\(\)` must be called with a function, Promise, or Observable/
417417
});
418418
});
419419

@@ -423,8 +423,8 @@ test('.notThrows should throw if passed a bad value', t => {
423423
t.throws(() => {
424424
assertions.notThrows('not a function');
425425
}, {
426-
name: 'TypeError',
427-
message: /t\.notThrows must be called with a function, Promise, or Observable/
426+
name: 'AssertionError',
427+
message: /`t\.notThrows\(\)` must be called with a function, Promise, or Observable/
428428
});
429429
});
430430

test/promise.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -293,8 +293,8 @@ test('reject', t => {
293293
});
294294
}).run().then(result => {
295295
t.is(result.passed, false);
296-
t.is(result.reason.name, 'Error');
297-
t.is(result.reason.message, 'unicorn');
296+
t.is(result.reason.actual.name, 'Error');
297+
t.is(result.reason.actual.message, 'unicorn');
298298
t.end();
299299
});
300300
});
@@ -303,7 +303,8 @@ test('reject with non-Error', t => {
303303
ava(() => Promise.reject('failure')).run().then(result => {
304304
t.is(result.passed, false);
305305
t.is(result.reason.name, 'AssertionError');
306-
t.is(result.reason.message, 'Promise rejected with: \'failure\'');
306+
t.is(result.reason.message, 'Rejected promise returned by test');
307+
t.is(result.reason.actual, 'failure');
307308
t.end();
308309
});
309310
});

test/test.js

+20-16
Original file line numberDiff line numberDiff line change
@@ -131,14 +131,15 @@ test('run more assertions than planned', t => {
131131
t.end();
132132
});
133133

134-
test('handle non-assertion errors', t => {
134+
test('wrap non-assertion errors', t => {
135+
const err = new Error();
135136
const result = ava(() => {
136-
throw new Error();
137+
throw err;
137138
}).run();
138139

139140
t.is(result.passed, false);
140-
t.is(result.reason.name, 'Error');
141-
t.true(result.reason instanceof Error);
141+
t.is(result.reason.name, 'AssertionError');
142+
t.is(result.reason.actual, err);
142143
t.end();
143144
});
144145

@@ -157,7 +158,9 @@ test('end can be used as callback with error', t => {
157158
a.end(err);
158159
}).run().then(result => {
159160
t.is(result.passed, false);
160-
t.is(result.reason, err);
161+
t.is(result.reason.name, 'AssertionError');
162+
t.is(result.reason.actual, err);
163+
t.is(result.reason.message, 'Callback called with an error');
161164
t.end();
162165
});
163166
});
@@ -171,20 +174,20 @@ test('end can be used as callback with a non-error as its error argument', t =>
171174
t.ok(result.reason);
172175
t.is(result.reason.name, 'AssertionError');
173176
t.is(result.reason.actual, nonError);
174-
t.is(result.reason.message, 'Callback called with an error: { foo: \'bar\' }');
177+
t.is(result.reason.message, 'Callback called with an error');
175178
t.end();
176179
});
177180
});
178181

179182
test('handle non-assertion errors even when planned', t => {
183+
const err = new Error('bar');
180184
const result = ava(a => {
181185
a.plan(1);
182-
throw new Error('bar');
186+
throw err;
183187
}).run();
184188

185189
t.is(result.passed, false);
186-
t.is(result.reason.name, 'Error');
187-
t.is(result.reason.message, 'bar');
190+
t.is(result.reason.actual, err);
188191
t.end();
189192
});
190193

@@ -288,7 +291,9 @@ test('fails if a bad value is passed to t.throws', t => {
288291

289292
t.is(result.passed, false);
290293
t.ok(result.reason);
291-
t.is(result.reason.name, 'TypeError');
294+
t.is(result.reason.name, 'AssertionError');
295+
t.is(result.reason.message, '`t.throws()` must be called with a function, Promise, or Observable');
296+
t.is(result.reason.actual, 'not a function');
292297
t.end();
293298
});
294299

@@ -427,9 +432,8 @@ test('fails with thrown falsy value', t => {
427432

428433
t.is(result.passed, false);
429434
t.is(result.reason.actual, 0);
430-
t.is(result.reason.message, 'Non-error thrown with value: 0');
435+
t.is(result.reason.message, 'Error thrown in test');
431436
t.is(result.reason.name, 'AssertionError');
432-
t.is(result.reason.operator, 'catch');
433437
t.end();
434438
});
435439

@@ -441,9 +445,8 @@ test('fails with thrown non-error object', t => {
441445

442446
t.is(result.passed, false);
443447
t.is(result.reason.actual, obj);
444-
t.is(result.reason.message, 'Non-error thrown with value: { foo: \'bar\' }');
448+
t.is(result.reason.message, 'Error thrown in test');
445449
t.is(result.reason.name, 'AssertionError');
446-
t.is(result.reason.operator, 'catch');
447450
t.end();
448451
});
449452

@@ -489,12 +492,13 @@ test('end should not be called multiple times', t => {
489492
});
490493

491494
test('cb test that throws sync', t => {
495+
const err = new Error('foo');
492496
const result = ava.cb(() => {
493-
throw new Error('foo');
497+
throw err;
494498
}).run();
495499

496500
t.is(result.passed, false);
497-
t.is(result.reason.message, 'foo');
501+
t.is(result.reason.actual, err);
498502
t.end();
499503
});
500504

0 commit comments

Comments
 (0)