Skip to content

Commit dd989e9

Browse files
feat: add timeouts for creating test runners (#390)
* feat: add timeout for DOM evaluator * feat: add timeout when creating worker test runners
1 parent 1686678 commit dd989e9

File tree

4 files changed

+142
-89
lines changed

4 files changed

+142
-89
lines changed

packages/main/src/index.ts

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,27 @@ declare global {
66
}
77
}
88

9+
interface EvaluatorConfig {
10+
// The compiled user code, evaluated before the tests.
11+
source?: string;
12+
type: "dom" | "javascript" | "python";
13+
// TODO: move assetPath to RunnerConfig when making a major version bump.
14+
assetPath?: string;
15+
// The original user code, available for the tests to use.
16+
code?: { contents?: string; editableContents?: string };
17+
hooks?: {
18+
beforeAll?: string;
19+
beforeEach?: string;
20+
afterEach?: string;
21+
afterAll?: string;
22+
};
23+
loadEnzyme?: boolean;
24+
}
25+
26+
interface RunnerConfig {
27+
timeout?: number;
28+
}
29+
930
export class FCCTestRunner {
1031
#DOMRunner: DOMTestRunner | null;
1132
#javascriptRunner: WorkerTestRunner | null;
@@ -31,29 +52,10 @@ export class FCCTestRunner {
3152
}
3253
}
3354

