Skip to content

Commit 043a1dd

Browse files
authored
Fix bug with injected ide in test case recorder (#1359)
This PR fixes a bug with our test case recorder. If an exception is thrown during command processing before we inject a spy ide during `TestCaseRecorder.preCommandHook`, we don't set the `originalIde` member variable on `TestCaseRecorder`. During `TestCaseRecorder.finallyHook`, we unconditionally call `injectIde(this.originalIde)` even in the case of an error, which means that we will inject an `undefined` ide, so Cursorless becomes completely broken, because `ide()` is now `undefined`, so we get an error about not having injected ide for every single subsequent command This PR just checks that `this.originalIde != null` before trying to inject it ## Checklist - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [ ] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [ ] I have not broken the cheatsheet
1 parent dde5802 commit 043a1dd

File tree

2 files changed

+61
-3
lines changed

2 files changed

+61
-3
lines changed

packages/cursorless-engine/src/testCaseRecorder/TestCaseRecorder.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,9 @@ export class TestCaseRecorder {
432432
}
433433

434434
finallyHook() {
435-
injectIde(this.originalIde!);
435+
if (this.originalIde != null) {
436+
injectIde(this.originalIde);
437+
}
436438
this.spyIde = undefined;
437439
this.originalIde = undefined;
438440

packages/cursorless-vscode-e2e/src/suite/testCaseRecorder.test.ts

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
ActionType,
23
getFixturePath,
34
getRecordedTestsDirPath,
45
HatTokenMap,
@@ -23,6 +24,7 @@ suite("testCaseRecorder", async function () {
2324

2425
test("no args", testCaseRecorderNoArgs);
2526
test("path arg", testCaseRecorderPathArg);
27+
test("graceful error", testCaseRecorderGracefulError);
2628
});
2729

2830
async function testCaseRecorderNoArgs() {
@@ -56,22 +58,76 @@ async function testCaseRecorderPathArg() {
5658
}
5759
}
5860

61+
async function testCaseRecorderGracefulError() {
62+
const { hatTokenMap } = (await getCursorlessApi()).testHelpers!;
63+
const tmpdir = path.join(os.tmpdir(), crypto.randomBytes(16).toString("hex"));
64+
await mkdir(tmpdir, { recursive: true });
65+
66+
try {
67+
await startRecording({
68+
directory: tmpdir,
69+
});
70+
71+
try {
72+
await runCursorlessCommand({
73+
version: 4,
74+
action: { name: "badActionName" as ActionType },
75+
targets: [
76+
{
77+
type: "primitive",
78+
mark: {
79+
type: "cursor",
80+
},
81+
},
82+
],
83+
usePrePhraseSnapshot: false,
84+
spokenForm: "bad command",
85+
});
86+
} catch (err) {
87+
// Ignore error
88+
}
89+
90+
await initalizeEditor(hatTokenMap);
91+
await takeHarp();
92+
await stopRecording();
93+
await checkRecordedTest(tmpdir);
94+
} finally {
95+
await rm(tmpdir, { recursive: true, force: true });
96+
}
97+
}
98+
5999
async function runAndCheckTestCaseRecorder(
60100
hatTokenMap: HatTokenMap,
61101
tmpdir: string,
62102
...extraArgs: unknown[]
63103
) {
104+
await initalizeEditor(hatTokenMap);
105+
await startRecording(...extraArgs);
106+
await takeHarp();
107+
await stopRecording();
108+
await checkRecordedTest(tmpdir);
109+
}
110+
111+
async function initalizeEditor(hatTokenMap: HatTokenMap) {
64112
const editor = await openNewEditor("hello world");
65113

66114
editor.selections = [new vscode.Selection(0, 11, 0, 11)];
67115

68116
await hatTokenMap.allocateHats();
117+
}
69118

119+
async function startRecording(...extraArgs: unknown[]) {
70120
await vscode.commands.executeCommand(
71121
"cursorless.recordTestCase",
72122
...extraArgs,
73123
);
124+
}
74125

126+
async function stopRecording() {
127+
await vscode.commands.executeCommand("cursorless.recordTestCase");
128+
}
129+
130+
async function takeHarp() {
75131
await runCursorlessCommand({
76132
version: 4,
77133
action: { name: "setSelection" },
@@ -88,9 +144,9 @@ async function runAndCheckTestCaseRecorder(
88144
usePrePhraseSnapshot: false,
89145
spokenForm: "take harp",
90146
});
147+
}
91148

92-
await vscode.commands.executeCommand("cursorless.recordTestCase");
93-
149+
async function checkRecordedTest(tmpdir: string) {
94150
const paths = await readdir(tmpdir);
95151
assert.lengthOf(paths, 1);
96152

0 commit comments

Comments
 (0)