Remove variant analysis monitor return value

The monitor return value was only used in tests, but we can also assert
the correct behavior using the calls it makes, rather than using the
result of the monitor.

Author: koesie10

extensions/ql-vscode/src/remote-queries/shared/variant-analysis-monitor-result.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/remote-queries/variant-analysis-monitor.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,