Add support for 'canceling' status for variant analysis (#3405)

extensions/ql-vscode/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [UNRELEASED]
 
+- Update variant analysis view to show when cancelation is in progress. [#3405](https://github.com/github/vscode-codeql/pull/3405)
 - Remove support for CodeQL CLI versions older than 2.13.5. [#3371](https://github.com/github/vscode-codeql/pull/3371)
 - Add a timeout to downloading databases and the CodeQL CLI. These can be changed using the `codeQL.addingDatabases.downloadTimeout` and `codeQL.cli.downloadTimeout` settings respectively. [#3373](https://github.com/github/vscode-codeql/pull/3373)
 

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

@@ -17,6 +17,8 @@ export function variantAnalysisStatusToQueryStatus(
       return QueryStatus.Failed;
     case VariantAnalysisStatus.InProgress:
       return QueryStatus.InProgress;
+    case VariantAnalysisStatus.Canceling:
+      return QueryStatus.InProgress;
     case VariantAnalysisStatus.Canceled:
       return QueryStatus.Completed;
     default:

extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts

@@ -195,6 +195,11 @@ function mapVariantAnalysisStatusToDto(
       return VariantAnalysisStatusDto.Succeeded;
     case VariantAnalysisStatus.Failed:
       return VariantAnalysisStatusDto.Failed;
+    case VariantAnalysisStatus.Canceling:
+      // The canceling state shouldn't be persisted. We can just
+      // assume that the analysis is still in progress, since the
+      // canceling state is very short-lived.
+      return VariantAnalysisStatusDto.InProgress;
     case VariantAnalysisStatus.Canceled:
       return VariantAnalysisStatusDto.Canceled;
     default:

extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisActions.stories.tsx

@@ -70,3 +70,9 @@ Failed.args = {
   ...InProgress.args,
   variantAnalysisStatus: VariantAnalysisStatus.Failed,
 };
+
+export const Canceling = Template.bind({});
+Canceling.args = {
+  ...InProgress.args,
+  variantAnalysisStatus: VariantAnalysisStatus.Canceling,
+};

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

@@ -39,6 +39,7 @@ export enum VariantAnalysisStatus {
   InProgress = "inProgress",
   Succeeded = "succeeded",
   Failed = "failed",
+  Canceling = "canceling",
   Canceled = "canceled",
 }
 

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

@@ -34,6 +34,7 @@ import {
   isVariantAnalysisComplete,
   parseVariantAnalysisQueryLanguage,
   VariantAnalysisScannedRepositoryDownloadStatus,
+  VariantAnalysisStatus,
 } from "./shared/variant-analysis";
 import { getErrorMessage } from "../common/helpers-pure";
 import { VariantAnalysisView } from "./variant-analysis-view";
@@ -145,6 +146,7 @@ export class VariantAnalysisManager
       new VariantAnalysisMonitor(
         app,
         this.shouldCancelMonitorVariantAnalysis.bind(this),
+        this.getVariantAnalysisStatus.bind(this),
       ),
     );
     this.variantAnalysisMonitor.onVariantAnalysisChange(
@@ -604,6 +606,19 @@ export class VariantAnalysisManager
     return !this.variantAnalyses.has(variantAnalysisId);
   }
 
+  private getVariantAnalysisStatus(
+    variantAnalysisId: number,
+  ): VariantAnalysisStatus {
+    const variantAnalysis = this.variantAnalyses.get(variantAnalysisId);
+    if (!variantAnalysis) {
+      throw new Error(
+        `No variant analysis found with id: ${variantAnalysisId}.`,
+      );
+    }
+
+    return variantAnalysis.status;
+  }
+
   public async onVariantAnalysisUpdated(
     variantAnalysis: VariantAnalysis | undefined,
   ): Promise<void> {
@@ -611,7 +626,11 @@ export class VariantAnalysisManager
       return;
     }
 
-    if (!this.variantAnalyses.has(variantAnalysis.id)) {
+    const originalVariantAnalysis = this.variantAnalyses.get(
+      variantAnalysis.id,
+    );
+
+    if (!originalVariantAnalysis) {
       return;
     }
 
@@ -844,11 +863,24 @@ export class VariantAnalysisManager
       );
     }
 
+    await this.onVariantAnalysisUpdated({
+      ...variantAnalysis,
+      status: VariantAnalysisStatus.Canceling,
+    });
+
     void showAndLogInformationMessage(
       this.app.logger,
       "Cancelling variant analysis. This may take a while.",
     );
+    try {
       await cancelVariantAnalysis(this.app.credentials, variantAnalysis);
+    } catch (e) {
+      await this.onVariantAnalysisUpdated({
+        ...variantAnalysis,
+        status: VariantAnalysisStatus.InProgress,
+      });
+      throw e;
+    }
   }
 
   public async openVariantAnalysisLogs(variantAnalysisId: number) {

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

@@ -40,6 +40,7 @@ export function mapVariantAnalysis(
       databases: submission.databases,
       executionStartTime: submission.startTime,
     },
+    undefined,
     response,
   );
 }
@@ -49,6 +50,7 @@ export function mapUpdatedVariantAnalysis(
     VariantAnalysis,
     "language" | "query" | "queries" | "databases" | "executionStartTime"
   >,
+  currentStatus: VariantAnalysisStatus | undefined,
   response: ApiVariantAnalysis,
 ): VariantAnalysis {
   let scannedRepos: VariantAnalysisScannedRepository[] = [];
@@ -66,6 +68,13 @@ export function mapUpdatedVariantAnalysis(
     );
   }
 
+  // Maintain the canceling status if we are still canceling.
+  const status =
+    currentStatus === VariantAnalysisStatus.Canceling &&
+    response.status === "in_progress"
+      ? VariantAnalysisStatus.Canceling
+      : mapApiStatus(response.status);
+
   const variantAnalysis: VariantAnalysis = {
     id: response.id,
     controllerRepo: {
@@ -80,7 +89,7 @@ export function mapUpdatedVariantAnalysis(
     executionStartTime: previousVariantAnalysis.executionStartTime,
     createdAt: response.created_at,
     updatedAt: response.updated_at,
-    status: mapApiStatus(response.status),
+    status,
     completedAt: response.completed_at,
     actionsWorkflowRunId: response.actions_workflow_run_id,
     scannedRepos,

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

@@ -5,6 +5,7 @@ import { RequestError } from "@octokit/request-error";
 import type {
   VariantAnalysis,
   VariantAnalysisScannedRepository,
+  VariantAnalysisStatus,
 } from "./shared/variant-analysis";
 import {
   isFinalVariantAnalysisStatus,
@@ -36,6 +37,9 @@ export class VariantAnalysisMonitor extends DisposableObject {
     private readonly shouldCancelMonitor: (
       variantAnalysisId: number,
     ) => Promise<boolean>,
+    private readonly getVariantAnalysisStatus: (
+      variantAnalysisId: number,
+    ) => VariantAnalysisStatus,
   ) {
     super();
   }
@@ -119,8 +123,13 @@ 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,
+        currentStatus,
         variantAnalysisSummary,
       );
 

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx

@@ -103,6 +103,11 @@ export const VariantAnalysisActions = ({
           Stop query
         </Button>
       )}
+      {variantAnalysisStatus === VariantAnalysisStatus.Canceling && (
+        <Button appearance="secondary" disabled={true}>
+          Stopping query
+        </Button>
+      )}
     </Container>
   );
 };

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx

@@ -48,6 +48,10 @@ export const VariantAnalysisStats = ({
       return "Failed";
     }
 
+    if (variantAnalysisStatus === VariantAnalysisStatus.Canceling) {
+      return "Canceling";
+    }
+
     if (variantAnalysisStatus === VariantAnalysisStatus.Canceled) {
       return "Stopped";
     }

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStatusStats.tsx

@@ -28,7 +28,8 @@ export const VariantAnalysisStatusStats = ({
 }: VariantAnalysisStatusStatsProps) => {
   return (
     <Container>
-      {variantAnalysisStatus === VariantAnalysisStatus.InProgress ? (
+      {variantAnalysisStatus === VariantAnalysisStatus.InProgress ||
+      variantAnalysisStatus === VariantAnalysisStatus.Canceling ? (
         <div>
           <Icon className="codicon codicon-loading codicon-modifier-spin" />
         </div>

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisActions.spec.tsx

@@ -45,6 +45,24 @@ describe(VariantAnalysisActions.name, () => {
     expect(onStopQueryClick).toHaveBeenCalledTimes(1);
   });
 
+  it("renders the stopping query disabled button when canceling", async () => {
+    render({
+      variantAnalysisStatus: VariantAnalysisStatus.Canceling,
+    });
+
+    const button = screen.getByText("Stopping query");
+    expect(button).toBeInTheDocument();
+    expect(button.getElementsByTagName("input")[0]).toBeDisabled();
+  });
+
+  it("does not render a stop query button when canceling", async () => {
+    render({
+      variantAnalysisStatus: VariantAnalysisStatus.Canceling,
+    });
+
+    expect(screen.queryByText("Stop query")).not.toBeInTheDocument();
+  });
+
   it("renders 3 buttons when in progress with results", async () => {
     const { container } = render({
       variantAnalysisStatus: VariantAnalysisStatus.InProgress,

extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx

@@ -160,6 +160,12 @@ describe(VariantAnalysisStats.name, () => {
     expect(screen.getByText("Failed")).toBeInTheDocument();
   });
 
+  it("renders 'Stopping' text when the variant analysis status is canceling", () => {
+    render({ variantAnalysisStatus: VariantAnalysisStatus.Canceling });
+
+    expect(screen.getByText("Canceling")).toBeInTheDocument();
+  });
+
   it("renders 'Stopped' text when the variant analysis status is canceled", () => {
     render({ variantAnalysisStatus: VariantAnalysisStatus.Canceled });
 

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

@@ -609,7 +609,7 @@ describe("Variant Analysis Manager", () => {
       }
     });
 
-    it("should return cancel if valid", async () => {
+    it("should make API request to cancel if valid", async () => {
       await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
 
       expect(mockCancelVariantAnalysis).toHaveBeenCalledWith(
@@ -617,6 +617,28 @@ describe("Variant Analysis Manager", () => {
         variantAnalysis,
       );
     });
+
+    it("should set the status to canceling", async () => {
+      await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
+
+      const updatedAnalysis =
+        await variantAnalysisManager.tryGetVariantAnalysis(variantAnalysis.id);
+      expect(updatedAnalysis?.status).toBe(VariantAnalysisStatus.Canceling);
+    });
+
+    it("should set the status back to in progress if canceling fails", async () => {
+      mockCancelVariantAnalysis.mockRejectedValueOnce(
+        new Error("Error when cancelling"),
+      );
+
+      await expect(
+        variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id),
+      ).rejects.toThrow("Error when cancelling");
+
+      const updatedAnalysis =
+        await variantAnalysisManager.tryGetVariantAnalysis(variantAnalysis.id);
+      expect(updatedAnalysis?.status).toBe(VariantAnalysisStatus.InProgress);
+    });
   });
 
   describe("copyRepoListToClipboard", () => {

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

@@ -32,6 +32,7 @@ describe("Variant Analysis Monitor", () => {
   >;
   let variantAnalysisMonitor: VariantAnalysisMonitor;
   let shouldCancelMonitor: jest.Mock<Promise<boolean>, [number]>;
+  let mockGetVariantAnalysisStatus: jest.Mock<VariantAnalysisStatus, [number]>;
   let variantAnalysis: VariantAnalysis;
 
   const onVariantAnalysisChangeSpy = jest.fn();
@@ -43,6 +44,7 @@ describe("Variant Analysis Monitor", () => {
     variantAnalysis = createMockVariantAnalysis({});
 
     shouldCancelMonitor = jest.fn();
+    mockGetVariantAnalysisStatus = jest.fn();
 
     logger = createMockLogger();
 
@@ -54,6 +56,7 @@ describe("Variant Analysis Monitor", () => {
         logger,
       }),
       shouldCancelMonitor,
+      mockGetVariantAnalysisStatus,
     );
     variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy);
 
@@ -128,7 +131,11 @@ describe("Variant Analysis Monitor", () => {
             index + 1,
             "codeQL.autoDownloadVariantAnalysisResult",
             mapScannedRepository(succeededRepo),
-            mapUpdatedVariantAnalysis(variantAnalysis, mockApiResponse),
+            mapUpdatedVariantAnalysis(
+              variantAnalysis,
+              variantAnalysis.status,
+              mockApiResponse,
+            ),
           );
         });
       });
@@ -336,6 +343,24 @@ describe("Variant Analysis Monitor", () => {
         );
       });
     });
+
+    describe("cancelation", () => {
+      it("should maintain canceling status", async () => {
+        mockGetVariantAnalysisStatus.mockReturnValueOnce(
+          VariantAnalysisStatus.Canceling,
+        );
+        mockApiResponse = createMockApiResponse("in_progress");
+        mockGetVariantAnalysis.mockResolvedValue(mockApiResponse);
+
+        await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis);
+
+        expect(onVariantAnalysisChangeSpy).toHaveBeenCalledWith(
+          expect.objectContaining({
+            status: VariantAnalysisStatus.Canceling,
+          }),
+        );
+      });
+    });
   });
 
   function limitNumberOfAttemptsToMonitor() {