diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index b690eaeda81..6590aa1b661 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -3,6 +3,12 @@ import type { Uri, Range } from "vscode"; import type { DbTreeViewItem } from "../databases/ui/db-tree-view-item"; import type { DatabaseItem } from "../local-databases"; import type { QueryHistoryInfo } from "../query-history/query-history-info"; +import type { RepositoriesFilterSortStateWithIds } from "../pure/variant-analysis-filter-sort"; +import type { + VariantAnalysis, + VariantAnalysisScannedRepository, + VariantAnalysisScannedRepositoryResult, +} from "../variant-analysis/shared/variant-analysis"; // A command function matching the signature that VS Code calls when // a command on a selection is invoked. @@ -123,9 +129,27 @@ export type LocalDatabasesCommands = { // Commands tied to variant analysis export type VariantAnalysisCommands = { + "codeQL.autoDownloadVariantAnalysisResult": ( + scannedRepo: VariantAnalysisScannedRepository, + variantAnalysisSummary: VariantAnalysis, + ) => Promise; + "codeQL.copyVariantAnalysisRepoList": ( + variantAnalysisId: number, + filterSort?: RepositoriesFilterSortStateWithIds, + ) => Promise; + "codeQL.loadVariantAnalysisRepoResults": ( + variantAnalysisId: number, + repositoryFullName: string, + ) => Promise; + "codeQL.monitorVariantAnalysis": ( + variantAnalysis: VariantAnalysis, + ) => Promise; "codeQL.openVariantAnalysisLogs": ( variantAnalysisId: number, ) => Promise; + "codeQL.openVariantAnalysisView": ( + variantAnalysisId: number, + ) => Promise; "codeQL.runVariantAnalysis": (uri?: Uri) => Promise; "codeQL.runVariantAnalysisContextEditor": (uri?: Uri) => Promise; }; diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 26a5a6a3863..e87e75bd283 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -110,16 +110,11 @@ import { NewQueryRunner } from "./query-server/query-runner"; import { QueryRunner } from "./queryRunner"; import { VariantAnalysisView } from "./variant-analysis/variant-analysis-view"; import { VariantAnalysisViewSerializer } from "./variant-analysis/variant-analysis-view-serializer"; -import { - VariantAnalysis, - VariantAnalysisScannedRepository, -} from "./variant-analysis/shared/variant-analysis"; import { VariantAnalysisManager } from "./variant-analysis/variant-analysis-manager"; import { createVariantAnalysisContentProvider } from "./variant-analysis/variant-analysis-content-provider"; import { VSCodeMockGitHubApiServer } from "./mocks/vscode-mock-gh-api-server"; import { VariantAnalysisResultsManager } from "./variant-analysis/variant-analysis-results-manager"; import { ExtensionApp } from "./common/vscode/vscode-app"; -import { RepositoriesFilterSortStateWithIds } from "./pure/variant-analysis-filter-sort"; import { DbModule } from "./databases/db-module"; import { redactableError } from "./pure/errors"; import { QueryHistoryDirs } from "./query-history/query-history-dirs"; @@ -847,72 +842,6 @@ async function activateWithInstalledDistribution( ); } - ctx.subscriptions.push( - commandRunner( - "codeQL.copyVariantAnalysisRepoList", - async ( - variantAnalysisId: number, - filterSort?: RepositoriesFilterSortStateWithIds, - ) => { - await variantAnalysisManager.copyRepoListToClipboard( - variantAnalysisId, - filterSort, - ); - }, - ), - ); - - ctx.subscriptions.push( - commandRunner( - "codeQL.monitorVariantAnalysis", - async (variantAnalysis: VariantAnalysis, token: CancellationToken) => { - await variantAnalysisManager.monitorVariantAnalysis( - variantAnalysis, - token, - ); - }, - ), - ); - - ctx.subscriptions.push( - commandRunner( - "codeQL.autoDownloadVariantAnalysisResult", - async ( - scannedRepo: VariantAnalysisScannedRepository, - variantAnalysisSummary: VariantAnalysis, - token: CancellationToken, - ) => { - await variantAnalysisManager.enqueueDownload( - scannedRepo, - variantAnalysisSummary, - token, - ); - }, - ), - ); - - ctx.subscriptions.push( - commandRunner( - "codeQL.loadVariantAnalysisRepoResults", - async (variantAnalysisId: number, repositoryFullName: string) => { - await variantAnalysisManager.loadResults( - variantAnalysisId, - repositoryFullName, - ); - }, - ), - ); - - // The "openVariantAnalysisView" command is internal-only. - ctx.subscriptions.push( - commandRunner( - "codeQL.openVariantAnalysisView", - async (variantAnalysisId: number) => { - await variantAnalysisManager.showView(variantAnalysisId); - }, - ), - ); - ctx.subscriptions.push( commandRunner("codeQL.openReferencedFile", async (selectedQuery: Uri) => { await openReferencedFile(qs, cliServer, selectedQuery); 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 9bd7f201d63..af4eea987bb 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -131,14 +131,19 @@ export class VariantAnalysisManager getCommands(): VariantAnalysisCommands { return { - "codeQL.openVariantAnalysisLogs": async (variantAnalysisId: number) => { - await this.openVariantAnalysisLogs(variantAnalysisId); - }, - "codeQL.runVariantAnalysis": async (uri?: Uri) => - this.runVariantAnalysisFromCommand(uri), + "codeQL.autoDownloadVariantAnalysisResult": + this.enqueueDownload.bind(this), + "codeQL.copyVariantAnalysisRepoList": + this.copyRepoListToClipboard.bind(this), + "codeQL.loadVariantAnalysisRepoResults": this.loadResults.bind(this), + "codeQL.monitorVariantAnalysis": this.monitorVariantAnalysis.bind(this), + "codeQL.openVariantAnalysisLogs": this.openVariantAnalysisLogs.bind(this), + "codeQL.openVariantAnalysisView": this.showView.bind(this), + "codeQL.runVariantAnalysis": + this.runVariantAnalysisFromCommand.bind(this), // Since we are tracking extension usage through commands, this command mirrors the "codeQL.runVariantAnalysis" command - "codeQL.runVariantAnalysisContextEditor": async (uri?: Uri) => - this.runVariantAnalysisFromCommand(uri), + "codeQL.runVariantAnalysisContextEditor": + this.runVariantAnalysisFromCommand.bind(this), }; } @@ -496,19 +501,16 @@ export class VariantAnalysisManager public async monitorVariantAnalysis( variantAnalysis: VariantAnalysis, - cancellationToken: CancellationToken, ): Promise { await this.variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, this.app.credentials, - cancellationToken, ); } public async autoDownloadVariantAnalysisResult( scannedRepo: VariantAnalysisScannedRepository, variantAnalysis: VariantAnalysis, - cancellationToken: CancellationToken, ): Promise { if ( this.repoStates.get(variantAnalysis.id)?.[scannedRepo.repository.id] @@ -525,13 +527,6 @@ export class VariantAnalysisManager await this.onRepoStateUpdated(variantAnalysis.id, repoState); - if (cancellationToken && cancellationToken.isCancellationRequested) { - repoState.downloadStatus = - VariantAnalysisScannedRepositoryDownloadStatus.Failed; - await this.onRepoStateUpdated(variantAnalysis.id, repoState); - return; - } - let repoTask: VariantAnalysisRepositoryTask; try { const repoTaskResponse = await getVariantAnalysisRepo( @@ -606,14 +601,9 @@ export class VariantAnalysisManager public async enqueueDownload( scannedRepo: VariantAnalysisScannedRepository, variantAnalysis: VariantAnalysis, - token: CancellationToken, ): Promise { await this.queue.add(() => - this.autoDownloadVariantAnalysisResult( - scannedRepo, - variantAnalysis, - token, - ), + this.autoDownloadVariantAnalysisResult(scannedRepo, variantAnalysis), ); } 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 f7683fb4b67..7fa8f3b1470 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts @@ -1,4 +1,4 @@ -import { CancellationToken, commands, EventEmitter } from "vscode"; +import { commands, EventEmitter } from "vscode"; import { getVariantAnalysis } from "./gh-api/gh-api-client"; import { @@ -37,7 +37,6 @@ export class VariantAnalysisMonitor extends DisposableObject { public async monitorVariantAnalysis( variantAnalysis: VariantAnalysis, credentials: Credentials, - cancellationToken: CancellationToken, ): Promise { let attemptCount = 0; const scannedReposDownloaded: number[] = []; @@ -45,10 +44,6 @@ export class VariantAnalysisMonitor extends DisposableObject { while (attemptCount <= VariantAnalysisMonitor.maxAttemptCount) { await sleep(VariantAnalysisMonitor.sleepTime); - if (cancellationToken && cancellationToken.isCancellationRequested) { - return; - } - if (await this.shouldCancelMonitor(variantAnalysis.id)) { return; } 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 024a1bc823e..05e9b5ed686 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 @@ -1,5 +1,4 @@ import { - CancellationTokenSource, commands, env, extensions, @@ -54,7 +53,6 @@ jest.setTimeout(3 * 60 * 1000); describe("Variant Analysis Manager", () => { let app: App; - let cancellationTokenSource: CancellationTokenSource; let variantAnalysisManager: VariantAnalysisManager; let variantAnalysisResultsManager: VariantAnalysisResultsManager; let variantAnalysis: VariantAnalysis; @@ -63,8 +61,6 @@ describe("Variant Analysis Manager", () => { beforeEach(async () => { jest.spyOn(extLogger, "log").mockResolvedValue(undefined); - cancellationTokenSource = new CancellationTokenSource(); - scannedRepos = createMockScannedRepos(); variantAnalysis = createMockVariantAnalysis({ status: VariantAnalysisStatus.InProgress, @@ -203,7 +199,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); expect(getVariantAnalysisRepoResultStub).not.toHaveBeenCalled(); @@ -228,23 +223,10 @@ describe("Variant Analysis Manager", () => { getVariantAnalysisRepoResultStub.mockResolvedValue(response); }); - it("should return early if variant analysis is cancelled", async () => { - cancellationTokenSource.cancel(); - - await variantAnalysisManager.autoDownloadVariantAnalysisResult( - scannedRepos[0], - variantAnalysis, - cancellationTokenSource.token, - ); - - expect(getVariantAnalysisRepoStub).not.toHaveBeenCalled(); - }); - it("should fetch a repo task", async () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); expect(getVariantAnalysisRepoStub).toHaveBeenCalled(); @@ -254,7 +236,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); expect(getVariantAnalysisRepoResultStub).toHaveBeenCalled(); @@ -265,7 +246,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); getVariantAnalysisRepoStub.mockClear(); @@ -273,7 +253,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); expect(getVariantAnalysisRepoStub).not.toHaveBeenCalled(); @@ -283,7 +262,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); await expect(fs.readJson(repoStatesPath)).resolves.toEqual({ @@ -304,7 +282,6 @@ describe("Variant Analysis Manager", () => { variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ), ).rejects.toThrow(); @@ -320,7 +297,6 @@ describe("Variant Analysis Manager", () => { variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ), ).rejects.toThrow(); @@ -329,7 +305,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[1], variantAnalysis, - cancellationTokenSource.token, ); await expect(fs.readJson(repoStatesPath)).resolves.toEqual({ @@ -355,7 +330,6 @@ describe("Variant Analysis Manager", () => { variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ), ).rejects.toThrow(); @@ -364,7 +338,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[1], variantAnalysis, - cancellationTokenSource.token, ); await expect(fs.readJson(repoStatesPath)).resolves.toEqual({ @@ -400,7 +373,6 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.autoDownloadVariantAnalysisResult( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); await expect(fs.readJson(repoStatesPath)).resolves.toEqual({ @@ -439,17 +411,14 @@ describe("Variant Analysis Manager", () => { await variantAnalysisManager.enqueueDownload( scannedRepos[0], variantAnalysis, - cancellationTokenSource.token, ); await variantAnalysisManager.enqueueDownload( scannedRepos[1], variantAnalysis, - cancellationTokenSource.token, ); await variantAnalysisManager.enqueueDownload( scannedRepos[2], variantAnalysis, - cancellationTokenSource.token, ); expect(variantAnalysisManager.downloadsQueueSize()).toBe(0); 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 0c9082c91ac..329e8568583 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 @@ -1,4 +1,4 @@ -import { CancellationTokenSource, commands, extensions } from "vscode"; +import { commands, extensions } from "vscode"; import { CodeQLExtensionInterface } from "../../../../src/extension"; import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client"; @@ -33,7 +33,6 @@ describe("Variant Analysis Monitor", () => { let mockGetVariantAnalysis: jest.SpiedFunction< typeof ghApiClient.getVariantAnalysis >; - let cancellationTokenSource: CancellationTokenSource; let variantAnalysisMonitor: VariantAnalysisMonitor; let shouldCancelMonitor: jest.Mock, [number]>; let variantAnalysis: VariantAnalysis; @@ -45,8 +44,6 @@ describe("Variant Analysis Monitor", () => { const onVariantAnalysisChangeSpy = jest.fn(); beforeEach(async () => { - cancellationTokenSource = new CancellationTokenSource(); - variantAnalysis = createMockVariantAnalysis({}); shouldCancelMonitor = jest.fn(); @@ -71,25 +68,12 @@ describe("Variant Analysis Monitor", () => { limitNumberOfAttemptsToMonitor(); }); - it("should return early if variant analysis is cancelled", async () => { - cancellationTokenSource.cancel(); - - await variantAnalysisMonitor.monitorVariantAnalysis( - variantAnalysis, - testCredentialsWithStub(), - cancellationTokenSource.token, - ); - - expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled(); - }); - it("should return early if variant analysis should be cancelled", async () => { shouldCancelMonitor.mockResolvedValue(true); await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(onVariantAnalysisChangeSpy).not.toHaveBeenCalled(); @@ -107,7 +91,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(mockGetVariantAnalysis).toHaveBeenCalledTimes(1); @@ -157,7 +140,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(commandSpy).toBeCalledTimes(succeededRepos.length); @@ -176,7 +158,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(mockGetDownloadResult).toBeCalledTimes(succeededRepos.length); @@ -186,7 +167,6 @@ describe("Variant Analysis Monitor", () => { index + 1, processScannedRepository(succeededRepo), processUpdatedVariantAnalysis(variantAnalysis, mockApiResponse), - undefined, ); }); }); @@ -209,7 +189,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(commandSpy).not.toHaveBeenCalled(); @@ -219,7 +198,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(mockGetDownloadResult).not.toBeCalled(); @@ -278,7 +256,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(mockGetVariantAnalysis).toBeCalledTimes(4); @@ -297,7 +274,6 @@ describe("Variant Analysis Monitor", () => { await variantAnalysisMonitor.monitorVariantAnalysis( variantAnalysis, testCredentialsWithStub(), - cancellationTokenSource.token, ); expect(mockGetDownloadResult).not.toBeCalled();