From a375afd61baa677172125264201da0e80ac68209 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 17 Mar 2023 15:38:08 +0000 Subject: [PATCH 1/2] Remove codeQL.cancelVariantAnalysis command --- extensions/ql-vscode/src/extension.ts | 9 ------- .../query-history/query-history-manager.ts | 3 +-- .../variant-analysis-view-manager.ts | 1 + .../variant-analysis/variant-analysis-view.ts | 5 +--- .../view/variant-analysis/VariantAnalysis.tsx | 1 + .../query-history-manager.test.ts | 26 ++++++++++--------- 6 files changed, 18 insertions(+), 27 deletions(-) diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index b6522377f..d85f05beb 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -1145,15 +1145,6 @@ async function activateWithInstalledDistribution( ), ); - ctx.subscriptions.push( - commandRunner( - "codeQL.cancelVariantAnalysis", - async (variantAnalysisId: number) => { - await variantAnalysisManager.cancelVariantAnalysis(variantAnalysisId); - }, - ), - ); - ctx.subscriptions.push( commandRunner("codeQL.exportSelectedVariantAnalysisResults", async () => { await exportSelectedVariantAnalysisResults(qhm); diff --git a/extensions/ql-vscode/src/query-history/query-history-manager.ts b/extensions/ql-vscode/src/query-history/query-history-manager.ts index 44529bdf1..cb702fa05 100644 --- a/extensions/ql-vscode/src/query-history/query-history-manager.ts +++ b/extensions/ql-vscode/src/query-history/query-history-manager.ts @@ -1008,8 +1008,7 @@ export class QueryHistoryManager extends DisposableObject { if (item.t === "local") { item.cancel(); } else if (item.t === "variant-analysis") { - await commands.executeCommand( - "codeQL.cancelVariantAnalysis", + await this.variantAnalysisManager.cancelVariantAnalysis( item.variantAnalysis.id, ); } else { diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts index 1fa1fe46d..07aa1be33 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts @@ -25,4 +25,5 @@ export interface VariantAnalysisViewManager< variantAnalysisId: number, ): Promise; openQueryFile(variantAnalysisId: number): Promise; + cancelVariantAnalysis(variantAnalysisId: number): Promise; } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index 351e5afbf..858256080 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -110,10 +110,7 @@ export class VariantAnalysisView break; case "cancelVariantAnalysis": - void commands.executeCommand( - "codeQL.cancelVariantAnalysis", - this.variantAnalysisId, - ); + await this.manager.cancelVariantAnalysis(this.variantAnalysisId); break; case "requestRepositoryResults": void commands.executeCommand( diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx index 6c2f27051..73aed5974 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx @@ -38,6 +38,7 @@ const stopQuery = () => { vscode.postMessage({ t: "cancelVariantAnalysis", }); + sendTelemetry("variant-analysis-cancel"); }; const openLogs = () => { diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts index b02ba6833..60f0095aa 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts @@ -41,6 +41,9 @@ describe("QueryHistoryManager", () => { let executeCommandSpy: jest.SpiedFunction< typeof vscode.commands.executeCommand >; + let cancelVariantAnalysisSpy: jest.SpiedFunction< + typeof variantAnalysisManagerStub.cancelVariantAnalysis + >; const doCompareCallback = jest.fn(); let queryHistoryManager: QueryHistoryManager; @@ -82,9 +85,14 @@ describe("QueryHistoryManager", () => { onVariantAnalysisStatusUpdated: jest.fn(), onVariantAnalysisRemoved: jest.fn(), removeVariantAnalysis: jest.fn(), + cancelVariantAnalysis: jest.fn(), showView: jest.fn(), } as any as VariantAnalysisManager; + cancelVariantAnalysisSpy = jest + .spyOn(variantAnalysisManagerStub, "cancelVariantAnalysis") + .mockResolvedValue(undefined); + localQueryHistory = [ // completed createMockLocalQueryInfo({ @@ -729,8 +737,7 @@ describe("QueryHistoryManager", () => { const inProgress1 = variantAnalysisHistory[1]; await queryHistoryManager.handleCancel(inProgress1, [inProgress1]); - expect(executeCommandSpy).toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).toBeCalledWith( inProgress1.variantAnalysis.id, ); }); @@ -746,12 +753,10 @@ describe("QueryHistoryManager", () => { inProgress1, inProgress2, ]); - expect(executeCommandSpy).toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).toBeCalledWith( inProgress1.variantAnalysis.id, ); - expect(executeCommandSpy).toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).toBeCalledWith( inProgress2.variantAnalysis.id, ); }); @@ -793,8 +798,7 @@ describe("QueryHistoryManager", () => { await queryHistoryManager.handleCancel(completedVariantAnalysis, [ completedVariantAnalysis, ]); - expect(executeCommandSpy).not.toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).not.toBeCalledWith( completedVariantAnalysis.variantAnalysis, ); }); @@ -810,12 +814,10 @@ describe("QueryHistoryManager", () => { completedVariantAnalysis, failedVariantAnalysis, ]); - expect(executeCommandSpy).not.toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).not.toBeCalledWith( completedVariantAnalysis.variantAnalysis.id, ); - expect(executeCommandSpy).not.toBeCalledWith( - "codeQL.cancelVariantAnalysis", + expect(cancelVariantAnalysisSpy).not.toBeCalledWith( failedVariantAnalysis.variantAnalysis.id, ); }); From 94db1dff73a21a89c1ec4d6fc3631677833bd706 Mon Sep 17 00:00:00 2001 From: Robert Date: Fri, 17 Mar 2023 15:51:00 +0000 Subject: [PATCH 2/2] Remove codeQL.openVariantAnalysisQueryText command --- extensions/ql-vscode/src/extension.ts | 9 --------- .../src/query-history/query-history-manager.ts | 3 +-- .../variant-analysis-view-manager.ts | 1 + .../src/variant-analysis/variant-analysis-view.ts | 5 +---- .../src/view/variant-analysis/VariantAnalysis.tsx | 1 + .../query-history/variant-analysis-history.test.ts | 14 ++++++++------ 6 files changed, 12 insertions(+), 21 deletions(-) diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index d85f05beb..ba03356a9 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -1198,15 +1198,6 @@ async function activateWithInstalledDistribution( ), ); - ctx.subscriptions.push( - commandRunner( - "codeQL.openVariantAnalysisQueryText", - async (variantAnalysisId: number) => { - await variantAnalysisManager.openQueryText(variantAnalysisId); - }, - ), - ); - ctx.subscriptions.push( commandRunner("codeQL.openReferencedFile", async (selectedQuery: Uri) => { await openReferencedFile(qs, cliServer, selectedQuery); diff --git a/extensions/ql-vscode/src/query-history/query-history-manager.ts b/extensions/ql-vscode/src/query-history/query-history-manager.ts index cb702fa05..d5d7ff19a 100644 --- a/extensions/ql-vscode/src/query-history/query-history-manager.ts +++ b/extensions/ql-vscode/src/query-history/query-history-manager.ts @@ -1034,8 +1034,7 @@ export class QueryHistoryManager extends DisposableObject { } if (finalSingleItem.t === "variant-analysis") { - await commands.executeCommand( - "codeQL.openVariantAnalysisQueryText", + await this.variantAnalysisManager.openQueryText( finalSingleItem.variantAnalysis.id, ); return; diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts index 07aa1be33..c47220455 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view-manager.ts @@ -25,5 +25,6 @@ export interface VariantAnalysisViewManager< variantAnalysisId: number, ): Promise; openQueryFile(variantAnalysisId: number): Promise; + openQueryText(variantAnalysisId: number): Promise; cancelVariantAnalysis(variantAnalysisId: number): Promise; } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index 858256080..f4f6e7280 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -123,10 +123,7 @@ export class VariantAnalysisView await this.manager.openQueryFile(this.variantAnalysisId); break; case "openQueryText": - void commands.executeCommand( - "codeQL.openVariantAnalysisQueryText", - this.variantAnalysisId, - ); + await this.manager.openQueryText(this.variantAnalysisId); break; case "copyRepositoryList": void commands.executeCommand( diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx index 73aed5974..4e932accb 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx @@ -32,6 +32,7 @@ const openQueryText = () => { vscode.postMessage({ t: "openQueryText", }); + sendTelemetry("variant-analysis-open-query-text"); }; const stopQuery = () => { diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/variant-analysis-history.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/variant-analysis-history.test.ts index 6441d2fea..f36267065 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/variant-analysis-history.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/variant-analysis-history.test.ts @@ -8,7 +8,7 @@ import { } from "fs-extra"; import { join } from "path"; -import { commands, ExtensionContext, Uri } from "vscode"; +import { ExtensionContext, Uri } from "vscode"; import { DatabaseManager } from "../../../../src/local-databases"; import { tmpDir, walkDirectory } from "../../../../src/helpers"; import { DisposableBucket } from "../../disposable-bucket"; @@ -54,9 +54,12 @@ describe("Variant Analyses and QueryHistoryManager", () => { rehydrateVariantAnalysis: rehydrateVariantAnalysisStub, onVariantAnalysisStatusUpdated: jest.fn(), showView: showViewStub, + openQueryText: jest.fn(), } as any as VariantAnalysisManager; - let executeCommandSpy: jest.SpiedFunction; + let openQueryTextSpy: jest.SpiedFunction< + typeof variantAnalysisManagerStub.openQueryText + >; beforeEach(async () => { // Since these tests change the state of the query history manager, we need to copy the original @@ -95,8 +98,8 @@ describe("Variant Analyses and QueryHistoryManager", () => { ); disposables.push(qhm); - executeCommandSpy = jest - .spyOn(commands, "executeCommand") + openQueryTextSpy = jest + .spyOn(variantAnalysisManagerStub, "openQueryText") .mockResolvedValue(undefined); }); @@ -180,8 +183,7 @@ describe("Variant Analyses and QueryHistoryManager", () => { await qhm.readQueryHistory(); await qhm.handleShowQueryText(qhm.treeDataProvider.allHistory[0], []); - expect(executeCommandSpy).toHaveBeenCalledWith( - "codeQL.openVariantAnalysisQueryText", + expect(openQueryTextSpy).toHaveBeenCalledWith( rawQueryHistory[0].variantAnalysis.id, ); });