Skip to content

Commit e6f84c4

Browse files
ojeytonwilliamsojeytonwilliams
authored
feat: throw in afterEach (#356)
* refactor: put afterEach at end of runTest It's simpler this way. * feat: fail test if afterEach throws If the afterEach fails, something has definitely gone wrong, so it's better that we know what it is and can fix it.
1 parent 1f299cf commit e6f84c4

5 files changed

Lines changed: 97 additions & 60 deletions

File tree

‎packages/dom-evaluator/src/dom-test-evaluator.ts‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type {
99
Fail,
1010
CodeEvent,
1111
TestEvent,
12+
TestError,
1213
InitEvent,
1314
Pass,
1415
InitTestFrameOptions,
@@ -53,6 +54,17 @@ export class DOMTestEvaluator implements TestEvaluator {
5354
#runTest?: TestEvaluator["runTest"];
5455
#proxyConsole: ProxyConsole;
5556

57+
#createErrorResponse(error: TestError) {
58+
return {
59+
err: {
60+
message: error.message,
61+
stack: error.stack,
62+
...(!!error.expected && { expected: error.expected }),
63+
...(!!error.actual && { actual: error.actual }),
64+
},
65+
};
66+
}
67+
5668
constructor(
5769
proxyConsole: ProxyConsole = new ProxyConsole(globalThis.console, format),
5870
) {
@@ -143,36 +155,28 @@ ${rawTest}`);
143155
await test();
144156
}
145157

146-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
147-
148158
return { pass: true, ...this.#proxyConsole.flush() };
149159
} catch (err) {
150160
this.#proxyConsole.off();
151161
console.error(err);
152162

153-
try {
154-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
155-
} catch (afterEachErr) {
156-
// Even though we're returning the original test error, we still
157-
// want to log for debugging purposes.
158-
console.error("Error in afterEach hook:", afterEachErr);
159-
}
160-
161163
const error = err as Fail["err"];
162164
// To provide useful debugging information when debugging the tests, we
163165
// have to extract the message, stack and, if they exist, expected and
164166
// actual before returning
165167
return {
166-
err: {
167-
message: error.message,
168-
stack: error.stack,
169-
...(!!error.expected && { expected: error.expected }),
170-
...(!!error.actual && { actual: error.actual }),
171-
},
168+
...this.#createErrorResponse(error),
172169
...this.#proxyConsole.flush(),
173170
};
174171
} finally {
175172
this.#proxyConsole.off();
173+
174+
try {
175+
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
176+
} catch (afterEachErr) {
177+
// eslint-disable-next-line no-unsafe-finally
178+
return this.#createErrorResponse(afterEachErr as TestError);
179+
}
176180
}
177181
};
178182

‎packages/javascript-evaluator/src/javascript-test-evaluator.ts‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as curriculumHelpers from "../../helpers/lib";
55

66
import type {
77
TestEvaluator,
8+
TestError,
89
Fail,
910
InitEvent,
1011
TestEvent,
@@ -36,6 +37,17 @@ export class JavascriptTestEvaluator implements TestEvaluator {
3637
#runTest?: TestEvaluator["runTest"];
3738
#proxyConsole: ProxyConsole;
3839

40+
#createErrorResponse(error: TestError) {
41+
return {
42+
err: {
43+
message: error.message,
44+
stack: error.stack,
45+
...(!!error.expected && { expected: error.expected }),
46+
...(!!error.actual && { actual: error.actual }),
47+
},
48+
};
49+
}
50+
3951
constructor(
4052
proxyConsole: ProxyConsole = new ProxyConsole(globalThis.console, format),
4153
) {
@@ -72,34 +84,26 @@ ${test};`);
7284
}
7385
}
7486

75-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
76-
7787
return { pass: true, ...this.#proxyConsole.flush() };
7888
} catch (err: unknown) {
7989
this.#proxyConsole.off();
8090
console.error(err);
8191

82-
try {
83-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
84-
} catch (afterEachErr) {
85-
// Even though we're returning the original test error, we still
86-
// want to log for debugging purposes.
87-
console.error("Error in afterEach hook:", afterEachErr);
88-
}
89-
9092
const error = err as Fail["err"];
9193

9294
return {
93-
err: {
94-
message: error.message,
95-
stack: error.stack,
96-
...(!!error.expected && { expected: error.expected }),
97-
...(!!error.actual && { actual: error.actual }),
98-
},
95+
...this.#createErrorResponse(error),
9996
...this.#proxyConsole.flush(),
10097
};
10198
} finally {
10299
this.#proxyConsole.off();
100+
101+
try {
102+
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
103+
} catch (afterEachErr) {
104+
// eslint-disable-next-line no-unsafe-finally
105+
return this.#createErrorResponse(afterEachErr as TestError);
106+
}
103107
}
104108
};
105109
}

