diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index 15cf3b35754..8d324491d6c 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/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) diff --git a/extensions/ql-vscode/src/query-history/query-status.ts b/extensions/ql-vscode/src/query-history/query-status.ts index c68fb9fd7b0..732b1d5f945 100644 --- a/extensions/ql-vscode/src/query-history/query-status.ts +++ b/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: diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts index 329a3404755..6ca7c63132e 100644 --- a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts +++ b/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: diff --git a/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisActions.stories.tsx b/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisActions.stories.tsx index 1e4eec19f78..daeb84e54b6 100644 --- a/extensions/ql-vscode/src/stories/variant-analysis/VariantAnalysisActions.stories.tsx +++ b/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, +}; diff --git a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts index 0da27a50afd..3a698d24cc7 100644 --- a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts +++ b/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", } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index 7dc7eda4851..82533e90135 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/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 { @@ -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.", ); - await cancelVariantAnalysis(this.app.credentials, variantAnalysis); + try { + await cancelVariantAnalysis(this.app.credentials, variantAnalysis); + } catch (e) { + await this.onVariantAnalysisUpdated({ + ...variantAnalysis, + status: VariantAnalysisStatus.InProgress, + }); + throw e; + } } public async openVariantAnalysisLogs(variantAnalysisId: number) { diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts index 22044fed2c8..7fee8186167 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts +++ b/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, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts index 9cfb7100ec6..a91b948cd7f 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts +++ b/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, + 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, ); diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx index d324e32158c..ed1b473c66a 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx @@ -103,6 +103,11 @@ export const VariantAnalysisActions = ({ Stop query )} + {variantAnalysisStatus === VariantAnalysisStatus.Canceling && ( + + )} ); }; diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx index eea7dd41c71..49020c138f7 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStats.tsx +++ b/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"; } diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStatusStats.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStatusStats.tsx index 0ee714ada0d..3969fd7951e 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStatusStats.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisStatusStats.tsx @@ -28,7 +28,8 @@ export const VariantAnalysisStatusStats = ({ }: VariantAnalysisStatusStatsProps) => { return ( - {variantAnalysisStatus === VariantAnalysisStatus.InProgress ? ( + {variantAnalysisStatus === VariantAnalysisStatus.InProgress || + variantAnalysisStatus === VariantAnalysisStatus.Canceling ? (
diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisActions.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisActions.spec.tsx index 9f23c3eec80..de19458646e 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisActions.spec.tsx +++ b/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, diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx index fd2bce56b99..e3a9f3a4abf 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx +++ b/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 }); diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts index 1ca034b4270..24b9567ba9f 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts +++ b/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", () => { diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts index ac7e3957e6a..b24bdb12f7e 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts +++ b/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, [number]>; + let mockGetVariantAnalysisStatus: jest.Mock; 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() {