Convert extensions/ql-vscode/src/vscode-utils/external-files.ts to call typed commands

Author: robertbrignull

extensions/ql-vscode/src/query-history/query-history-manager.ts

@@ -680,7 +680,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (singleItem.completedQuery.logFileLocation) {
-      await tryOpenExternalFile(singleItem.completedQuery.logFileLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        singleItem.completedQuery.logFileLocation,
+      );
     } else {
       void showAndLogWarningMessage("No log file available");
     }
@@ -797,7 +800,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogLocation) {
-      await tryOpenExternalFile(finalSingleItem.evalLogLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        finalSingleItem.evalLogLocation,
+      );
     } else {
       this.warnNoEvalLogs();
     }
@@ -822,7 +828,10 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogSummaryLocation) {
-      await tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
+      await tryOpenExternalFile(
+        this.app.commands,
+        finalSingleItem.evalLogSummaryLocation,
+      );
       return;
     }
 
@@ -965,7 +974,10 @@ export class QueryHistoryManager extends DisposableObject {
     const query = finalSingleItem.completedQuery.query;
     const hasInterpretedResults = query.canHaveInterpretedResults();
     if (hasInterpretedResults) {
-      await tryOpenExternalFile(query.resultsPaths.interpretedResultsPath);
+      await tryOpenExternalFile(
+        this.app.commands,
+        query.resultsPaths.interpretedResultsPath,
+      );
     } else {
       const label = this.labelProvider.getLabel(finalSingleItem);
       void showAndLogInformationMessage(
@@ -994,11 +1006,11 @@ export class QueryHistoryManager extends DisposableObject {
     }
     const query = finalSingleItem.completedQuery.query;
     if (await query.hasCsv()) {
-      void tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(this.app.commands, query.csvPath);
       return;
     }
     if (await query.exportCsvResults(this.qs.cliServer, query.csvPath)) {
-      void tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(this.app.commands, query.csvPath);
     }
   }
 
@@ -1022,6 +1034,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     await tryOpenExternalFile(
+      this.app.commands,
       await finalSingleItem.completedQuery.query.ensureCsvAlerts(
         this.qs.cliServer,
         this.dbm,
@@ -1049,6 +1062,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     await tryOpenExternalFile(
+      this.app.commands,
       await finalSingleItem.completedQuery.query.ensureDilPath(
         this.qs.cliServer,
       ),

extensions/ql-vscode/src/vscode-utils/external-files.ts

@@ -1,12 +1,16 @@
-import { commands, Uri, window } from "vscode";
+import { Uri, window } from "vscode";
+import { AppCommandManager } from "../common/commands";
 import {
   showAndLogExceptionWithTelemetry,
   showBinaryChoiceDialog,
 } from "../helpers";
 import { redactableError } from "../pure/errors";
 import { asError, getErrorMessage, getErrorStack } from "../pure/helpers-pure";
 
-export async function tryOpenExternalFile(fileLocation: string) {
+export async function tryOpenExternalFile(
+  CommandManager: AppCommandManager,
+  fileLocation: string,
+) {
   const uri = Uri.file(fileLocation);
   try {
     await window.showTextDocument(uri, { preview: false });
@@ -25,7 +29,7 @@ the file in the file explorer and dragging it into the workspace.`,
       );
       if (res) {
         try {
-          await commands.executeCommand("revealFileInOS", uri);
+          await CommandManager.execute("revealFileInOS", uri);
         } catch (e) {
           void showAndLogExceptionWithTelemetry(
             redactableError(

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/vscode-utils/external-files.test.ts

@@ -1,5 +1,6 @@
 import * as vscode from "vscode";
 import { tryOpenExternalFile } from "../../../../../src/vscode-utils/external-files";
+import { createMockCommandManager } from "../../../../__mocks__/commandsMock";
 import { mockedObject } from "../../../utils/mocking.helpers";
 
 describe("tryOpenExternalFile", () => {
@@ -9,9 +10,6 @@ describe("tryOpenExternalFile", () => {
   let showInformationMessageSpy: jest.SpiedFunction<
     typeof vscode.window.showInformationMessage
   >;
-  let executeCommandSpy: jest.SpiedFunction<
-    typeof vscode.commands.executeCommand
-  >;
 
   beforeEach(() => {
     showTextDocumentSpy = jest
@@ -20,46 +18,52 @@ describe("tryOpenExternalFile", () => {
     showInformationMessageSpy = jest
       .spyOn(vscode.window, "showInformationMessage")
       .mockResolvedValue(undefined);
-    executeCommandSpy = jest
-      .spyOn(vscode.commands, "executeCommand")
-      .mockResolvedValue(undefined);
   });
 
   it("should open an external file", async () => {
-    await tryOpenExternalFile("xxx");
+    const executeCommand = jest.fn();
+    const commandManager = createMockCommandManager({ executeCommand });
+
+    await tryOpenExternalFile(commandManager, "xxx");
     expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
     expect(showTextDocumentSpy).toHaveBeenCalledWith(
       vscode.Uri.file("xxx"),
       expect.anything(),
     );
-    expect(executeCommandSpy).not.toBeCalled();
+    expect(executeCommand).not.toBeCalled();
   });
 
   [
     "too large to open",
     "Files above 50MB cannot be synchronized with extensions",
   ].forEach((msg) => {
     it(`should fail to open a file because "${msg}" and open externally`, async () => {
+      const executeCommand = jest.fn();
+      const commandManager = createMockCommandManager({ executeCommand });
+
       showTextDocumentSpy.mockRejectedValue(new Error(msg));
       showInformationMessageSpy.mockResolvedValue({ title: "Yes" });
 
-      await tryOpenExternalFile("xxx");
+      await tryOpenExternalFile(commandManager, "xxx");
       const uri = vscode.Uri.file("xxx");
       expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
       expect(showTextDocumentSpy).toHaveBeenCalledWith(uri, expect.anything());
-      expect(executeCommandSpy).toHaveBeenCalledWith("revealFileInOS", uri);
+      expect(executeCommand).toHaveBeenCalledWith("revealFileInOS", uri);
     });
 
     it(`should fail to open a file because "${msg}" and NOT open externally`, async () => {
+      const executeCommand = jest.fn();
+      const commandManager = createMockCommandManager({ executeCommand });
+
       showTextDocumentSpy.mockRejectedValue(new Error(msg));
       showInformationMessageSpy.mockResolvedValue({ title: "No" });
 
-      await tryOpenExternalFile("xxx");
+      await tryOpenExternalFile(commandManager, "xxx");
       const uri = vscode.Uri.file("xxx");
       expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
       expect(showTextDocumentSpy).toHaveBeenCalledWith(uri, expect.anything());
       expect(showInformationMessageSpy).toBeCalled();
-      expect(executeCommandSpy).not.toBeCalled();
+      expect(executeCommand).not.toBeCalled();
     });
   });
 });