Merge pull request #1789 from github/koesie10/remove-variant-analysis-monitor-return-value

Remove variant analysis monitor return value

extensions/ql-vscode/src/remote-queries/shared/variant-analysis-monitor-result.ts

@@ -1,9 +0,0 @@
-import { VariantAnalysis } from "./variant-analysis";
-
-export type VariantAnalysisMonitorStatus = "Completed" | "Canceled";
-
-export interface VariantAnalysisMonitorResult {
-  status: VariantAnalysisMonitorStatus;
-  scannedReposDownloaded?: number[];
-  variantAnalysis?: VariantAnalysis;
-}

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

@@ -13,7 +13,6 @@ import {
   VariantAnalysis,
   VariantAnalysisScannedRepository,
 } from "./shared/variant-analysis";
-import { VariantAnalysisMonitorResult } from "./shared/variant-analysis-monitor-result";
 import { processUpdatedVariantAnalysis } from "./variant-analysis-processor";
 import { DisposableObject } from "../pure/disposable-object";
 import { sleep } from "../pure/time";
@@ -36,7 +35,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
   public async monitorVariantAnalysis(
     variantAnalysis: VariantAnalysis,
     cancellationToken: CancellationToken,
-  ): Promise<VariantAnalysisMonitorResult> {
+  ): Promise<void> {
     const credentials = await Credentials.initialize(this.extensionContext);
     if (!credentials) {
       throw Error("Error authenticating with GitHub");
@@ -49,7 +48,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
       await sleep(VariantAnalysisMonitor.sleepTime);
 
       if (cancellationToken && cancellationToken.isCancellationRequested) {
-        return { status: "Canceled" };
+        return;
       }
 
       const variantAnalysisSummary = await ghApiClient.getVariantAnalysis(
@@ -77,8 +76,6 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
       attemptCount++;
     }
-
-    return { status: "Completed", scannedReposDownloaded, variantAnalysis };
   }
 
   private scheduleForDownload(

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-monitor.test.ts

@@ -42,6 +42,8 @@ describe("Variant Analysis Monitor", () => {
     typeof variantAnalysisManager.autoDownloadVariantAnalysisResult
   >;
 
+  const onVariantAnalysisChangeSpy = jest.fn();
+
   beforeEach(async () => {
     jest
       .spyOn(config, "isVariantAnalysisLiveResultsEnabled")
@@ -57,6 +59,7 @@ describe("Variant Analysis Monitor", () => {
       )!
       .activate();
     variantAnalysisMonitor = new VariantAnalysisMonitor(extension.ctx);
+    variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy);
 
     variantAnalysisManager = extension.variantAnalysisManager;
     mockGetDownloadResult = jest
@@ -103,12 +106,12 @@ describe("Variant Analysis Monitor", () => {
     it("should return early if variant analysis is cancelled", async () => {
       cancellationTokenSource.cancel();
 
-      const result = await variantAnalysisMonitor.monitorVariantAnalysis(
+      await variantAnalysisMonitor.monitorVariantAnalysis(
         variantAnalysis,
         cancellationTokenSource.token,
       );
 
-      expect(result).toEqual({ status: "Canceled" });
+      expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled();
     });
 
     describe("when the variant analysis fails", () => {
@@ -119,34 +122,22 @@ describe("Variant Analysis Monitor", () => {
         mockGetVariantAnalysis.mockResolvedValue(mockFailedApiResponse);
       });
 
-      it("should mark as failed locally and stop monitoring", async () => {
-        const result = await variantAnalysisMonitor.monitorVariantAnalysis(
+      it("should mark as failed and stop monitoring", async () => {
+        await variantAnalysisMonitor.monitorVariantAnalysis(
           variantAnalysis,
           cancellationTokenSource.token,
         );
 
         expect(mockGetVariantAnalysis).toHaveBeenCalledTimes(1);
-        expect(result.status).toEqual("Completed");
-        expect(result.variantAnalysis?.status).toBe(
-          VariantAnalysisStatus.Failed,
-        );
-        expect(result.variantAnalysis?.failureReason).toBe(
-          processFailureReason(
-            mockFailedApiResponse.failure_reason as VariantAnalysisFailureReason,
-          ),
-        );
-      });
 
-      it("should emit `onVariantAnalysisChange`", async () => {
-        const spy = jest.fn();
-        variantAnalysisMonitor.onVariantAnalysisChange(spy);
-
-        const result = await variantAnalysisMonitor.monitorVariantAnalysis(
-          variantAnalysis,
-          cancellationTokenSource.token,
+        expect(onVariantAnalysisChangeSpy).toHaveBeenCalledWith(
+          expect.objectContaining({
+            status: VariantAnalysisStatus.Failed,
+            failureReason: processFailureReason(
+              mockFailedApiResponse.failure_reason as VariantAnalysisFailureReason,
+            ),
+          }),
         );
-
-        expect(spy).toBeCalledWith(result.variantAnalysis);
       });
     });
 
@@ -173,18 +164,6 @@ describe("Variant Analysis Monitor", () => {
           );
         });
 
-        it("should succeed and return a list of scanned repo ids", async () => {
-          const result = await variantAnalysisMonitor.monitorVariantAnalysis(
-            variantAnalysis,
-            cancellationTokenSource.token,
-          );
-
-          expect(result.status).toBe("Completed");
-          expect(result.scannedReposDownloaded).toEqual(
-            succeededRepos.map((r) => r.repository.id),
-          );
-        });
-
         it("should trigger a download extension command for each repo", async () => {
           const succeededRepos = scannedRepos.filter(
             (r) => r.analysis_status === "succeeded",
@@ -238,14 +217,17 @@ describe("Variant Analysis Monitor", () => {
           mockGetVariantAnalysis.mockResolvedValue(mockApiResponse);
         });
 
-        it("should succeed and return an empty list of scanned repo ids", async () => {
-          const result = await variantAnalysisMonitor.monitorVariantAnalysis(
+        it("should succeed and not download any repos via a command", async () => {
+          const commandSpy = jest
+            .spyOn(commands, "executeCommand")
+            .mockResolvedValue(undefined);
+
+          await variantAnalysisMonitor.monitorVariantAnalysis(
             variantAnalysis,
             cancellationTokenSource.token,
           );
 
-          expect(result.status).toBe("Completed");
-          expect(result.scannedReposDownloaded).toEqual([]);
+          expect(commandSpy).not.toHaveBeenCalled();
         });
 
         it("should not try to download any repos", async () => {
@@ -265,16 +247,6 @@ describe("Variant Analysis Monitor", () => {
           mockGetVariantAnalysis.mockResolvedValue(mockApiResponse);
         });
 
-        it("should succeed and return an empty list of scanned repo ids", async () => {
-          const result = await variantAnalysisMonitor.monitorVariantAnalysis(
-            variantAnalysis,
-            cancellationTokenSource.token,
-          );
-
-          expect(result.status).toBe("Completed");
-          expect(result.scannedReposDownloaded).toEqual([]);
-        });
-
         it("should not try to download any repos", async () => {
           await variantAnalysisMonitor.monitorVariantAnalysis(
             variantAnalysis,