‎packages/python-evaluator/src/python-test-evaluator.ts‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
TestEvaluator,
1515
TestEvent,
1616
CodeEvent,
17+
TestError,
1718
} from "../../shared/src/interfaces/test-evaluator";
1819
import { ReadyEvent } from "../../shared/src/interfaces/test-runner";
1920
import { postCloneableMessage } from "../../shared/src/messages";
@@ -36,6 +37,20 @@ class PythonTestEvaluator implements TestEvaluator {
3637
#runTest?: TestEvaluator["runTest"];
3738
#proxyConsole: ProxyConsole;
3839

40+
#createErrorResponse(error: TestError) {
41+
const expected = serialize((error as { expected: unknown }).expected);
42+
const actual = serialize((error as { actual: unknown }).actual);
43+
return {
44+
err: {
45+
message: error.message,
46+
stack: error.stack,
47+
...(!!expected && { expected }),
48+
...(!!actual && { actual }),
49+
type: error.type,
50+
},
51+
};
52+
}
53+
3954
constructor(
4055
proxyConsole: ProxyConsole = new ProxyConsole(globalThis.console, format),
4156
) {
@@ -102,41 +117,32 @@ input = __fake_input
102117

103118
await eval(iifeTest);
104119

105-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
106-
107120
return { pass: true, ...this.#proxyConsole.flush() };
108121
} catch (err) {
109122
this.#proxyConsole.off();
110123
console.error(err);
111124

112-
try {
113-
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
114-
} catch (afterEachErr) {
115-
// Even though we're returning the original test error, we still
116-
// want to log for debugging purposes.
117-
console.error("Error in afterEach hook:", afterEachErr);
118-
}
119-
120125
const error = err as PythonError;
121126

122-
const expected = serialize((err as { expected: unknown }).expected);
123-
const actual = serialize((err as { actual: unknown }).actual);
124-
125127
// To provide useful debugging information when debugging the tests, we
126128
// have to extract the message, stack and, if they exist, expected and
127129
// actual before returning
128130
return {
129-
err: {
130-
message: error.message,
131-
stack: error.stack,
132-
...(!!expected && { expected }),
133-
...(!!actual && { actual }),
134-
type: error.type,
135-
},
131+
...this.#createErrorResponse(error),
136132
...this.#proxyConsole.flush(),
137133
};
138134
} finally {
139135
this.#proxyConsole.off();
136+
137+
try {
138+
if (opts.hooks?.afterEach) eval(opts.hooks.afterEach);
139+
} catch (afterEachErr) {
140+
// eslint-disable-next-line no-unsafe-finally
141+
return {
142+
...this.#createErrorResponse(afterEachErr as TestError),
143+
};
144+
}
145+
140146
__userGlobals.destroy();
141147
}
142148
};

‎packages/shared/src/interfaces/test-evaluator.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ export interface Pass extends Logged {
88
pass: true;
99
}
1010

11+
export interface TestError {
12+
message: string;
13+
stack?: string;
14+
// TODO: enforce string for expected and actual?
15+
expected?: unknown;
16+
actual?: unknown;
17+
type?: string;
18+
}
19+
1120
export interface Fail extends Logged {
12-
err: {
13-
message: string;
14-
stack?: string;
15-
// TODO: enforce string for expected and actual?
16-
expected?: unknown;
17-
actual?: unknown;
18-
type?: string;
19-
};
21+
err: TestError;
2022
}
2123

2224
export type TestEvent = MessageEvent<{ type: "test"; value: string }>;

‎packages/tests/integration-tests/index.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -556,6 +556,27 @@ for(let i = 0; i < 3; i++) {
556556
expect(result).toMatchObject({ err: { actual: 1 } });
557557
},
558558
);
559+
560+
it("should fail the test if the afterEach hook throws an error", async () => {
561+
const result = await page.evaluate(async (type) => {
562+
const runner = await window.FCCTestRunner.createTestRunner({
563+
type,
564+
hooks: {
565+
afterEach: "throw new Error('afterEach error')",
566+
},
567+
});
568+
569+
return runner.runTest("");
570+
}, type);
571+
572+
expect(result).toEqual({
573+
err: {
574+
message: "afterEach error",
575+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
576+
stack: expect.stringMatching("Error: afterEach error"),
577+
},
578+
});
579+
});
559580
});
560581

561582
describe.each([

0 commit comments

Comments
 (0)