Allow for assertions against intentional non-match (#705)

* Allow for assertions against intentional non-match

* Rename yml key for new test

* Address PR feedback

* Change asserted on property `errorName`

* Address PR feedback pt.2

* Rename error field

* Re-add empty line

* Change error checking code to work

* Fix ci

Co-authored-by: Pokey Rule <[email protected]>

Author: Will-Sommers

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", {"recordErrors": 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/errors.ts

@@ -54,3 +54,17 @@ export class ActionableError extends Error {
       });
   }
 }
+/**
+ * Throw this error if you have attempted to match based on a language scope but have not
+ * returned a match.
+ */
+export class NoContainingScopeError extends Error {
+  /**
+   *
+   * @param scopeType The scopeType for the failed match to show to the user
+   */
+  constructor(scopeType: string) {
+    super(`Couldn't find containing ${scopeType}.`);
+    this.name = "NoContainingScopeError";
+  }
+}

src/processTargets/modifiers/processModifier.ts

@@ -17,6 +17,7 @@ import {
 } from "../../typings/Types";
 import { processSurroundingPair } from "./surroundingPair";
 import { getNodeMatcher } from "../../languages/getNodeMatcher";
+import { NoContainingScopeError } from "../../errors";
 
 export type SelectionWithEditorWithContext = {
   selection: SelectionWithEditor;
@@ -99,7 +100,7 @@ function processScopeType(
   );
 
   if (result == null) {
-    throw new Error(`Couldn't find containing ${modifier.scopeType}`);
+    throw new NoContainingScopeError(modifier.scopeType);
   }
 
   return result;

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: {name: NoContainingScopeError}

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: {name: NoContainingScopeError}

src/test/suite/recorded.test.ts

@@ -106,17 +106,33 @@ async function runTest(file: string) {
   // Assert that recorded decorations are present
   checkMarks(fixture.initialState.marks, readableHatMap);
 
+  if (fixture.thrownError != null) {
+    await assert.rejects(
+      async () => {
+        await vscode.commands.executeCommand(
+          "cursorless.command",
+          fixture.command
+        );
+      },
+      (err: Error) => {
+        assert.strictEqual(err.name, fixture.thrownError?.name);
+        return true;
+      }
+    );
+    return;
+  }
+
   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 ThrownError = {
+  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. Undefined if we are testing a non-match(error) case. */
+  finalState?: TestCaseSnapshot;
+  /** Used to assert if an error has been thrown. */
+  thrownError?: ThrownError;
   returnValue: unknown;
   /** Inferred full targets added for context; not currently used in testing */
   fullTargets: Target[];
@@ -46,7 +53,8 @@ export class TestCase {
   languageId: string;
   fullTargets: Target[];
   initialState: TestCaseSnapshot | null = null;
-  finalState: TestCaseSnapshot | null = null;
+  finalState?: TestCaseSnapshot;
+  thrownError?: ThrownError;
   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,
     };
     return serialize(fixture);
   }

src/testUtil/TestCaseRecorder.ts

@@ -47,12 +47,18 @@ interface RecordTestCaseCommandArg {
    * Whether to flash a background for calibrating a video recording
    */
   showCalibrationDisplay?: boolean;
+
+  /**
+   * Whether we should record a tests which yield errors in addition to tests
+   * which do not error.
+   */
+  recordErrors?: boolean;
 }
 
 export class TestCaseRecorder {
   private active: boolean = false;
   private workspacePath: string | null;
-  private workSpaceFolder: string | null;
+  private workspaceName: string | null;
   private fixtureRoot: string | null;
   private targetDirectory: string | null = null;
   private testCase: TestCase | null = null;
@@ -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,
   });
@@ -74,7 +81,7 @@ export class TestCaseRecorder {
         ? graph.extensionContext.extensionPath
         : vscode.workspace.workspaceFolders?.[0].uri.path ?? null;
 
-    this.workSpaceFolder = this.workspacePath
+    this.workspaceName = this.workspacePath
       ? path.basename(this.workspacePath)
       : null;
 
@@ -168,6 +175,7 @@ export class TestCaseRecorder {
       isSilent = false,
       extraSnapshotFields = [],
       showCalibrationDisplay = false,
+      recordErrors: 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(
@@ -286,11 +295,13 @@ export class TestCaseRecorder {
 
   private async promptSubdirectory(): Promise<string | undefined> {
     if (
-      this.workspacePath == null ||
+      this.workspaceName == null ||
       this.fixtureRoot == null ||
-      this.workSpaceFolder !== "cursorless-vscode"
+      !["cursorless-vscode", "cursorless"].includes(this.workspaceName)
     ) {
-      throw new Error("Can't prompt for subdirectory");
+      throw new Error(
+        '"Cursorless record" must be run from within cursorless directory'
+      );
     }
 
     const subdirectories = walkDirsSync(this.fixtureRoot).concat("/");
@@ -351,8 +362,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() {