Allow for assertions against intentional non-match

docs/contributing/test-case-recorder.md

@@ -13,13 +13,19 @@ command run, and the final state, all in the form of a yaml document. See
 1.  Add a voice command for recording to your personal talon files:
     - `cursorless record: user.vscode("cursorless.recordTestCase")`
     - We don't want to commit this so add it to your own repository.
-1.  If you'd like to be able to do tests which check the navigation map, you should also add the following to your personal talon files:
+1.  If you'd like to be able to record tests which check the navigation map, please add the following to your personal talon files:
 
     - https://github.com/pokey/pokey_talon/blob/9298c25dd6d28fd9fcf5ed39f305bc6b93e5f229/apps/vscode/vscode.talon#L468
     - https://github.com/pokey/pokey_talon/blob/49643bfa8f62cbec18b5ddad1658f5a28785eb01/apps/vscode/vscode.py#L203-L205
 
     It is quite unlikely you'll need this second step. Most tests don't check the navigation map.
 
+1.  If you'd like to be able to record tests which assert on non-matches, please add another command to your personal talon files. See the two files links above for context. Add the command below to your to your `vscode.py` and ensure that there is a matching Talon command.
+
+```
+  actions.user.vscode_with_plugin("cursorless.recordTestCase", {"isErrorTest": True})
+```
+
 ## Recording new tests
 
 1. Start debugging (F5)

src/core/commandRunner/CommandRunner.ts

@@ -106,8 +106,6 @@ export default class CommandRunner {
         getNodeAtLocation: this.graph.getNodeAtLocation,
       };
 
