Pull tryOpenExternalFile out to a separate file so it can be more easily tested

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

@@ -26,12 +26,7 @@ import { extLogger } from "../common";
 import { URLSearchParams } from "url";
 import { DisposableObject } from "../pure/disposable-object";
 import { ONE_HOUR_IN_MS, TWO_HOURS_IN_MS } from "../pure/time";
-import {
-  asError,
-  assertNever,
-  getErrorMessage,
-  getErrorStack,
-} from "../pure/helpers-pure";
+import { asError, assertNever, getErrorMessage } from "../pure/helpers-pure";
 import { CompletedLocalQueryInfo, LocalQueryInfo } from "../query-results";
 import {
   getActionsWorkflowRunUrl,
@@ -66,6 +61,7 @@ import { HistoryTreeDataProvider } from "./history-tree-data-provider";
 import { redactableError } from "../pure/errors";
 import { QueryHistoryDirs } from "./query-history-dirs";
 import { QueryHistoryCommands } from "../common/commands";
+import { tryOpenExternalFile } from "../vscode-utils/external-files";
 
 /**
  * query-history-manager.ts
@@ -683,7 +679,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (singleItem.completedQuery.logFileLocation) {
-      await this.tryOpenExternalFile(singleItem.completedQuery.logFileLocation);
+      await tryOpenExternalFile(singleItem.completedQuery.logFileLocation);
     } else {
       void showAndLogWarningMessage("No log file available");
     }
@@ -800,7 +796,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogLocation) {
-      await this.tryOpenExternalFile(finalSingleItem.evalLogLocation);
+      await tryOpenExternalFile(finalSingleItem.evalLogLocation);
     } else {
       this.warnNoEvalLogs();
     }
@@ -825,7 +821,7 @@ export class QueryHistoryManager extends DisposableObject {
     }
 
     if (finalSingleItem.evalLogSummaryLocation) {
-      await this.tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
+      await tryOpenExternalFile(finalSingleItem.evalLogSummaryLocation);
       return;
     }
 
@@ -968,7 +964,7 @@ export class QueryHistoryManager extends DisposableObject {
     const query = finalSingleItem.completedQuery.query;
     const hasInterpretedResults = query.canHaveInterpretedResults();
     if (hasInterpretedResults) {
-      await this.tryOpenExternalFile(query.resultsPaths.interpretedResultsPath);
+      await tryOpenExternalFile(query.resultsPaths.interpretedResultsPath);
     } else {
       const label = this.labelProvider.getLabel(finalSingleItem);
       void showAndLogInformationMessage(
@@ -997,11 +993,11 @@ export class QueryHistoryManager extends DisposableObject {
     }
     const query = finalSingleItem.completedQuery.query;
     if (await query.hasCsv()) {
-      void this.tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(query.csvPath);
       return;
     }
     if (await query.exportCsvResults(this.qs.cliServer, query.csvPath)) {
-      void this.tryOpenExternalFile(query.csvPath);
+      void tryOpenExternalFile(query.csvPath);
     }
   }
 
@@ -1024,7 +1020,7 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    await this.tryOpenExternalFile(
+    await tryOpenExternalFile(
       await finalSingleItem.completedQuery.query.ensureCsvAlerts(
         this.qs.cliServer,
         this.dbm,
@@ -1051,7 +1047,7 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    await this.tryOpenExternalFile(
+    await tryOpenExternalFile(
       await finalSingleItem.completedQuery.query.ensureDilPath(
         this.qs.cliServer,
       ),
@@ -1171,47 +1167,6 @@ export class QueryHistoryManager extends DisposableObject {
     }
   }
 
-  private async tryOpenExternalFile(fileLocation: string) {
-    const uri = Uri.file(fileLocation);
-    try {
-      await window.showTextDocument(uri, { preview: false });
-    } catch (e) {
-      const msg = getErrorMessage(e);
-      if (
-        msg.includes(
-          "Files above 50MB cannot be synchronized with extensions",
-        ) ||
-        msg.includes("too large to open")
-      ) {
-        const res = await showBinaryChoiceDialog(
-          `VS Code does not allow extensions to open files >50MB. This file
-exceeds that limit. Do you want to open it outside of VS Code?
-
-You can also try manually opening it inside VS Code by selecting
-the file in the file explorer and dragging it into the workspace.`,
-        );
-        if (res) {
-          try {
-            await commands.executeCommand("revealFileInOS", uri);
-          } catch (e) {
-            void showAndLogExceptionWithTelemetry(
-              redactableError(
-                asError(e),
-              )`Failed to reveal file in OS: ${getErrorMessage(e)}`,
-            );
-          }
-        }
-      } else {
-        void showAndLogExceptionWithTelemetry(
-          redactableError(asError(e))`Could not open file ${fileLocation}`,
-          {
-            fullMessage: `${getErrorMessage(e)}\n${getErrorStack(e)}`,
-          },
-        );
-      }
-    }
-  }
-
   private async findOtherQueryToCompare(
     singleItem: QueryHistoryInfo,
     multiSelect: QueryHistoryInfo[],

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

@@ -0,0 +1,46 @@
+import { commands, Uri, window } from "vscode";
+import {
+  showAndLogExceptionWithTelemetry,
+  showBinaryChoiceDialog,
+} from "../helpers";
+import { redactableError } from "../pure/errors";
+import { asError, getErrorMessage, getErrorStack } from "../pure/helpers-pure";
+
+export async function tryOpenExternalFile(fileLocation: string) {
+  const uri = Uri.file(fileLocation);
+  try {
+    await window.showTextDocument(uri, { preview: false });
+  } catch (e) {
+    const msg = getErrorMessage(e);
+    if (
+      msg.includes("Files above 50MB cannot be synchronized with extensions") ||
+      msg.includes("too large to open")
+    ) {
+      const res = await showBinaryChoiceDialog(
+        `VS Code does not allow extensions to open files >50MB. This file
+exceeds that limit. Do you want to open it outside of VS Code?
+
+You can also try manually opening it inside VS Code by selecting
+the file in the file explorer and dragging it into the workspace.`,
+      );
+      if (res) {
+        try {
+          await commands.executeCommand("revealFileInOS", uri);
+        } catch (e) {
+          void showAndLogExceptionWithTelemetry(
+            redactableError(
+              asError(e),
+            )`Failed to reveal file in OS: ${getErrorMessage(e)}`,
+          );
+        }
+      }
+    } else {
+      void showAndLogExceptionWithTelemetry(
+        redactableError(asError(e))`Could not open file ${fileLocation}`,
+        {
+          fullMessage: `${getErrorMessage(e)}\n${getErrorStack(e)}`,
+        },
+      );
+    }
+  }
+}

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts

@@ -22,59 +22,49 @@ import { createMockVariantAnalysisHistoryItem } from "../../../factories/query-h
 import { VariantAnalysisHistoryItem } from "../../../../src/query-history/variant-analysis-history-item";
 import { QueryStatus } from "../../../../src/query-status";
 import { VariantAnalysisStatus } from "../../../../src/variant-analysis/shared/variant-analysis";
-import { TextEditor } from "vscode";
 import { WebviewReveal } from "../../../../src/interface-utils";
 import * as helpers from "../../../../src/helpers";
-import { mockedObject, mockedQuickPickItem } from "../../utils/mocking.helpers";
+import { mockedQuickPickItem } from "../../utils/mocking.helpers";
 import { createMockQueryHistoryDirs } from "../../../factories/query-history/query-history-dirs";
+import { createMockApp } from "../../../__mocks__/appMock";
+import { App } from "../../../../src/common/app";
+import { createMockCommandManager } from "../../../__mocks__/commandsMock";
 
 describe("QueryHistoryManager", () => {
   const mockExtensionLocation = join(tmpDir.name, "mock-extension-location");
   let configListener: QueryHistoryConfigListener;
-  let showTextDocumentSpy: jest.SpiedFunction<
-    typeof vscode.window.showTextDocument
-  >;
-  let showInformationMessageSpy: jest.SpiedFunction<
-    typeof vscode.window.showInformationMessage
-  >;
   let showQuickPickSpy: jest.SpiedFunction<typeof vscode.window.showQuickPick>;
-  let executeCommandSpy: jest.SpiedFunction<
-    typeof vscode.commands.executeCommand
-  >;
   let cancelVariantAnalysisSpy: jest.SpiedFunction<
     typeof variantAnalysisManagerStub.cancelVariantAnalysis
   >;
   const doCompareCallback = jest.fn();
 
+  let executeCommand: jest.MockedFn<
+    (commandName: string, ...args: any[]) => Promise<any>
+  >;
+  let mockApp: App;
+
   let queryHistoryManager: QueryHistoryManager;
 
   let localQueriesResultsViewStub: ResultsView;
   let variantAnalysisManagerStub: VariantAnalysisManager;
 
-  let tryOpenExternalFile: Function;
-
   let allHistory: QueryHistoryInfo[];
   let localQueryHistory: LocalQueryInfo[];
   let variantAnalysisHistory: VariantAnalysisHistoryItem[];
 
   beforeEach(() => {
-    showTextDocumentSpy = jest
-      .spyOn(vscode.window, "showTextDocument")
-      .mockResolvedValue(mockedObject<TextEditor>({}));
-    showInformationMessageSpy = jest
-      .spyOn(vscode.window, "showInformationMessage")
-      .mockResolvedValue(undefined);
     showQuickPickSpy = jest
       .spyOn(vscode.window, "showQuickPick")
       .mockResolvedValue(undefined);
-    executeCommandSpy = jest
-      .spyOn(vscode.commands, "executeCommand")
-      .mockResolvedValue(undefined);
+
+    executeCommand = jest.fn();
+    mockApp = createMockApp({
+      commands: createMockCommandManager({ executeCommand }),
+    });
 
     jest.spyOn(extLogger, "log").mockResolvedValue(undefined);
 
-    tryOpenExternalFile = (QueryHistoryManager.prototype as any)
-      .tryOpenExternalFile;
     configListener = new QueryHistoryConfigListener();
     localQueriesResultsViewStub = {
       showResults: jest.fn(),
@@ -157,51 +147,6 @@ describe("QueryHistoryManager", () => {
       queryHistoryManager.dispose();
     }
   });
-  describe("tryOpenExternalFile", () => {
-    it("should open an external file", async () => {
-      await tryOpenExternalFile("xxx");
-      expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
-      expect(showTextDocumentSpy).toHaveBeenCalledWith(
-        vscode.Uri.file("xxx"),
-        expect.anything(),
-      );
-      expect(executeCommandSpy).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 () => {
-        showTextDocumentSpy.mockRejectedValue(new Error(msg));
-        showInformationMessageSpy.mockResolvedValue({ title: "Yes" });
-
-        await tryOpenExternalFile("xxx");
-        const uri = vscode.Uri.file("xxx");
-        expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
-        expect(showTextDocumentSpy).toHaveBeenCalledWith(
-          uri,
-          expect.anything(),
-        );
-        expect(executeCommandSpy).toHaveBeenCalledWith("revealFileInOS", uri);
-      });
-
-      it(`should fail to open a file because "${msg}" and NOT open externally`, async () => {
-        showTextDocumentSpy.mockRejectedValue(new Error(msg));
-        showInformationMessageSpy.mockResolvedValue({ title: "No" });
-
-        await tryOpenExternalFile("xxx");
-        const uri = vscode.Uri.file("xxx");
-        expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
-        expect(showTextDocumentSpy).toHaveBeenCalledWith(
-          uri,
-          expect.anything(),
-        );
-        expect(showInformationMessageSpy).toBeCalled();
-        expect(executeCommandSpy).not.toBeCalled();
-      });
-    });
-  });
 
   describe("handleItemClicked", () => {
     describe("single click", () => {

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

@@ -0,0 +1,65 @@
+import * as vscode from "vscode";
+import { tryOpenExternalFile } from "../../../../../src/vscode-utils/external-files";
+import { mockedObject } from "../../../utils/mocking.helpers";
+
+describe("tryOpenExternalFile", () => {
+  let showTextDocumentSpy: jest.SpiedFunction<
+    typeof vscode.window.showTextDocument
+  >;
+  let showInformationMessageSpy: jest.SpiedFunction<
+    typeof vscode.window.showInformationMessage
+  >;
+  let executeCommandSpy: jest.SpiedFunction<
+    typeof vscode.commands.executeCommand
+  >;
+
+  beforeEach(() => {
+    showTextDocumentSpy = jest
+      .spyOn(vscode.window, "showTextDocument")
+      .mockResolvedValue(mockedObject<vscode.TextEditor>({}));
+    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");
+    expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
+    expect(showTextDocumentSpy).toHaveBeenCalledWith(
+      vscode.Uri.file("xxx"),
+      expect.anything(),
+    );
+    expect(executeCommandSpy).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 () => {
+      showTextDocumentSpy.mockRejectedValue(new Error(msg));
+      showInformationMessageSpy.mockResolvedValue({ title: "Yes" });
+
+      await tryOpenExternalFile("xxx");
+      const uri = vscode.Uri.file("xxx");
+      expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
+      expect(showTextDocumentSpy).toHaveBeenCalledWith(uri, expect.anything());
+      expect(executeCommandSpy).toHaveBeenCalledWith("revealFileInOS", uri);
+    });
+
+    it(`should fail to open a file because "${msg}" and NOT open externally`, async () => {
+      showTextDocumentSpy.mockRejectedValue(new Error(msg));
+      showInformationMessageSpy.mockResolvedValue({ title: "No" });
+
+      await tryOpenExternalFile("xxx");
+      const uri = vscode.Uri.file("xxx");
+      expect(showTextDocumentSpy).toHaveBeenCalledTimes(1);
+      expect(showTextDocumentSpy).toHaveBeenCalledWith(uri, expect.anything());
+      expect(showInformationMessageSpy).toBeCalled();
+      expect(executeCommandSpy).not.toBeCalled();
+    });
+  });
+});