Update variant analysis monitor to get the whole variant analysis object (#3415)

Author: charisk

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

@@ -146,7 +146,7 @@ export class VariantAnalysisManager
       new VariantAnalysisMonitor(
         app,
         this.shouldCancelMonitorVariantAnalysis.bind(this),
-        this.getVariantAnalysisStatus.bind(this),
+        this.getVariantAnalysis.bind(this),
       ),
     );
     this.variantAnalysisMonitor.onVariantAnalysisChange(
@@ -606,17 +606,14 @@ export class VariantAnalysisManager
     return !this.variantAnalyses.has(variantAnalysisId);
   }
 
-  private getVariantAnalysisStatus(
-    variantAnalysisId: number,
-  ): VariantAnalysisStatus {
-    const variantAnalysis = this.variantAnalyses.get(variantAnalysisId);
+  private getVariantAnalysis(variantAnalysisId: number): VariantAnalysis {
+    const variantAnalysis = this.tryGetVariantAnalysis(variantAnalysisId);
+
     if (!variantAnalysis) {
-      throw new Error(
-        `No variant analysis found with id: ${variantAnalysisId}.`,
-      );
+      throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
     }
 
-    return variantAnalysis.status;
+    return variantAnalysis;
   }
 
   public async onVariantAnalysisUpdated(

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

@@ -5,7 +5,6 @@ import { RequestError } from "@octokit/request-error";
 import type {
   VariantAnalysis,
   VariantAnalysisScannedRepository,
-  VariantAnalysisStatus,
 } from "./shared/variant-analysis";
 import {
   isFinalVariantAnalysisStatus,
@@ -18,6 +17,7 @@ import { sleep } from "../common/time";
 import { getErrorMessage } from "../common/helpers-pure";
 import type { App } from "../common/app";
 import { showAndLogWarningMessage } from "../common/logging";
+import type { QueryLanguage } from "../common/query-language";
 
 export class VariantAnalysisMonitor extends DisposableObject {
   // With a sleep of 5 seconds, the maximum number of attempts takes
@@ -37,9 +37,9 @@ export class VariantAnalysisMonitor extends DisposableObject {
     private readonly shouldCancelMonitor: (
       variantAnalysisId: number,
     ) => Promise<boolean>,
-    private readonly getVariantAnalysisStatus: (
+    private readonly getVariantAnalysis: (
       variantAnalysisId: number,
-    ) => VariantAnalysisStatus,
+    ) => VariantAnalysis,
   ) {
     super();
   }
@@ -60,20 +60,28 @@ export class VariantAnalysisMonitor extends DisposableObject {
 
     this.monitoringVariantAnalyses.add(variantAnalysis.id);
     try {
-      await this._monitorVariantAnalysis(variantAnalysis);
+      await this._monitorVariantAnalysis(
+        variantAnalysis.id,
+        variantAnalysis.controllerRepo.id,
+        variantAnalysis.executionStartTime,
+        variantAnalysis.query.name,
+        variantAnalysis.language,
+      );
     } finally {
       this.monitoringVariantAnalyses.delete(variantAnalysis.id);
     }
   }
 
   private async _monitorVariantAnalysis(
-    variantAnalysis: VariantAnalysis,
+    variantAnalysisId: number,
+    controllerRepoId: number,
+    executionStartTime: number,
+    queryName: string,
+    language: QueryLanguage,
   ): Promise<void> {
-    const variantAnalysisLabel = `${variantAnalysis.query.name} (${
-      variantAnalysis.language
-    }) [${new Date(variantAnalysis.executionStartTime).toLocaleString(
-      env.language,
-    )}]`;
+    const variantAnalysisLabel = `${queryName} (${language}) [${new Date(
+      executionStartTime,
+    ).toLocaleString(env.language)}]`;
 
     let attemptCount = 0;
     const scannedReposDownloaded: number[] = [];
@@ -83,16 +91,16 @@ export class VariantAnalysisMonitor extends DisposableObject {
     while (attemptCount <= VariantAnalysisMonitor.maxAttemptCount) {
       await sleep(VariantAnalysisMonitor.sleepTime);
 
-      if (await this.shouldCancelMonitor(variantAnalysis.id)) {
+      if (await this.shouldCancelMonitor(variantAnalysisId)) {
         return;
       }
 
       let variantAnalysisSummary: ApiVariantAnalysis;
       try {
         variantAnalysisSummary = await getVariantAnalysis(
           this.app.credentials,
-          variantAnalysis.controllerRepo.id,
-          variantAnalysis.id,
+          controllerRepoId,
+          variantAnalysisId,
         );
       } catch (e) {
         const errorMessage = getErrorMessage(e);
@@ -123,15 +131,10 @@ export class VariantAnalysisMonitor extends DisposableObject {
         continue;
       }
 
-      // Get the current status of the variant analysis as known by the rest
-      // of the app, because it may have been changed by the user and this code
-      // may not be aware of it yet.
-      const currentStatus = this.getVariantAnalysisStatus(variantAnalysis.id);
-      variantAnalysis = mapUpdatedVariantAnalysis(
-        {
-          ...variantAnalysis,
-          status: currentStatus,
-        },
+      const variantAnalysis = mapUpdatedVariantAnalysis(
+        // Get the variant analysis as known by the rest of the app, because it may
+        // have been changed by the user and the monitors may not be aware of it yet.
+        this.getVariantAnalysis(variantAnalysisId),
         variantAnalysisSummary,
       );
 

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

@@ -32,7 +32,7 @@ describe("Variant Analysis Monitor", () => {
   >;
   let variantAnalysisMonitor: VariantAnalysisMonitor;
   let shouldCancelMonitor: jest.Mock<Promise<boolean>, [number]>;
-  let mockGetVariantAnalysisStatus: jest.Mock<VariantAnalysisStatus, [number]>;
+  let mockGetVariantAnalysis: jest.Mock<VariantAnalysis, [number]>;
   let variantAnalysis: VariantAnalysis;
 
   const onVariantAnalysisChangeSpy = jest.fn();
@@ -44,7 +44,7 @@ describe("Variant Analysis Monitor", () => {
     variantAnalysis = createMockVariantAnalysis({});
 
     shouldCancelMonitor = jest.fn();
-    mockGetVariantAnalysisStatus = jest.fn();
+    mockGetVariantAnalysis = jest.fn();
 
     logger = createMockLogger();
 
@@ -56,14 +56,16 @@ describe("Variant Analysis Monitor", () => {
         logger,
       }),
       shouldCancelMonitor,
-      mockGetVariantAnalysisStatus,
+      mockGetVariantAnalysis,
     );
     variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy);
 
     mockGetVariantAnalysisFromApi = jest
       .spyOn(ghApiClient, "getVariantAnalysis")
       .mockRejectedValue(new Error("Not mocked"));
 
+    mockGetVariantAnalysis.mockReturnValue(variantAnalysis);
+
     limitNumberOfAttemptsToMonitor();
   });
 
@@ -342,9 +344,11 @@ describe("Variant Analysis Monitor", () => {
 
     describe("cancelation", () => {
       it("should maintain canceling status", async () => {
-        mockGetVariantAnalysisStatus.mockReturnValueOnce(
-          VariantAnalysisStatus.Canceling,
-        );
+        mockGetVariantAnalysis.mockReturnValueOnce({
+          ...variantAnalysis,
+          status: VariantAnalysisStatus.Canceling,
+        });
+
         mockApiResponse = createMockApiResponse("in_progress");
         mockGetVariantAnalysisFromApi.mockResolvedValue(mockApiResponse);