Ensure the assertion message is a string

Fixes #1125.

Author: novemberborn

lib/assert.js

@@ -265,18 +265,46 @@ class Assertions {
 			patterns: [pattern]
 		});
 
+		const checkMessage = (assertion, message, powerAssert = false) => {
+			if (typeof message === 'undefined' || typeof message === 'string') {
+				return true;
+			}
+
+			const error = new AssertionError({
+				assertion,
+				improperUsage: true,
+				message: 'The assertion message must be a string',
+				values: [formatWithLabel('Called with:', message)]
+			});
+
+			if (powerAssert) {
+				throw error;
+			}
+
+			fail(error);
+			return false;
+		};
+
 		this.pass = withSkip(() => {
 			pass();
 		});
 
 		this.fail = withSkip(message => {
+			if (!checkMessage('fail', message)) {
+				return;
+			}
+
 			fail(new AssertionError({
 				assertion: 'fail',
 				message: message || 'Test failed via `t.fail()`'
 			}));
 		});
 
 		this.is = withSkip((actual, expected, message) => {
+			if (!checkMessage('is', message)) {
+				return;
+			}
+
 			if (Object.is(actual, expected)) {
 				pass();
 			} else {
@@ -303,6 +331,10 @@ class Assertions {
 		});
 
 		this.not = withSkip((actual, expected, message) => {
+			if (!checkMessage('not', message)) {
+				return;
+			}
+
 			if (Object.is(actual, expected)) {
 				fail(new AssertionError({
 					assertion: 'not',
@@ -316,6 +348,10 @@ class Assertions {
 		});
 
 		this.deepEqual = withSkip((actual, expected, message) => {
+			if (!checkMessage('deepEqual', message)) {
+				return;
+			}
+
 			const result = concordance.compare(actual, expected, concordanceOptions);
 			if (result.pass) {
 				pass();
@@ -332,6 +368,10 @@ class Assertions {
 		});
 
 		this.notDeepEqual = withSkip((actual, expected, message) => {
+			if (!checkMessage('notDeepEqual', message)) {
+				return;
+			}
+
 			const result = concordance.compare(actual, expected, concordanceOptions);
 			if (result.pass) {
 				const actualDescriptor = result.actual || concordance.describe(actual, concordanceOptions);
@@ -351,6 +391,11 @@ class Assertions {
 			// operator, so we can determine the total number of arguments passed
 			// to the function.
 			let [fn, expectations, message] = args;
+
+			if (!checkMessage('throws', message)) {
+				return;
+			}
+
 			if (typeof fn !== 'function') {
 				fail(new AssertionError({
 					assertion: 'throws',
@@ -412,6 +457,11 @@ class Assertions {
 
 		this.throwsAsync = withSkip((...args) => {
 			let [thrower, expectations, message] = args;
+
+			if (!checkMessage('throwsAsync', message)) {
+				return Promise.resolve();
+			}
+
 			if (typeof thrower !== 'function' && !isPromise(thrower)) {
 				fail(new AssertionError({
 					assertion: 'throwsAsync',
@@ -492,6 +542,10 @@ class Assertions {
 		});
 
 		this.notThrows = withSkip((fn, message) => {
+			if (!checkMessage('notThrows', message)) {
+				return;
+			}
+
 			if (typeof fn !== 'function') {
 				fail(new AssertionError({
 					assertion: 'notThrows',
@@ -518,6 +572,10 @@ class Assertions {
 		});
 
 		this.notThrowsAsync = withSkip((nonThrower, message) => {
+			if (!checkMessage('notThrowsAsync', message)) {
+				return Promise.resolve();
+			}
+
 			if (typeof nonThrower !== 'function' && !isPromise(nonThrower)) {
 				fail(new AssertionError({
 					assertion: 'notThrowsAsync',
@@ -574,20 +632,31 @@ class Assertions {
 			return handlePromise(retval, true);
 		});
 
-		this.snapshot = withSkip((expected, optionsOrMessage, message) => {
-			const options = {};
-			if (typeof optionsOrMessage === 'string') {
-				message = optionsOrMessage;
-			} else if (optionsOrMessage) {
-				options.id = optionsOrMessage.id;
+		this.snapshot = withSkip((expected, ...rest) => {
+			let message;
+			let snapshotOptions;
+			if (rest.length > 1) {
+				[snapshotOptions, message] = rest;
+			} else {
+				const [optionsOrMessage] = rest;
+				if (typeof optionsOrMessage === 'object') {
+					snapshotOptions = optionsOrMessage;
+				} else {
+					message = optionsOrMessage;
+				}
 			}
 
-			options.expected = expected;
-			options.message = message;
+			if (!checkMessage('snapshot', message)) {
+				return;
+			}
 
 			let result;
 			try {
-				result = compareWithSnapshot(options);
+				result = compareWithSnapshot({
+					expected,
+					id: snapshotOptions ? snapshotOptions.id : undefined,
+					message
+				});
 			} catch (error) {
 				if (!(error instanceof snapshotManager.SnapshotError)) {
 					throw error;
@@ -625,6 +694,10 @@ class Assertions {
 		});
 
 		this.truthy = withSkip((actual, message) => {
+			if (!checkMessage('truthy', message)) {
+				return;
+			}
+
 			if (actual) {
 				pass();
 			} else {
@@ -638,6 +711,10 @@ class Assertions {
 		});
 
 		this.falsy = withSkip((actual, message) => {
+			if (!checkMessage('falsy', message)) {
+				return;
+			}
+
 			if (actual) {
 				fail(new AssertionError({
 					assertion: 'falsy',
@@ -651,6 +728,10 @@ class Assertions {
 		});
 
 		this.true = withSkip((actual, message) => {
+			if (!checkMessage('true', message)) {
+				return;
+			}
+
 			if (actual === true) {
 				pass();
 			} else {
@@ -663,6 +744,10 @@ class Assertions {
 		});
 
 		this.false = withSkip((actual, message) => {
+			if (!checkMessage('false', message)) {
+				return;
+			}
+
 			if (actual === false) {
 				pass();
 			} else {
@@ -675,6 +760,10 @@ class Assertions {
 		});
 
 		this.regex = withSkip((string, regex, message) => {
+			if (!checkMessage('regex', message)) {
+				return;
+			}
+
 			if (typeof string !== 'string') {
 				fail(new AssertionError({
 					assertion: 'regex',
@@ -711,6 +800,10 @@ class Assertions {
 		});
 
 		this.notRegex = withSkip((string, regex, message) => {
+			if (!checkMessage('notRegex', message)) {
+				return;
+			}
+
 			if (typeof string !== 'string') {
 				fail(new AssertionError({
 					assertion: 'notRegex',
@@ -749,6 +842,8 @@ class Assertions {
 		this.assert = withSkip(withPowerAssert(
 			'assert(value, [message])',
 			(actual, message) => {
+				checkMessage('assert', message, true);
+
 				if (!actual) {
 					throw new AssertionError({
 						assertion: 'assert',