Convert extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts to call typed commands

Author: robertbrignull

extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts

@@ -115,6 +115,7 @@ export class VariantAnalysisManager
     super();
     this.variantAnalysisMonitor = this.push(
       new VariantAnalysisMonitor(
+        app,
         this.shouldCancelMonitorVariantAnalysis.bind(this),
       ),
     );
@@ -501,10 +502,7 @@ export class VariantAnalysisManager
   public async monitorVariantAnalysis(
     variantAnalysis: VariantAnalysis,
   ): Promise<void> {
-    await this.variantAnalysisMonitor.monitorVariantAnalysis(
-      variantAnalysis,
-      this.app.credentials,
-    );
+    await this.variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
   }
 
   public async autoDownloadVariantAnalysisResult(

extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts

@@ -1,4 +1,4 @@
-import { commands, EventEmitter } from "vscode";
+import { EventEmitter } from "vscode";
 import { getVariantAnalysis } from "./gh-api/gh-api-client";
 
 import {
@@ -13,7 +13,7 @@ import { DisposableObject } from "../pure/disposable-object";
 import { sleep } from "../pure/time";
 import { getErrorMessage } from "../pure/helpers-pure";
 import { showAndLogWarningMessage } from "../helpers";
-import { Credentials } from "../common/authentication";
+import { App } from "../common/app";
 
 export class VariantAnalysisMonitor extends DisposableObject {
   // With a sleep of 5 seconds, the maximum number of attempts takes
@@ -27,6 +27,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
   readonly onVariantAnalysisChange = this._onVariantAnalysisChange.event;
 
   constructor(
+    private readonly app: App,
     private readonly shouldCancelMonitor: (
       variantAnalysisId: number,
     ) => Promise<boolean>,
@@ -36,7 +37,6 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
   public async monitorVariantAnalysis(
     variantAnalysis: VariantAnalysis,
-    credentials: Credentials,
   ): Promise<void> {
     let attemptCount = 0;
     const scannedReposDownloaded: number[] = [];
@@ -51,7 +51,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
       let variantAnalysisSummary: ApiVariantAnalysis;
       try {
         variantAnalysisSummary = await getVariantAnalysis(
-          credentials,
+          this.app.credentials,
           variantAnalysis.controllerRepo.id,
           variantAnalysis.id,
         );
@@ -87,7 +87,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
     scannedRepo: VariantAnalysisScannedRepository,
     variantAnalysisSummary: VariantAnalysis,
   ) {
-    void commands.executeCommand(
+    void this.app.commands.execute(
       "codeQL.autoDownloadVariantAnalysisResult",
       scannedRepo,
       variantAnalysisSummary,

extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts

@@ -1,6 +1,3 @@
-import { commands, extensions } from "vscode";
-import { CodeQLExtensionInterface } from "../../../../src/extension";
-
 import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client";
 import { VariantAnalysisMonitor } from "../../../../src/variant-analysis/variant-analysis-monitor";
 import {
@@ -23,44 +20,37 @@ import {
   processUpdatedVariantAnalysis,
 } from "../../../../src/variant-analysis/variant-analysis-processor";
 import { createMockVariantAnalysis } from "../../../factories/variant-analysis/shared/variant-analysis";
-import { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager";
-import { testCredentialsWithStub } from "../../../factories/authentication";
+import { createMockApp } from "../../../__mocks__/appMock";
+import { createMockCommandManager } from "../../../__mocks__/commandsMock";
 
 jest.setTimeout(60_000);
 
 describe("Variant Analysis Monitor", () => {
-  let extension: CodeQLExtensionInterface | Record<string, never>;
   let mockGetVariantAnalysis: jest.SpiedFunction<
     typeof ghApiClient.getVariantAnalysis
   >;
   let variantAnalysisMonitor: VariantAnalysisMonitor;
   let shouldCancelMonitor: jest.Mock<Promise<boolean>, [number]>;
   let variantAnalysis: VariantAnalysis;
-  let variantAnalysisManager: VariantAnalysisManager;
-  let mockGetDownloadResult: jest.SpiedFunction<
-    typeof variantAnalysisManager.autoDownloadVariantAnalysisResult
-  >;
 
   const onVariantAnalysisChangeSpy = jest.fn();
+  const mockEecuteCommand = jest.fn();
 
   beforeEach(async () => {
     variantAnalysis = createMockVariantAnalysis({});
 
     shouldCancelMonitor = jest.fn();
 
-    extension = await extensions
-      .getExtension<CodeQLExtensionInterface | Record<string, never>>(
-        "GitHub.vscode-codeql",
-      )!
-      .activate();
-    variantAnalysisMonitor = new VariantAnalysisMonitor(shouldCancelMonitor);
+    variantAnalysisMonitor = new VariantAnalysisMonitor(
+      createMockApp({
+        commands: createMockCommandManager({
+          executeCommand: mockEecuteCommand,
+        }),
+      }),
+      shouldCancelMonitor,
+    );
     variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy);
 
-    variantAnalysisManager = extension.variantAnalysisManager;
-    mockGetDownloadResult = jest
-      .spyOn(variantAnalysisManager, "autoDownloadVariantAnalysisResult")
-      .mockResolvedValue(undefined);
-
     mockGetVariantAnalysis = jest
       .spyOn(ghApiClient, "getVariantAnalysis")
       .mockRejectedValue(new Error("Not mocked"));
@@ -71,10 +61,7 @@ describe("Variant Analysis Monitor", () => {
   it("should return early if variant analysis should be cancelled", async () => {
     shouldCancelMonitor.mockResolvedValue(true);
 
-    await variantAnalysisMonitor.monitorVariantAnalysis(
-      variantAnalysis,
-      testCredentialsWithStub(),
-    );
+    await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
     expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled();
   });
@@ -88,10 +75,7 @@ describe("Variant Analysis Monitor", () => {
     });
 
     it("should mark as failed and stop monitoring", async () => {
-      await variantAnalysisMonitor.monitorVariantAnalysis(
-        variantAnalysis,
-        testCredentialsWithStub(),
-      );
+      await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
       expect(mockGetVariantAnalysis).toHaveBeenCalledTimes(1);
 
@@ -109,7 +93,6 @@ describe("Variant Analysis Monitor", () => {
   describe("when the variant analysis is in progress", () => {
     let mockApiResponse: VariantAnalysisApiResponse;
     let scannedRepos: ApiVariantAnalysisScannedRepository[];
-    let succeededRepos: ApiVariantAnalysisScannedRepository[];
 
     describe("when there are successfully scanned repos", () => {
       beforeEach(async () => {
@@ -124,52 +107,25 @@ describe("Variant Analysis Monitor", () => {
         ]);
         mockApiResponse = createMockApiResponse("succeeded", scannedRepos);
         mockGetVariantAnalysis.mockResolvedValue(mockApiResponse);
-        succeededRepos = scannedRepos.filter(
-          (r) => r.analysis_status === "succeeded",
-        );
       });
 
       it("should trigger a download extension command for each repo", async () => {
         const succeededRepos = scannedRepos.filter(
           (r) => r.analysis_status === "succeeded",
         );
-        const commandSpy = jest
-          .spyOn(commands, "executeCommand")
-          .mockResolvedValue(undefined);
-
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
-        expect(commandSpy).toBeCalledTimes(succeededRepos.length);
+        expect(mockEecuteCommand).toBeCalledTimes(succeededRepos.length);
 
         succeededRepos.forEach((succeededRepo, index) => {
-          expect(commandSpy).toHaveBeenNthCalledWith(
+          expect(mockEecuteCommand).toHaveBeenNthCalledWith(
             index + 1,
             "codeQL.autoDownloadVariantAnalysisResult",
             processScannedRepository(succeededRepo),
             processUpdatedVariantAnalysis(variantAnalysis, mockApiResponse),
           );
         });
       });
-
-      it("should download all available results", async () => {
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
-
-        expect(mockGetDownloadResult).toBeCalledTimes(succeededRepos.length);
-
-        succeededRepos.forEach((succeededRepo, index) => {
-          expect(mockGetDownloadResult).toHaveBeenNthCalledWith(
-            index + 1,
-            processScannedRepository(succeededRepo),
-            processUpdatedVariantAnalysis(variantAnalysis, mockApiResponse),
-          );
-        });
-      });
     });
 
     describe("when there are only in progress repos", () => {
@@ -182,25 +138,9 @@ describe("Variant Analysis Monitor", () => {
       });
 
       it("should succeed and not download any repos via a command", async () => {
-        const commandSpy = jest
-          .spyOn(commands, "executeCommand")
-          .mockResolvedValue(undefined);
-
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
-        expect(commandSpy).not.toHaveBeenCalled();
-      });
-
-      it("should not try to download any repos", async () => {
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
-
-        expect(mockGetDownloadResult).not.toBeCalled();
+        expect(mockEecuteCommand).not.toHaveBeenCalled();
       });
     });
 
@@ -249,17 +189,10 @@ describe("Variant Analysis Monitor", () => {
       });
 
       it("should trigger a download extension command for each repo", async () => {
-        const commandSpy = jest
-          .spyOn(commands, "executeCommand")
-          .mockResolvedValue(undefined);
-
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
         expect(mockGetVariantAnalysis).toBeCalledTimes(4);
-        expect(commandSpy).toBeCalledTimes(5);
+        expect(mockEecuteCommand).toBeCalledTimes(5);
       });
     });
 
@@ -271,12 +204,9 @@ describe("Variant Analysis Monitor", () => {
       });
 
       it("should not try to download any repos", async () => {
-        await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          testCredentialsWithStub(),
-        );
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
 
-        expect(mockGetDownloadResult).not.toBeCalled();
+        expect(mockEecuteCommand).not.toBeCalled();
       });
     });
   });