Skip to content

Commit 0aad35a

Browse files
committed
Make assertion validation stricter.
We can always loosen this over time, but trying to reclaim strictness would cause a major version bump. I think it’s nice when low-level libraries like a _testing library_ are exceedingly strict / clear about call site expectations. In my experience, this kind of strictness typically helps to _catch bugs_.
1 parent e7b2ec1 commit 0aad35a

5 files changed

Lines changed: 189 additions & 32 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88

99
### Added
1010

11+
- All `assert.*` functions now validate their arguments strictly: wrong argument
12+
count, wrong types, or a non-string `message` throw immediately with a
13+
descriptive error pointing at the call site (#99).
1114
- `assert.throws(fn, error, message?)` and `assert.rejects(fn, error, message?)`
1215
for asserting that a function throws or an async function rejects. The `error`
1316
argument is a `RegExp` tested against `String(thrown)` (consistent with

test/test-assert.js

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,18 @@ suite('assert', () => {
1616
test('uses custom message on failure', () => {
1717
assert.throws(() => assert(false, 'custom'), /^Error: custom$/);
1818
});
19+
20+
test('throws if message is not a string', () => {
21+
assert.throws(() => assert(false, 42), /^Error: unexpected message, expected string but got "42"$/);
22+
});
23+
24+
test('throws with no arguments', () => {
25+
assert.throws(() => assert(), /^Error: expected value to assert, but got none$/);
26+
});
27+
28+
test('throws on extra arguments', () => {
29+
assert.throws(() => assert(false, 'msg', 'extra'), /^Error: unexpected extra arguments$/);
30+
});
1931
});
2032

2133
suite('assert.deepEqual', () => {
@@ -41,34 +53,91 @@ suite('assert.deepEqual', () => {
4153
const sym = Symbol('x');
4254
assert.throws(() => assert.deepEqual({ [sym]: 1 }, {}), /^Error: deepEqual does not support symbol-keyed properties\.$/);
4355
});
56+
57+
test('throws with too few arguments', () => {
58+
assert.throws(() => assert.deepEqual(), /^Error: expected actual and expected values, but got too few arguments$/);
59+
assert.throws(() => assert.deepEqual(1), /^Error: expected actual and expected values, but got too few arguments$/);
60+
});
61+
62+
test('throws if message is not a string', () => {
63+
assert.throws(() => assert.deepEqual(1, 2, 42), /^Error: unexpected message, expected string but got "42"$/);
64+
});
65+
66+
test('throws on extra arguments', () => {
67+
assert.throws(() => assert.deepEqual(1, 1, 'msg', 'extra'), /^Error: unexpected extra arguments$/);
68+
});
4469
});
4570

4671
suite('assert.throws', () => {
4772
test('exercises the public assert.throws export', () => {
4873
assert.throws(() => { throw new Error('boom'); }, /boom/);
4974
assert.throws(() => { throw new Error('boom'); }, /^Error: boom$/);
75+
assert.throws(() => { throw new Error('boom'); }, /boom/, 'with message');
76+
});
77+
78+
test('throws with too few arguments', () => {
79+
assert.throws(() => assert.throws(), /^Error: expected fn and error arguments, but got too few arguments$/);
80+
assert.throws(() => assert.throws(() => {}), /^Error: expected fn and error arguments, but got too few arguments$/);
5081
});
5182

5283
test('throws early if fn is not a function', () => {
53-
assert.throws(() => assert.throws('not a function', /boom/), /^Error: unexpected fn value "not a function"$/);
84+
assert.throws(() => assert.throws('not a function', /boom/), /^Error: unexpected fn, expected Function but got "not a function"$/);
5485
});
5586

5687
test('throws early if error is not a RegExp', () => {
57-
assert.throws(() => assert.throws(() => {}, 'not a regexp'), /^Error: unexpected error value "not a regexp"$/);
88+
assert.throws(() => assert.throws(() => {}, 'not a regexp'), /^Error: unexpected error, expected RegExp but got "not a regexp"$/);
89+
});
90+
91+
test('throws early if fn is not a function with message', () => {
92+
assert.throws(() => assert.throws('not a function', /boom/, 'msg'), /^Error: unexpected fn, expected Function but got "not a function"$/);
93+
});
94+
95+
test('throws early if error is not a RegExp with message', () => {
96+
assert.throws(() => assert.throws(() => {}, 'not a regexp', 'msg'), /^Error: unexpected error, expected RegExp but got "not a regexp"$/);
97+
});
98+
99+
test('throws if message is not a string', () => {
100+
assert.throws(() => assert.throws(() => { throw new Error('boom'); }, /boom/, 42), /^Error: unexpected message, expected string but got "42"$/);
101+
});
102+
103+
test('throws on extra arguments', () => {
104+
assert.throws(() => assert.throws(() => {}, /boom/, 'msg', 'extra'), /^Error: unexpected extra arguments$/);
58105
});
59106
});
60107

61108
suite('assert.rejects', () => {
62109
test('exercises the public assert.rejects export', async () => {
63110
await assert.rejects(async () => { throw new Error('boom'); }, /boom/);
64111
await assert.rejects(async () => { throw new Error('boom'); }, /^Error: boom$/);
112+
await assert.rejects(async () => { throw new Error('boom'); }, /boom/, 'with message');
113+
});
114+
115+
test('throws with too few arguments', async () => {
116+
await assert.rejects(() => assert.rejects(), /^Error: expected fn and error arguments, but got too few arguments$/);
117+
await assert.rejects(() => assert.rejects(() => {}), /^Error: expected fn and error arguments, but got too few arguments$/);
65118
});
66119

67120
test('throws early if fn is not a function', async () => {
68-
await assert.rejects(() => assert.rejects('not a function', /boom/), /^Error: unexpected fn value "not a function"$/);
121+
await assert.rejects(() => assert.rejects('not a function', /boom/), /^Error: unexpected fn, expected Function but got "not a function"$/);
69122
});
70123

71124
test('throws early if error is not a RegExp', async () => {
72-
await assert.rejects(() => assert.rejects(() => {}, 'not a regexp'), /^Error: unexpected error value "not a regexp"$/);
125+
await assert.rejects(() => assert.rejects(() => {}, 'not a regexp'), /^Error: unexpected error, expected RegExp but got "not a regexp"$/);
126+
});
127+
128+
test('throws early if fn is not a function with message', async () => {
129+
await assert.rejects(() => assert.rejects('not a function', /boom/, 'msg'), /^Error: unexpected fn, expected Function but got "not a function"$/);
130+
});
131+
132+
test('throws early if error is not a RegExp with message', async () => {
133+
await assert.rejects(() => assert.rejects(() => {}, 'not a regexp', 'msg'), /^Error: unexpected error, expected RegExp but got "not a regexp"$/);
134+
});
135+
136+
test('throws if message is not a string', async () => {
137+
await assert.rejects(() => assert.rejects(async () => { throw new Error('boom'); }, /boom/, 42), /^Error: unexpected message, expected string but got "42"$/);
138+
});
139+
140+
test('throws on extra arguments', async () => {
141+
await assert.rejects(() => assert.rejects(async () => {}, /boom/, 'msg', 'extra'), /^Error: unexpected extra arguments$/);
73142
});
74143
});

types/x-test.d.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
export function assert(value: unknown, message?: string): asserts value;
1+
/**
2+
* Simple assertion which throws exception when value is not truthy.
3+
* @example
4+
* assert('foo' === 'bar', 'foo does not equal bar');
5+
* @param {unknown} value - The condition to assert (truthy/falsy)
6+
* @param {string} [message] - The assertion message
7+
* @returns {asserts value} Throws if condition is falsy or arguments are invalid.
8+
*/
9+
export function assert(value: unknown, message?: string, ...args: any[]): asserts value;
210
export namespace assert {
311
/**
412
* Strict deep-equality assertion. Supports primitives, plain objects, and
@@ -10,9 +18,9 @@ export namespace assert {
1018
* @param {unknown} actual - The actual value
1119
* @param {T} expected - The expected value
1220
* @param {string} [message] - The assertion message
13-
* @returns {asserts actual is T} Throws if values are not deeply equal.
21+
* @returns {asserts actual is T} Throws if values are not deeply equal or arguments are invalid.
1422
*/
15-
function deepEqual<T>(actual: unknown, expected: T, message?: string): asserts actual is T;
23+
function deepEqual<T>(actual: unknown, expected: T, message?: string, ...args: any[]): asserts actual is T;
1624
/**
1725
* Asserts that a function throws, testing the thrown value against a RegExp via `String(thrown)`.
1826
* @example
@@ -21,9 +29,9 @@ export namespace assert {
2129
* @param {() => void} fn - The function expected to throw
2230
* @param {RegExp} error - Tested against `String(thrown)`
2331
* @param {string} [message] - The assertion message
24-
* @returns {void}
32+
* @returns {void} Throws if the function does not throw, the thrown value does not match, or arguments are invalid.
2533
*/
26-
function throws(fn: () => void, error: RegExp, message?: string): void;
34+
function throws(fn: () => void, error: RegExp, message?: string, ...args: any[]): void;
2735
/**
2836
* Asserts that an async function rejects, testing the rejection against a RegExp via `String(thrown)`.
2937
* @example
@@ -32,9 +40,9 @@ export namespace assert {
3240
* @param {() => Promise<unknown>} fn - The function expected to reject
3341
* @param {RegExp} error - Tested against `String(thrown)`
3442
* @param {string} [message] - The assertion message
35-
* @returns {Promise<void>}
43+
* @returns {Promise<void>} Rejects if the function does not reject, the rejection value does not match, or arguments are invalid.
3644
*/
37-
function rejects(fn: () => Promise<unknown>, error: RegExp, message?: string): Promise<void>;
45+
function rejects(fn: () => Promise<unknown>, error: RegExp, message?: string, ...args: any[]): Promise<void>;
3846
}
3947
export function load(href: string): void;
4048
export function suite(text: string, callback: () => void): void;

x-test-frame.js

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export class XTestFrame {
114114
* @param {any} context
115115
* @param {any} caller
116116
* @param {any} value
117-
* @param {any} message
117+
* @param {any} [message]
118118
*/
119119
static assert(context, caller, value, message) {
120120
if (context && !context.state.bailed) {
@@ -134,12 +134,6 @@ export class XTestFrame {
134134
* @param {any} [message]
135135
*/
136136
static throws(context, caller, fn, error, message) {
137-
if (!(fn instanceof Function)) {
138-
throw new Error(`unexpected fn value "${fn}"`);
139-
}
140-
if (!(error instanceof RegExp)) {
141-
throw new Error(`unexpected error value "${error}"`);
142-
}
143137
let threw = false;
144138
let thrownValue;
145139
try {
@@ -163,12 +157,6 @@ export class XTestFrame {
163157
* @param {any} [message]
164158
*/
165159
static async rejects(context, caller, fn, error, message) {
166-
if (!(fn instanceof Function)) {
167-
throw new Error(`unexpected fn value "${fn}"`);
168-
}
169-
if (!(error instanceof RegExp)) {
170-
throw new Error(`unexpected error value "${error}"`);
171-
}
172160
let rejected = false;
173161
let rejectionValue;
174162
try {

x-test.js

Lines changed: 97 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,25 @@ import { XTestFrame } from './x-test-frame.js';
88
* assert('foo' === 'bar', 'foo does not equal bar');
99
* @param {unknown} value - The condition to assert (truthy/falsy)
1010
* @param {string} [message] - The assertion message
11-
* @returns {asserts value} Throws if condition is falsy.
11+
* @returns {asserts value} Throws if condition is falsy or arguments are invalid.
1212
*/
13-
export const assert = (value, message) => XTestFrame.assert(suiteContext, assert, value, message);
13+
export function assert(value, message) {
14+
switch (arguments.length) {
15+
case 0:
16+
throw new Error('expected value to assert, but got none');
17+
case 1:
18+
XTestFrame.assert(suiteContext, assert, value);
19+
break;
20+
case 2:
21+
if (typeof message !== 'string') {
22+
throw new Error(`unexpected message, expected string but got "${message}"`);
23+
}
24+
XTestFrame.assert(suiteContext, assert, value, message);
25+
break;
26+
default:
27+
throw new Error('unexpected extra arguments');
28+
}
29+
}
1430

1531
/**
1632
* Strict deep-equality assertion. Supports primitives, plain objects, and
@@ -22,9 +38,26 @@ export const assert = (value, message) => XTestFrame.assert(suiteContext, assert
2238
* @param {unknown} actual - The actual value
2339
* @param {T} expected - The expected value
2440
* @param {string} [message] - The assertion message
25-
* @returns {asserts actual is T} Throws if values are not deeply equal.
41+
* @returns {asserts actual is T} Throws if values are not deeply equal or arguments are invalid.
2642
*/
27-
assert.deepEqual = (actual, expected, message) => XTestFrame.deepEqual(suiteContext, assert.deepEqual, actual, expected, message);
43+
assert.deepEqual = function deepEqual(actual, expected, message) {
44+
switch (arguments.length) {
45+
case 0:
46+
case 1:
47+
throw new Error('expected actual and expected values, but got too few arguments');
48+
case 2:
49+
XTestFrame.deepEqual(suiteContext, assert.deepEqual, actual, expected);
50+
break;
51+
case 3:
52+
if (typeof message !== 'string') {
53+
throw new Error(`unexpected message, expected string but got "${message}"`);
54+
}
55+
XTestFrame.deepEqual(suiteContext, assert.deepEqual, actual, expected, message);
56+
break;
57+
default:
58+
throw new Error('unexpected extra arguments');
59+
}
60+
};
2861

2962
/**
3063
* Asserts that a function throws, testing the thrown value against a RegExp via `String(thrown)`.
@@ -34,9 +67,38 @@ assert.deepEqual = (actual, expected, message) => XTestFrame.deepEqual(suiteCont
3467
* @param {() => void} fn - The function expected to throw
3568
* @param {RegExp} error - Tested against `String(thrown)`
3669
* @param {string} [message] - The assertion message
37-
* @returns {void}
70+
* @returns {void} Throws if the function does not throw, the thrown value does not match, or arguments are invalid.
3871
*/
39-
assert.throws = (fn, error, message) => XTestFrame.throws(suiteContext, assert.throws, fn, error, message);
72+
assert.throws = function throws(fn, error, message) {
73+
switch (arguments.length) {
74+
case 0:
75+
case 1:
76+
throw new Error('expected fn and error arguments, but got too few arguments');
77+
case 2:
78+
if (!(fn instanceof Function)) {
79+
throw new Error(`unexpected fn, expected Function but got "${fn}"`);
80+
}
81+
if (!(error instanceof RegExp)) {
82+
throw new Error(`unexpected error, expected RegExp but got "${error}"`);
83+
}
84+
XTestFrame.throws(suiteContext, assert.throws, fn, error);
85+
break;
86+
case 3:
87+
if (!(fn instanceof Function)) {
88+
throw new Error(`unexpected fn, expected Function but got "${fn}"`);
89+
}
90+
if (!(error instanceof RegExp)) {
91+
throw new Error(`unexpected error, expected RegExp but got "${error}"`);
92+
}
93+
if (typeof message !== 'string') {
94+
throw new Error(`unexpected message, expected string but got "${message}"`);
95+
}
96+
XTestFrame.throws(suiteContext, assert.throws, fn, error, message);
97+
break;
98+
default:
99+
throw new Error('unexpected extra arguments');
100+
}
101+
};
40102

41103
/**
42104
* Asserts that an async function rejects, testing the rejection against a RegExp via `String(thrown)`.
@@ -46,9 +108,36 @@ assert.throws = (fn, error, message) => XTestFrame.throws(suiteContext, assert.t
46108
* @param {() => Promise<unknown>} fn - The function expected to reject
47109
* @param {RegExp} error - Tested against `String(thrown)`
48110
* @param {string} [message] - The assertion message
49-
* @returns {Promise<void>}
111+
* @returns {Promise<void>} Rejects if the function does not reject, the rejection value does not match, or arguments are invalid.
50112
*/
51-
assert.rejects = (fn, error, message) => XTestFrame.rejects(suiteContext, assert.rejects, fn, error, message);
113+
assert.rejects = function rejects(fn, error, message) {
114+
switch (arguments.length) {
115+
case 0:
116+
case 1:
117+
throw new Error('expected fn and error arguments, but got too few arguments');
118+
case 2:
119+
if (!(fn instanceof Function)) {
120+
throw new Error(`unexpected fn, expected Function but got "${fn}"`);
121+
}
122+
if (!(error instanceof RegExp)) {
123+
throw new Error(`unexpected error, expected RegExp but got "${error}"`);
124+
}
125+
return XTestFrame.rejects(suiteContext, assert.rejects, fn, error);
126+
case 3:
127+
if (!(fn instanceof Function)) {
128+
throw new Error(`unexpected fn, expected Function but got "${fn}"`);
129+
}
130+
if (!(error instanceof RegExp)) {
131+
throw new Error(`unexpected error, expected RegExp but got "${error}"`);
132+
}
133+
if (typeof message !== 'string') {
134+
throw new Error(`unexpected message, expected string but got "${message}"`);
135+
}
136+
return XTestFrame.rejects(suiteContext, assert.rejects, fn, error, message);
137+
default:
138+
throw new Error('unexpected extra arguments');
139+
}
140+
};
52141

53142
/**
54143
* Load a new frame.

0 commit comments

Comments
 (0)