-      const selections = processTargets(processedTargetsContext, targets);
-
       if (this.graph.testCaseRecorder.isActive()) {
         const context = {
           targets,
@@ -122,6 +120,8 @@ export default class CommandRunner {
         );
       }
 
+      const selections = processTargets(processedTargetsContext, targets);
+
       const {
         returnValue,
         thatMark: newThatMark,
@@ -137,7 +137,7 @@ export default class CommandRunner {
 
       return returnValue;
     } catch (e) {
-      this.graph.testCaseRecorder.commandErrorHook();
+      await this.graph.testCaseRecorder.commandErrorHook(e as Error);
       const err = e as Error;
       if ((err as Error).name === "ActionableError") {
         (err as ActionableError).showErrorMessage();

src/test/suite/fixtures/recorded/nonMatchErrors/takeFunk.yml

@@ -0,0 +1,19 @@
+languageId: ruby
+command:
+  version: 1
+  spokenForm: take funk
+  action: setSelection
+  targets:
+    - type: primitive
+      modifier: {type: containingScope, scopeType: namedFunction, includeSiblings: false}
+initialState:
+  documentContents: |2
+      [1, 2, 3, [4, 5, 6]] 
+  selections:
+    - anchor: {line: 0, character: 11}
+      active: {line: 0, character: 11}
+  marks: {}
+finalState: null
+returnValue: null
+fullTargets: [{type: primitive, mark: {type: cursor}, selectionType: token, position: contents, insideOutsideType: inside, modifier: {type: containingScope, scopeType: namedFunction, includeSiblings: false}, isImplicit: false}]
+thrownError: Error

src/test/suite/fixtures/recorded/nonMatchErrors/takeList.yml

@@ -0,0 +1,19 @@
+languageId: ruby
+command:
+  version: 1
+  spokenForm: take list
+  action: setSelection
+  targets:
+    - type: primitive
+      modifier: {type: containingScope, scopeType: list, includeSiblings: false}
+initialState:
+  documentContents: |2
+      [1, 2, 3, [4, 5, 6]] 
+  selections:
+    - anchor: {line: 0, character: 0}
+      active: {line: 0, character: 0}
+  marks: {}
+finalState: null
+returnValue: null
+fullTargets: [{type: primitive, mark: {type: cursor}, selectionType: token, position: contents, insideOutsideType: inside, modifier: {type: containingScope, scopeType: list, includeSiblings: false}, isImplicit: false}]
+thrownError: Error

src/test/suite/recorded.test.ts

@@ -105,18 +105,25 @@ async function runTest(file: string) {
 
   // Assert that recorded decorations are present
   checkMarks(fixture.initialState.marks, readableHatMap);
-
+  if (fixture.thrownError) {
+    return assert.rejects(async () => {
+      await vscode.commands.executeCommand(
+        "cursorless.command",
+        fixture.command
+      );
+    });
+  }
   const returnValue = await vscode.commands.executeCommand(
     "cursorless.command",
     fixture.command
   );
 
   const marks =
-    fixture.finalState.marks == null
+    fixture.finalState!.marks == null
       ? undefined
       : marksToPlainObject(
           extractTargetedMarks(
-            Object.keys(fixture.finalState.marks) as string[],
+            Object.keys(fixture.finalState!.marks) as string[],
             readableHatMap
           )
         );

src/testUtil/TestCase.ts

@@ -26,6 +26,10 @@ export type TestCaseContext = {
   hatTokenMap: ReadOnlyHatMap;
 };
 
+export type ErrorInfo = {
+  name: string;
+};
+
 export type TestCaseFixture = {
   languageId: string;
   command: TestCaseCommand;
@@ -36,7 +40,10 @@ export type TestCaseFixture = {
   marksToCheck?: string[];
 
   initialState: TestCaseSnapshot;
-  finalState: TestCaseSnapshot;
+  /** The final state after a command is issued. If we are testing a non-match(error) case this is null. */
+  finalState: TestCaseSnapshot | null;
+  /** Used to assert if an error has been thrown. */
+  thrownError?: string;
   returnValue: unknown;
   /** Inferred full targets added for context; not currently used in testing */
   fullTargets: Target[];
@@ -47,6 +54,7 @@ export class TestCase {
   fullTargets: Target[];
   initialState: TestCaseSnapshot | null = null;
   finalState: TestCaseSnapshot | null = null;
+  thrownError: ErrorInfo | null = null;
   returnValue: unknown = null;
   targetKeys: string[];
   private _awaitingFinalMarkInfo: boolean;
@@ -132,7 +140,10 @@ export class TestCase {
   }
 
   toYaml() {
-    if (this.initialState == null || this.finalState == null) {
+    if (
+      this.initialState == null ||
+      (this.finalState == null && this.thrownError == null)
+    ) {
       throw Error("Two snapshots must be taken before serializing");
     }
     const fixture: TestCaseFixture = {
@@ -143,6 +154,7 @@ export class TestCase {
       finalState: this.finalState,
       returnValue: this.returnValue,
       fullTargets: this.fullTargets,
+      thrownError: this.thrownError?.name,
     };
     return serialize(fixture);
   }

src/testUtil/TestCaseRecorder.ts

@@ -47,6 +47,12 @@ interface RecordTestCaseCommandArg {
    * Whether to flash a background for calibrating a video recording
    */
   showCalibrationDisplay?: boolean;
+
+  /**
+   * Whether we should record a test which yields an error
+   */
+
+  isErrorTest?: boolean;
 }
 
 export class TestCaseRecorder {
@@ -62,6 +68,7 @@ export class TestCaseRecorder {
   private startTimestamp?: bigint;
   private extraSnapshotFields?: ExtraSnapshotField[];
   private paused: boolean = false;
+  private isErrorTest: boolean = false;
   private calibrationStyle = vscode.window.createTextEditorDecorationType({
     backgroundColor: CALIBRATION_DISPLAY_BACKGROUND_COLOR,
   });
@@ -168,6 +175,7 @@ export class TestCaseRecorder {
       isSilent = false,
       extraSnapshotFields = [],
       showCalibrationDisplay = false,
+      isErrorTest = false,
     } = arg ?? {};
 
     if (directory != null) {
@@ -187,6 +195,7 @@ export class TestCaseRecorder {
       this.isHatTokenMapTest = isHatTokenMapTest;
       this.isSilent = isSilent;
       this.extraSnapshotFields = extraSnapshotFields;
+      this.isErrorTest = isErrorTest;
       this.paused = false;
 
       vscode.window.showInformationMessage(
@@ -248,10 +257,9 @@ export class TestCaseRecorder {
       // command for a navigation map test
       return;
     }
+    await this.testCase!.recordFinalState(returnValue);
 
-    await this.testCase.recordFinalState(returnValue);
-
-    if (this.testCase.awaitingFinalMarkInfo) {
+    if (this.testCase!.awaitingFinalMarkInfo) {
       // We don't finish the test case here in the case of a navigation map
       // test because we'll do it after we get the follow up command indicating
       // which marks we wanted to track
@@ -351,8 +359,13 @@ export class TestCaseRecorder {
     return filePath;
   }
 
-  commandErrorHook() {
-    this.testCase = null;
+  async commandErrorHook(error: Error) {
+    if (this.isErrorTest && this.testCase) {
+      this.testCase.thrownError = { name: error.name };
+      await this.finishTestCase();
+    } else {
+      this.testCase = null;
+    }
   }
 
   dispose() {