Merge pull request #555 from novemberborn/errors-are-errors

Consistent error determination

Author: sindresorhus

api.js

@@ -114,9 +114,7 @@ Api.prototype._handleStats = function (stats) {
 Api.prototype._handleTest = function (test) {
 	test.title = this._prefixTitle(test.file) + test.title;
 
-	var isError = test.error.message;
-
-	if (isError) {
+	if (test.error) {
 		if (test.error.powerAssertContext) {
 			var message = formatter(test.error.powerAssertContext);
 
@@ -132,8 +130,6 @@ Api.prototype._handleTest = function (test) {
 		}
 
 		this.errors.push(test);
-	} else {
-		test.error = null;
 	}
 
 	this.emit('test', test);

index.js

@@ -46,10 +46,13 @@ function test(props) {
 		return;
 	}
 
-	props.error = hasError ? serializeError(props.error) : {};
-
-	if (props.error.stack) {
-		props.error.stack = beautifyStack(props.error.stack);
+	if (hasError) {
+		props.error = serializeError(props.error);
+		if (props.error.stack) {
+			props.error.stack = beautifyStack(props.error.stack);
+		}
+	} else {
+		props.error = null;
 	}
 
 	send('test', props);

lib/test.js

@@ -7,6 +7,7 @@ var co = require('co-with-promise');
 var observableToPromise = require('observable-to-promise');
 var isPromise = require('is-promise');
 var isObservable = require('is-observable');
+var inspect = require('util').inspect;
 var assert = require('./assert');
 var enhanceAssert = require('./enhance-assert');
 var globals = require('./globals');
@@ -70,10 +71,6 @@ Test.prototype._setAssertError = function (err) {
 		return;
 	}
 
-	if (err === undefined) {
-		err = 'undefined';
-	}
-
 	this.assertError = err;
 };
 
@@ -95,7 +92,15 @@ Test.prototype._run = function () {
 	try {
 		ret = this.fn(this._publicApi());
 	} catch (err) {
-		this._setAssertError(err);
+		if (err instanceof Error) {
+			this._setAssertError(err);
+		} else {
+			this._setAssertError(new assert.AssertionError({
+				actual: err,
+				message: 'Non-error thrown with value: ' + inspect(err, {depth: null}),
+				operator: 'catch'
+			}));
+		}
 	}
 
 	return ret;
@@ -156,7 +161,7 @@ Test.prototype.run = function () {
 				if (!(err instanceof Error)) {
 					err = new assert.AssertionError({
 						actual: err,
-						message: 'Promise rejected with "' + err + '"',
+						message: 'Promise rejected with: ' + inspect(err, {depth: null}),
 						operator: 'promise'
 					});
 				}
@@ -192,11 +197,14 @@ Object.defineProperty(Test.prototype, 'end', {
 
 Test.prototype._end = function (err) {
 	if (err) {
-		this._setAssertError(new assert.AssertionError({
-			actual: err,
-			message: 'Callback called with an error → ' + err,
-			operator: 'callback'
-		}));
+		if (!(err instanceof Error)) {
+			err = new assert.AssertionError({
+				actual: err,
+				message: 'Callback called with an error: ' + inspect(err, {depth: null}),
+				operator: 'callback'
+			});
+		}
+		this._setAssertError(err);
 
 		this.exit();
 		return;

test/api.js

@@ -287,6 +287,17 @@ test('uncaught exception will throw an error', function (t) {
 		});
 });
 
+test('errors can occur without messages', function (t) {
+	t.plan(2);
+
+	var api = new Api([path.join(__dirname, 'fixture/error-without-message.js')]);
+	api.run()
+		.then(function () {
+			t.is(api.failCount, 1);
+			t.is(api.errors.length, 1);
+		});
+});
+
 test('stack traces for exceptions are corrected using a source map file', function (t) {
 	t.plan(4);
 

test/fixture/error-without-message.js

@@ -0,0 +1,5 @@
+import test from '../../';
+
+test('throw an error without a message', () => {
+	throw new Error();
+});

test/hooks.js

@@ -309,7 +309,7 @@ test('don\'t display hook title if it did not fail', function (t) {
 	fork(path.join(__dirname, 'fixture', 'hooks-passing.js'))
 		.run()
 		.on('test', function (test) {
-			t.same(test.error, {});
+			t.same(test.error, null);
 			t.is(test.title, 'pass');
 		})
 		.then(function () {

test/promise.js

@@ -318,7 +318,7 @@ test('reject with non-Error', function (t) {
 	}).run().then(function (result) {
 		t.is(result.passed, false);
 		t.is(result.reason.name, 'AssertionError');
-		t.is(result.reason.message, 'Promise rejected with "failure"');
+		t.is(result.reason.message, 'Promise rejected with: \'failure\'');
 		t.end();
 	});
 });

test/test.js

@@ -128,13 +128,26 @@ test('end can be used as callback without maintaining thisArg', function (t) {
 });
 
 test('end can be used as callback with error', function (t) {
+	var err = new Error('failed');
 	ava.cb(function (a) {
-		a.end(new Error('failed'));
+		a.end(err);
 	}).run().then(function (result) {
 		t.is(result.passed, false);
-		t.true(result.reason instanceof Error);
-		// TODO: Question - why not just set the reason to the error?
-		t.match(result.reason.message, /Callback called with an error/);
+		t.is(result.reason, err);
+		t.end();
+	});
+});
+
+test('end can be used as callback with a non-error as its error argument', function (t) {
+	var nonError = {foo: 'bar'};
+	ava.cb(function (a) {
+		a.end(nonError);
+	}).run().then(function (result) {
+		t.is(result.passed, false);
+		t.ok(result.reason);
+		t.is(result.reason.name, 'AssertionError');
+		t.is(result.reason.actual, nonError);
+		t.is(result.reason.message, 'Callback called with an error: { foo: \'bar\' }');
 		t.end();
 	});
 });
@@ -364,17 +377,24 @@ test('fails with thrown falsy value', function (t) {
 	}).run();
 
 	t.is(result.passed, false);
-	t.is(result.reason, 0);
+	t.is(result.reason.actual, 0);
+	t.is(result.reason.message, 'Non-error thrown with value: 0');
+	t.is(result.reason.name, 'AssertionError');
+	t.is(result.reason.operator, 'catch');
 	t.end();
 });
 
-test('throwing undefined will be converted to string "undefined"', function (t) {
+test('fails with thrown non-error object', function (t) {
+	var obj = {foo: 'bar'};
 	var result = ava(function () {
-		throw undefined; // eslint-disable-line no-throw-literal
+		throw obj;
 	}).run();
 
 	t.is(result.passed, false);
-	t.is(result.reason, 'undefined');
+	t.is(result.reason.actual, obj);
+	t.is(result.reason.message, 'Non-error thrown with value: { foo: \'bar\' }');
+	t.is(result.reason.name, 'AssertionError');
+	t.is(result.reason.operator, 'catch');
 	t.end();
 });