34-
async createTestRunner({
35-
source,
36-
type,
37-
code,
38-
assetPath,
39-
hooks,
40-
loadEnzyme,
41-
}: {
42-
// The compiled user code, evaluated before the tests.
43-
source?: string;
44-
type: "dom" | "javascript" | "python";
45-
// TODO: can we avoid using `assetPath` and use `import.meta.url` instead?
46-
assetPath?: string;
47-
// The original user code, available for the tests to use.
48-
code?: { contents?: string; editableContents?: string };
49-
hooks?: {
50-
beforeAll?: string;
51-
beforeEach?: string;
52-
afterEach?: string;
53-
afterAll?: string;
54-
};
55-
loadEnzyme?: boolean;
56-
}) {
55+
async createTestRunner(
56+
{ source, type, code, assetPath, hooks, loadEnzyme }: EvaluatorConfig,
57+
{ timeout }: RunnerConfig = { timeout: 20000 },
58+
) {
5759
let testRunner: DOMTestRunner | WorkerTestRunner | null = null;
5860
// eslint-disable-next-line default-case
5961
switch (type) {
@@ -80,7 +82,7 @@ export class FCCTestRunner {
8082
break;
8183
}
8284

83-
await testRunner.init({ code, source, loadEnzyme, hooks });
85+
await testRunner.init({ code, source, loadEnzyme, hooks }, timeout);
8486

8587
return testRunner;
8688
}

packages/main/src/test-runner.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
import { post } from "./awaitable-post";
1616

1717
interface Runner {
18-
init(opts?: InitOptions): Promise<void>;
18+
init(opts?: InitOptions, timeout?: number): Promise<void>;
1919
// Note: timeouts are currently ignored in the FrameRunner, since the purpose
2020
// is to stop evaluation if it is looping indefinitely, but any abort
2121
// mechanism (e.g. Promise.race or AbortController) would not get called in
@@ -92,7 +92,7 @@ export class DOMTestRunner implements Runner {
9292
}
9393

9494
// Rather than trying to create an async constructor, we'll use an init method
95-
async init(opts: InitTestFrameOptions) {
95+
async init(opts: InitTestFrameOptions, timeout?: number) {
9696
const { hooks } = opts;
9797
const hooksScript = hooks?.beforeAll
9898
? `<script id='${TEST_EVALUATOR_HOOKS_ID}'>
@@ -102,9 +102,14 @@ ${hooks.beforeAll}
102102

103103
this.#afterAll = hooks?.afterAll;
104104

105-
const isReady = new Promise((resolve) => {
105+
const isReady = new Promise((resolve, reject) => {
106+
const timerId = setTimeout(
107+
() => reject(Error("Timed out waiting for the test frame to load")),
108+
timeout,
109+
);
106110
const listener = () => {
107111
this.#testEvaluator.removeEventListener("load", listener);
112+
clearTimeout(timerId);
108113
resolve(true);
109114
};
110115

@@ -181,6 +186,7 @@ ${opts.source}`;
181186
export class WorkerTestRunner implements Runner {
182187
#testEvaluator: Worker;
183188
#opts: InitWorkerOptions | null = null;
189+
#timeout?: number;
184190
#scriptUrl = "";
185191

186192
#createTestEvaluator({ assetPath, script }: RunnerConfig) {
@@ -192,24 +198,41 @@ export class WorkerTestRunner implements Runner {
192198
this.#testEvaluator = this.#createTestEvaluator(config);
193199
}
194200

195-
async init(opts: InitWorkerOptions) {
201+
async init(opts: InitWorkerOptions, timeout?: number) {
196202
this.#opts = opts;
203+
this.#timeout = timeout;
197204
const msg: InitEvent<InitWorkerOptions>["data"] = {
198205
type: "init",
199206
value: opts,
200207
};
201-
await post({
208+
209+
let timerId;
210+
const isFrozen = new Promise<void>((_resolve, reject) => {
211+
timerId = setTimeout(
212+
() =>
213+
reject(
214+
new Error("Timed out waiting for the test worker to initialize"),
215+
),
216+
timeout,
217+
);
218+
});
219+
220+
const response = post({
202221
messenger: this.#testEvaluator,
203222
message: msg,
204223
});
224+
225+
await Promise.race([response, isFrozen]);
226+
227+
clearTimeout(timerId);
205228
}
206229

207230
async #recreateRunner() {
208231
if (!this.#opts || !this.#scriptUrl) {
209232
throw new Error("WorkerTestRunner not initialized");
210233
} else {
211234
this.#testEvaluator = new Worker(this.#scriptUrl);
212-
await this.init(this.#opts);
235+
await this.init(this.#opts, this.#timeout);
213236
}
214237
}
215238

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

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,34 @@ describe("Test Runner", () => {
2424
});
2525
});
2626

27+
it("should throw if createTestRunner times out while creating a DOM runner", async () => {
28+
await expect(() =>
29+
page.evaluate(async () => {
30+
await window.FCCTestRunner.createTestRunner(
31+
{ type: "dom" },
32+
{ timeout: 0 },
33+
);
34+
}),
35+
).rejects.toThrow("Timed out waiting for the test frame to load");
36+
});
37+
38+
it("should throw if createTestRunner times out while creating a Worker runner", async () => {
39+
await expect(() =>
40+
page.evaluate(async () => {
41+
await window.FCCTestRunner.createTestRunner(
42+
{ type: "javascript" },
43+
{ timeout: 0 },
44+
);
45+
}),
46+
).rejects.toThrow("Timed out waiting for the test worker to initialize");
47+
});
48+
49+
// Worker runners are not timing out yet, but if they start to we'll need this
50+
// test to help debug
51+
it.todo(
52+
"should throw if createTestRunner times out while creating a worker runner",
53+
);
54+
2755
it("should add a FCCTestRunner to the window object", async () => {
2856
const sandbox = await page.evaluate(() => window.FCCTestRunner);
2957

@@ -1148,19 +1176,12 @@ assert(mocked.find('.greeting').length === 1);
11481176
});
11491177

11501178
describe("Python evaluator", () => {
1151-
beforeAll(async () => {
1152-
await page.evaluate(async () => {
1153-
await window.FCCTestRunner.createTestRunner({
1154-
type: "python",
1155-
});
1156-
});
1157-
});
11581179
it("should run tests after evaluating the source supplied to the runner", async () => {
11591180
const source = `def get_five():
11601181
return 5`;
11611182
const result = await page.evaluate(async (source) => {
1162-
const runner = window.FCCTestRunner.getRunner("python");
1163-
await runner?.init({
1183+
const runner = await window.FCCTestRunner.createTestRunner({
1184+
type: "python",
11641185
source,
11651186
});
11661187
return runner?.runTest(
@@ -1177,15 +1198,15 @@ test: () => assert.equal(runPython('get_five()'), 5),
11771198
const source = `def get_five():
11781199
return 5`;
11791200
await page.evaluate(async (source) => {
1180-
const runner = window.FCCTestRunner.getRunner("python");
1181-
await runner?.init({
1201+
await window.FCCTestRunner.createTestRunner({
1202+
type: "python",
11821203
source,
11831204
});
11841205
}, source);
11851206

11861207
const result = await page.evaluate(async () => {
11871208
const runner = window.FCCTestRunner.getRunner("python");
1188-
await runner?.init({});
1209+
await runner?.init({}, 1000);
11891210
return runner?.runTest(
11901211
`({
11911212
test: () => assert.equal(runPython('get_five()'), 5),
@@ -1205,8 +1226,9 @@ test: () => assert.equal(runPython('get_five()'), 5),
12051226

12061227
it("should set __name__ to __main__ when running tests", async () => {
12071228
const result = await page.evaluate(async () => {
1208-
const runner = window.FCCTestRunner.getRunner("python");
1209-
await runner?.init({});
1229+
const runner = await window.FCCTestRunner.createTestRunner({
1230+
type: "python",
1231+
});
12101232
return runner?.runTest(
12111233
`({
12121234
test: () => assert.equal(runPython('__name__'), '__main__'),
@@ -1219,8 +1241,8 @@ test: () => assert.equal(runPython('__name__'), '__main__'),
12191241

12201242
it("should handle js-only tests", async () => {
12211243
const result = await page.evaluate(async () => {
1222-
const runner = window.FCCTestRunner.getRunner("python");
1223-
await runner?.init({
1244+
const runner = await window.FCCTestRunner.createTestRunner({
1245+
type: "python",
12241246
code: {
12251247
contents: "# wrong comment for test",
12261248
},
@@ -1243,8 +1265,9 @@ test: () => assert.equal(runPython('__name__'), '__main__'),
12431265

12441266
it("should reject testStrings that evaluate to an invalid object", async () => {
12451267
const result = await page.evaluate(async () => {
1246-
const runner = window.FCCTestRunner.getRunner("python");
1247-
await runner?.init({});
1268+
const runner = await window.FCCTestRunner.createTestRunner({
1269+
type: "python",
1270+
});
12481271
return runner?.runTest(`({ invalid: 'test' })`);
12491272
});
12501273

@@ -1262,13 +1285,14 @@ test: () => assert.equal(runPython('__name__'), '__main__'),
12621285

12631286
it("should be able to test with mock input", async () => {
12641287
const result = await page.evaluate(async () => {
1265-
const runner = window.FCCTestRunner.getRunner("python");
1266-
await runner?.init({
1288+
const runner = await window.FCCTestRunner.createTestRunner({
1289+
type: "python",
12671290
source: `
12681291
first = input()
12691292
second = input()
12701293
`,
12711294
});
1295+
12721296
return runner?.runTest(`({
12731297
input: ["argle", "bargle"],
12741298
test: () => assert.equal(runPython('first + second'), "arglebargle")
@@ -1280,12 +1304,13 @@ second = input()
12801304

12811305
it("should make user code available to the python code as the _code variable", async () => {
12821306
const result = await page.evaluate(async () => {
1283-
const runner = window.FCCTestRunner.getRunner("python");
1284-
await runner?.init({
1307+
const runner = await window.FCCTestRunner.createTestRunner({
1308+
type: "python",
12851309
code: {
12861310
contents: "test = 'value'",
12871311
},
12881312
});
1313+
12891314
return runner?.runTest(`({
12901315
test: () => assert.equal(runPython('_code'), "test = 'value'")
12911316
})`);
@@ -1296,8 +1321,9 @@ second = input()
12961321

12971322
it("should make the AST helper available to the python code as _Node", async () => {
12981323
const result = await page.evaluate(async () => {
1299-
const runner = window.FCCTestRunner.getRunner("python");
1300-
await runner?.init({});
1324+
const runner = await window.FCCTestRunner.createTestRunner({
1325+
type: "python",
1326+
});
13011327
return runner?.runTest(`({
13021328
test: () => assert.equal(runPython('_Node("x = 1").get_variable("x")'), 1)
13031329
})`);
@@ -1308,15 +1334,16 @@ second = input()
13081334

13091335
it("should have access to _code in the beforeEach hook", async () => {
13101336
const result = await page.evaluate(async () => {
1311-
const runner = window.FCCTestRunner.getRunner("python");
1312-
await runner?.init({
1337+
const runner = await window.FCCTestRunner.createTestRunner({
1338+
type: "python",
13131339
code: {
13141340
contents: "test = 'value'",
13151341
},
13161342
hooks: {
13171343
beforeEach: "assert.equal(runPython('_code'), \"test = 'value'\")",
13181344
},
13191345
});
1346+
13201347
return runner?.runTest(`({
13211348
test: () => {}
13221349
})`);
@@ -1327,12 +1354,13 @@ second = input()
13271354

13281355
it("should have access to runPython in the beforeEach hook", async () => {
13291356
const result = await page.evaluate(async () => {
1330-
const runner = window.FCCTestRunner.getRunner("python");
1331-
await runner?.init({
1357+
const runner = await window.FCCTestRunner.createTestRunner({
1358+
type: "python",
13321359
hooks: {
13331360
beforeEach: "assert.equal(runPython('1 + 1'), 2)",
13341361
},
13351362
});
1363+
13361364
return runner?.runTest("");
13371365
});
13381366

@@ -1341,8 +1369,9 @@ second = input()
13411369

13421370
it("should return error types if the python code raises an exception", async () => {
13431371
const result = await page.evaluate(async () => {
1344-
const runner = window.FCCTestRunner.getRunner("python");
1345-
await runner?.init({});
1372+
const runner = await window.FCCTestRunner.createTestRunner({
1373+
type: "python",
1374+
});
13461375
return runner?.runTest(`({
13471376
test: () => assert.equal(runPython('1 + "1"'), 2)
13481377
})`);
@@ -1367,8 +1396,8 @@ second = input()
13671396
pattern = re.compile('l+')
13681397
`;
13691398
const result = await page.evaluate(async (source) => {
1370-
const runner = window.FCCTestRunner.getRunner("python");
1371-
await runner?.init({
1399+
const runner = await window.FCCTestRunner.createTestRunner({
1400+
type: "python",
13721401
source,
13731402
});
13741403
return runner?.runTest(`({
@@ -1394,8 +1423,8 @@ import re
13941423
13951424
pattern = re.compile('l+')`;
13961425
const result = await page.evaluate(async (source) => {
1397-
const runner = window.FCCTestRunner.getRunner("python");
1398-
await runner?.init({
1426+
const runner = await window.FCCTestRunner.createTestRunner({
1427+
type: "python",
13991428
source,
14001429
});
14011430
// Since the comparison includes a PyProxy object, that will be

0 commit comments

Comments
 (0)