From c93449ab9f9b094feb5ba846ed06f75f529b2527 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 19 Jun 2023 17:13:40 +0100 Subject: [PATCH 1/3] Remove ProgressCallback / CancellationToken arguments where they aren't used --- .../data-extensions-editor-view.ts | 12 +--- .../src/databases/database-fetcher.ts | 16 +---- .../src/databases/local-databases-ui.ts | 31 +++------- .../local-databases/database-manager.ts | 60 +++++------------- .../src/local-queries/local-queries.ts | 3 +- .../legacy/legacy-query-runner.ts | 29 ++------- .../src/query-server/legacy/run-queries.ts | 3 +- .../src/query-server/new-query-runner.ts | 29 ++------- .../src/query-server/query-runner.ts | 14 +---- .../src/query-testing/test-runner.ts | 17 +---- .../ql-vscode/src/skeleton-query-wizard.ts | 4 +- .../databases/database-fetcher.test.ts | 4 +- .../skeleton-query-wizard.test.ts | 4 -- .../test/vscode-tests/global.helper.ts | 14 +---- .../minimal-workspace/local-databases.test.ts | 62 +++---------------- .../no-workspace/run-queries.test.ts | 12 +--- 16 files changed, 60 insertions(+), 254 deletions(-) diff --git a/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts b/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts index 5464604343c..aba17154158 100644 --- a/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts +++ b/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts @@ -300,7 +300,6 @@ export class DataExtensionsEditorView extends AbstractWebview< this.app.workspaceStoragePath ?? this.app.globalStoragePath, this.app.credentials, (update) => this.showProgress(update), - tokenSource.token, this.cliServer, ); if (!database) { @@ -354,16 +353,7 @@ export class DataExtensionsEditorView extends AbstractWebview< // After the flow model has been generated, we can remove the temporary database // which we used for generating the flow model. - await this.databaseManager.removeDatabaseItem( - () => - this.showProgress({ - step: 3900, - maxStep: 4000, - message: "Removing temporary database", - }), - tokenSource.token, - database, - ); + await this.databaseManager.removeDatabaseItem(database); await this.clearProgress(); } diff --git a/extensions/ql-vscode/src/databases/database-fetcher.ts b/extensions/ql-vscode/src/databases/database-fetcher.ts index dc3674fb4f5..b7b7724f5d3 100644 --- a/extensions/ql-vscode/src/databases/database-fetcher.ts +++ b/extensions/ql-vscode/src/databases/database-fetcher.ts @@ -1,7 +1,7 @@ import fetch, { Response } from "node-fetch"; import { zip } from "zip-a-folder"; import { Open } from "unzipper"; -import { Uri, CancellationToken, window, InputBoxOptions } from "vscode"; +import { Uri, window, InputBoxOptions } from "vscode"; import { CodeQLCliServer } from "../codeql-cli/cli"; import { ensureDir, @@ -44,7 +44,6 @@ export async function promptImportInternetDatabase( databaseManager: DatabaseManager, storagePath: string, progress: ProgressCallback, - token: CancellationToken, cli?: CodeQLCliServer, ): Promise { const databaseUrl = await window.showInputBox({ @@ -63,7 +62,6 @@ export async function promptImportInternetDatabase( storagePath, undefined, progress, - token, cli, ); @@ -86,7 +84,6 @@ export async function promptImportInternetDatabase( * @param storagePath where to store the unzipped database. * @param credentials the credentials to use to authenticate with GitHub * @param progress the progress callback - * @param token the cancellation token * @param cli the CodeQL CLI server */ export async function promptImportGithubDatabase( @@ -95,7 +92,6 @@ export async function promptImportGithubDatabase( storagePath: string, credentials: Credentials | undefined, progress: ProgressCallback, - token: CancellationToken, cli?: CodeQLCliServer, ): Promise { const githubRepo = await askForGitHubRepo(progress); @@ -109,7 +105,6 @@ export async function promptImportGithubDatabase( storagePath, credentials, progress, - token, cli, ); @@ -157,7 +152,6 @@ export async function askForGitHubRepo( * @param storagePath where to store the unzipped database. * @param credentials the credentials to use to authenticate with GitHub * @param progress the progress callback - * @param token the cancellation token * @param cli the CodeQL CLI server * @param language the language to download. If undefined, the user will be prompted to choose a language. **/ @@ -167,7 +161,6 @@ export async function downloadGitHubDatabase( storagePath: string, credentials: Credentials | undefined, progress: ProgressCallback, - token: CancellationToken, cli?: CodeQLCliServer, language?: string, ): Promise { @@ -213,7 +206,6 @@ export async function downloadGitHubDatabase( storagePath, `${owner}/${name}`, progress, - token, cli, ); } @@ -231,7 +223,6 @@ export async function importArchiveDatabase( databaseManager: DatabaseManager, storagePath: string, progress: ProgressCallback, - token: CancellationToken, cli?: CodeQLCliServer, ): Promise { try { @@ -242,7 +233,6 @@ export async function importArchiveDatabase( storagePath, undefined, progress, - token, cli, ); if (item) { @@ -275,7 +265,6 @@ export async function importArchiveDatabase( * @param storagePath where to store the unzipped database. * @param nameOverride a name for the database that overrides the default * @param progress callback to send progress messages to - * @param token cancellation token */ async function databaseArchiveFetcher( databaseUrl: string, @@ -284,7 +273,6 @@ async function databaseArchiveFetcher( storagePath: string, nameOverride: string | undefined, progress: ProgressCallback, - token: CancellationToken, cli?: CodeQLCliServer, ): Promise { progress({ @@ -327,8 +315,6 @@ async function databaseArchiveFetcher( const makeSelected = true; const item = await databaseManager.openDatabase( - progress, - token, Uri.file(dbPath), makeSelected, nameOverride, diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 76bddf3b237..826bf9c44c9 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -314,7 +314,7 @@ export class DatabaseUI extends DisposableObject { private async handleSetDefaultTourDatabase(): Promise { return withProgress( - async (progress, token) => { + async () => { try { if (!workspace.workspaceFolders?.length) { throw new Error("No workspace folder is open."); @@ -332,8 +332,6 @@ export class DatabaseUI extends DisposableObject { const isTutorialDatabase = true; await this.databaseManager.openDatabase( - progress, - token, uri, makeSelected, nameOverride, @@ -485,13 +483,12 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseInternet(): Promise { return withProgress( - async (progress, token) => { + async (progress) => { await promptImportInternetDatabase( this.app.commands, this.databaseManager, this.storagePath, progress, - token, this.queryServer?.cliServer, ); }, @@ -503,7 +500,7 @@ export class DatabaseUI extends DisposableObject { private async handleChooseDatabaseGithub(): Promise { return withProgress( - async (progress, token) => { + async (progress) => { const credentials = isCanary() ? this.app.credentials : undefined; await promptImportGithubDatabase( @@ -512,7 +509,6 @@ export class DatabaseUI extends DisposableObject { this.storagePath, credentials, progress, - token, this.queryServer?.cliServer, ); }, @@ -608,14 +604,13 @@ export class DatabaseUI extends DisposableObject { private async handleClearCache(): Promise { return withProgress( - async (progress, token) => { + async (_progress, token) => { if ( this.queryServer !== undefined && this.databaseManager.currentDatabaseItem !== undefined ) { await this.queryServer.clearCacheInDatabase( this.databaseManager.currentDatabaseItem, - progress, token, ); } @@ -633,7 +628,7 @@ export class DatabaseUI extends DisposableObject { private async handleSetCurrentDatabase(uri: Uri): Promise { return withProgress( - async (progress, token) => { + async (progress) => { try { // Assume user has selected an archive if the file has a .zip extension if (uri.path.endsWith(".zip")) { @@ -643,11 +638,10 @@ export class DatabaseUI extends DisposableObject { this.databaseManager, this.storagePath, progress, - token, this.queryServer?.cliServer, ); } else { - await this.databaseManager.openDatabase(progress, token, uri); + await this.databaseManager.openDatabase(uri); } } catch (e) { // rethrow and let this be handled by default error handling. @@ -668,10 +662,10 @@ export class DatabaseUI extends DisposableObject { databaseItems: DatabaseItem[], ): Promise { return withProgress( - async (progress, token) => { + async () => { await Promise.all( databaseItems.map((dbItem) => - this.databaseManager.removeDatabaseItem(progress, token, dbItem), + this.databaseManager.removeDatabaseItem(dbItem), ), ); }, @@ -758,15 +752,11 @@ export class DatabaseUI extends DisposableObject { return await withInheritedProgress( progress, - async (progress, token) => { + async (progress) => { if (byFolder) { const fixedUri = await this.fixDbUri(uri); // we are selecting a database folder - return await this.databaseManager.openDatabase( - progress, - token, - fixedUri, - ); + return await this.databaseManager.openDatabase(fixedUri); } else { // we are selecting a database archive. Must unzip into a workspace-controlled area // before importing. @@ -776,7 +766,6 @@ export class DatabaseUI extends DisposableObject { this.databaseManager, this.storagePath, progress, - token, this.queryServer?.cliServer, ); } diff --git a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts index d6aabf31d62..0deb966aba2 100644 --- a/extensions/ql-vscode/src/databases/local-databases/database-manager.ts +++ b/extensions/ql-vscode/src/databases/local-databases/database-manager.ts @@ -104,8 +104,6 @@ export class DatabaseManager extends DisposableObject { * databases. */ public async openDatabase( - progress: ProgressCallback, - token: vscode.CancellationToken, uri: vscode.Uri, makeSelected = true, displayName?: string, @@ -115,9 +113,7 @@ export class DatabaseManager extends DisposableObject { return await this.addExistingDatabaseItem( databaseItem, - progress, makeSelected, - token, isTutorialDatabase, ); } @@ -130,9 +126,7 @@ export class DatabaseManager extends DisposableObject { */ private async addExistingDatabaseItem( databaseItem: DatabaseItemImpl, - progress: ProgressCallback, makeSelected: boolean, - token: vscode.CancellationToken, isTutorialDatabase?: boolean, ): Promise { const existingItem = this.findDatabaseItem(databaseItem.databaseUri); @@ -143,7 +137,7 @@ export class DatabaseManager extends DisposableObject { return existingItem; } - await this.addDatabaseItem(progress, token, databaseItem); + await this.addDatabaseItem(databaseItem); if (makeSelected) { await this.setCurrentDatabaseItem(databaseItem); } @@ -260,14 +254,11 @@ export class DatabaseManager extends DisposableObject { } } - private async reregisterDatabases( - progress: ProgressCallback, - token: vscode.CancellationToken, - ) { + private async reregisterDatabases(progress: ProgressCallback) { let completed = 0; await Promise.all( this._databaseItems.map(async (databaseItem) => { - await this.registerDatabase(progress, token, databaseItem); + await this.registerDatabase(databaseItem); completed++; progress({ maxStep: this._databaseItems.length, @@ -324,8 +315,6 @@ export class DatabaseManager extends DisposableObject { } private async createDatabaseItemFromPersistedState( - progress: ProgressCallback, - token: vscode.CancellationToken, state: PersistedDatabaseItem, ): Promise { let displayName: string | undefined = undefined; @@ -356,12 +345,12 @@ export class DatabaseManager extends DisposableObject { // Avoid persisting the database state after adding since that should happen only after // all databases have been added. - await this.addDatabaseItem(progress, token, item, false); + await this.addDatabaseItem(item, false); return item; } public async loadPersistedState(): Promise { - return withProgress(async (progress, token) => { + return withProgress(async (progress) => { const currentDatabaseUri = this.ctx.workspaceState.get(CURRENT_DB); const databases = this.ctx.workspaceState.get( @@ -388,13 +377,11 @@ export class DatabaseManager extends DisposableObject { }); const databaseItem = await this.createDatabaseItemFromPersistedState( - progress, - token, database, ); try { await this.refreshDatabase(databaseItem); - await this.registerDatabase(progress, token, databaseItem); + await this.registerDatabase(databaseItem); if (currentDatabaseUri === database.uri) { await this.setCurrentDatabaseItem(databaseItem, true); } @@ -489,8 +476,6 @@ export class DatabaseManager extends DisposableObject { } private async addDatabaseItem( - progress: ProgressCallback, - token: vscode.CancellationToken, item: DatabaseItemImpl, updatePersistedState = true, ) { @@ -504,7 +489,7 @@ export class DatabaseManager extends DisposableObject { // Database items reconstituted from persisted state // will not have their contents yet. if (item.contents?.datasetUri) { - await this.registerDatabase(progress, token, item); + await this.registerDatabase(item); } // note that we use undefined as the item in order to reset the entire tree this._onDidChangeDatabaseItem.fire({ @@ -523,11 +508,7 @@ export class DatabaseManager extends DisposableObject { }); } - public async removeDatabaseItem( - progress: ProgressCallback, - token: vscode.CancellationToken, - item: DatabaseItem, - ) { + public async removeDatabaseItem(item: DatabaseItem) { if (this._currentDatabaseItem === item) { this._currentDatabaseItem = undefined; } @@ -549,7 +530,7 @@ export class DatabaseManager extends DisposableObject { } // Remove this database item from the allow-list - await this.deregisterDatabase(progress, token, item); + await this.deregisterDatabase(item); // Delete folder from file system only if it is controlled by the extension if (this.isExtensionControlledLocation(item.databaseUri)) { @@ -572,22 +553,15 @@ export class DatabaseManager extends DisposableObject { }); } - public async removeAllDatabases( - progress: ProgressCallback, - token: vscode.CancellationToken, - ) { + public async removeAllDatabases() { for (const item of this.databaseItems) { - await this.removeDatabaseItem(progress, token, item); + await this.removeDatabaseItem(item); } } - private async deregisterDatabase( - progress: ProgressCallback, - token: vscode.CancellationToken, - dbItem: DatabaseItem, - ) { + private async deregisterDatabase(dbItem: DatabaseItem) { try { - await this.qs.deregisterDatabase(progress, token, dbItem); + await this.qs.deregisterDatabase(dbItem); } catch (e) { const message = getErrorMessage(e); if (message === "Connection is disposed.") { @@ -600,12 +574,8 @@ export class DatabaseManager extends DisposableObject { throw e; } } - private async registerDatabase( - progress: ProgressCallback, - token: vscode.CancellationToken, - dbItem: DatabaseItem, - ) { - await this.qs.registerDatabase(progress, token, dbItem); + private async registerDatabase(dbItem: DatabaseItem) { + await this.qs.registerDatabase(dbItem); } /** diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index 28a94b6c3c8..1dc2b6694ca 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -284,7 +284,7 @@ export class LocalQueries extends DisposableObject { private async createSkeletonQuery(): Promise { await withProgress( - async (progress: ProgressCallback, token: CancellationToken) => { + async (progress: ProgressCallback) => { const credentials = isCanary() ? this.app.credentials : undefined; const contextStoragePath = this.app.workspaceStoragePath || this.app.globalStoragePath; @@ -294,7 +294,6 @@ export class LocalQueries extends DisposableObject { credentials, this.app.logger, this.databaseManager, - token, contextStoragePath, ); await skeletonQueryWizard.execute(); diff --git a/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts b/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts index 0e07595064e..3232d7f1806 100644 --- a/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts +++ b/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts @@ -55,10 +55,9 @@ export class LegacyQueryRunner extends QueryRunner { } async clearCacheInDatabase( dbItem: DatabaseItem, - progress: ProgressCallback, token: CancellationToken, ): Promise { - await clearCacheInDatabase(this.qs, dbItem, progress, token); + await clearCacheInDatabase(this.qs, dbItem, token); } public async compileAndRunQueryAgainstDatabaseCore( @@ -88,11 +87,7 @@ export class LegacyQueryRunner extends QueryRunner { ); } - async deregisterDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise { + async deregisterDatabase(dbItem: DatabaseItem): Promise { if (dbItem.contents) { const databases: Dataset[] = [ { @@ -100,19 +95,10 @@ export class LegacyQueryRunner extends QueryRunner { workingSet: "default", }, ]; - await this.qs.sendRequest( - deregisterDatabases, - { databases }, - token, - progress, - ); + await this.qs.sendRequest(deregisterDatabases, { databases }); } } - async registerDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise { + async registerDatabase(dbItem: DatabaseItem): Promise { if (dbItem.contents) { const databases: Dataset[] = [ { @@ -120,12 +106,7 @@ export class LegacyQueryRunner extends QueryRunner { workingSet: "default", }, ]; - await this.qs.sendRequest( - registerDatabases, - { databases }, - token, - progress, - ); + await this.qs.sendRequest(registerDatabases, { databases }); } } diff --git a/extensions/ql-vscode/src/query-server/legacy/run-queries.ts b/extensions/ql-vscode/src/query-server/legacy/run-queries.ts index 6c82614645c..d488708c259 100644 --- a/extensions/ql-vscode/src/query-server/legacy/run-queries.ts +++ b/extensions/ql-vscode/src/query-server/legacy/run-queries.ts @@ -200,7 +200,6 @@ export class QueryInProgress { export async function clearCacheInDatabase( qs: qsClient.QueryServerClient, dbItem: DatabaseItem, - progress: ProgressCallback, token: CancellationToken, ): Promise { if (dbItem.contents === undefined) { @@ -217,7 +216,7 @@ export async function clearCacheInDatabase( db, }; - return qs.sendRequest(messages.clearCache, params, token, progress); + return qs.sendRequest(messages.clearCache, params, token); } function reportNoUpgradePath( diff --git a/extensions/ql-vscode/src/query-server/new-query-runner.ts b/extensions/ql-vscode/src/query-server/new-query-runner.ts index 9932ce8c435..94d57ca4ea7 100644 --- a/extensions/ql-vscode/src/query-server/new-query-runner.ts +++ b/extensions/ql-vscode/src/query-server/new-query-runner.ts @@ -56,7 +56,6 @@ export class NewQueryRunner extends QueryRunner { async clearCacheInDatabase( dbItem: DatabaseItem, - progress: ProgressCallback, token: CancellationToken, ): Promise { if (dbItem.contents === undefined) { @@ -68,7 +67,7 @@ export class NewQueryRunner extends QueryRunner { dryRun: false, db, }; - await this.qs.sendRequest(clearCache, params, token, progress); + await this.qs.sendRequest(clearCache, params, token); } public async compileAndRunQueryAgainstDatabaseCore( @@ -98,34 +97,16 @@ export class NewQueryRunner extends QueryRunner { ); } - async deregisterDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise { + async deregisterDatabase(dbItem: DatabaseItem): Promise { if (dbItem.contents) { const databases: string[] = [dbItem.databaseUri.fsPath]; - await this.qs.sendRequest( - deregisterDatabases, - { databases }, - token, - progress, - ); + await this.qs.sendRequest(deregisterDatabases, { databases }); } } - async registerDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise { + async registerDatabase(dbItem: DatabaseItem): Promise { if (dbItem.contents) { const databases: string[] = [dbItem.databaseUri.fsPath]; - await this.qs.sendRequest( - registerDatabases, - { databases }, - token, - progress, - ); + await this.qs.sendRequest(registerDatabases, { databases }); } } diff --git a/extensions/ql-vscode/src/query-server/query-runner.ts b/extensions/ql-vscode/src/query-server/query-runner.ts index 03f886ffd29..ab923219953 100644 --- a/extensions/ql-vscode/src/query-server/query-runner.ts +++ b/extensions/ql-vscode/src/query-server/query-runner.ts @@ -61,9 +61,9 @@ export abstract class QueryRunner { token: CancellationToken, ) => Promise, ): void; + abstract clearCacheInDatabase( dbItem: DatabaseItem, - progress: ProgressCallback, token: CancellationToken, ): Promise; @@ -83,17 +83,9 @@ export abstract class QueryRunner { logger: BaseLogger, ): Promise; - abstract deregisterDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise; + abstract deregisterDatabase(dbItem: DatabaseItem): Promise; - abstract registerDatabase( - progress: ProgressCallback, - token: CancellationToken, - dbItem: DatabaseItem, - ): Promise; + abstract registerDatabase(dbItem: DatabaseItem): Promise; abstract upgradeDatabaseExplicit( dbItem: DatabaseItem, diff --git a/extensions/ql-vscode/src/query-testing/test-runner.ts b/extensions/ql-vscode/src/query-testing/test-runner.ts index 4bceef0915a..55f5efff453 100644 --- a/extensions/ql-vscode/src/query-testing/test-runner.ts +++ b/extensions/ql-vscode/src/query-testing/test-runner.ts @@ -48,7 +48,7 @@ export class TestRunner extends DisposableObject { } } - await this.removeDatabasesBeforeTests(databasesUnderTest, token); + await this.removeDatabasesBeforeTests(databasesUnderTest); try { const workspacePaths = getOnDiskWorkspaceFolders(); for await (const event of this.cliServer.runTests(tests, workspacePaths, { @@ -66,24 +66,16 @@ export class TestRunner extends DisposableObject { await this.reopenDatabasesAfterTests( databasesUnderTest, currentDatabaseUri, - token, ); } } private async removeDatabasesBeforeTests( databasesUnderTest: DatabaseItem[], - token: CancellationToken, ): Promise { for (const database of databasesUnderTest) { try { - await this.databaseManager.removeDatabaseItem( - (_) => { - /* no progress reporting */ - }, - token, - database, - ); + await this.databaseManager.removeDatabaseItem(database); } catch (e) { // This method is invoked from Test Explorer UI, and testing indicates that Test // Explorer UI swallows any thrown exception without reporting it to the user. @@ -103,17 +95,12 @@ export class TestRunner extends DisposableObject { private async reopenDatabasesAfterTests( databasesUnderTest: DatabaseItem[], currentDatabaseUri: Uri | undefined, - token: CancellationToken, ): Promise { for (const closedDatabase of databasesUnderTest) { const uri = closedDatabase.databaseUri; if (await isFileAccessible(uri)) { try { const reopenedDatabase = await this.databaseManager.openDatabase( - (_) => { - /* no progress reporting */ - }, - token, uri, false, ); diff --git a/extensions/ql-vscode/src/skeleton-query-wizard.ts b/extensions/ql-vscode/src/skeleton-query-wizard.ts index 8db5da2097d..65ee9de08a3 100644 --- a/extensions/ql-vscode/src/skeleton-query-wizard.ts +++ b/extensions/ql-vscode/src/skeleton-query-wizard.ts @@ -1,5 +1,5 @@ import { join } from "path"; -import { CancellationToken, Uri, workspace, window as Window } from "vscode"; +import { Uri, workspace, window as Window } from "vscode"; import { CodeQLCliServer } from "./codeql-cli/cli"; import { BaseLogger } from "./common"; import { Credentials } from "./common/authentication"; @@ -51,7 +51,6 @@ export class SkeletonQueryWizard { private readonly credentials: Credentials | undefined, private readonly logger: BaseLogger, private readonly databaseManager: DatabaseManager, - private readonly token: CancellationToken, private readonly databaseStoragePath: string | undefined, ) {} @@ -258,7 +257,6 @@ export class SkeletonQueryWizard { this.databaseStoragePath, this.credentials, this.progress, - this.token, this.cliServer, this.language, ); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts index 2799e32f749..9b2205f3d7e 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/databases/database-fetcher.test.ts @@ -1,5 +1,5 @@ import { join } from "path"; -import { CancellationToken, Uri, window } from "vscode"; +import { Uri, window } from "vscode"; import { CodeQLCliServer } from "../../../../src/codeql-cli/cli"; import { DatabaseManager } from "../../../../src/databases/local-databases"; @@ -52,7 +52,6 @@ describe("database-fetcher", () => { databaseManager, storagePath, progressCallback, - {} as CancellationToken, cli, ); expect(dbItem).toBe(databaseManager.currentDatabaseItem); @@ -74,7 +73,6 @@ describe("database-fetcher", () => { databaseManager, storagePath, progressCallback, - {} as CancellationToken, cli, ); expect(dbItem).toBeDefined(); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts index 7c56775bba9..562683dd3f5 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/skeleton-query-wizard.test.ts @@ -11,7 +11,6 @@ import { QlPackGenerator } from "../../../src/qlpack-generator"; import * as workspaceFolders from "../../../src/common/vscode/workspace-folders"; import { createFileSync, ensureDirSync, removeSync } from "fs-extra"; import { join } from "path"; -import { CancellationTokenSource } from "vscode-jsonrpc"; import { testCredentialsWithStub } from "../../factories/authentication"; import { DatabaseItem, @@ -47,7 +46,6 @@ describe("SkeletonQueryWizard", () => { typeof workspace.openTextDocument >; - const token = new CancellationTokenSource().token; const credentials = testCredentialsWithStub(); const chosenLanguage = "ruby"; @@ -117,7 +115,6 @@ describe("SkeletonQueryWizard", () => { credentials, extLogger, mockDatabaseManager, - token, storagePath, ); @@ -252,7 +249,6 @@ describe("SkeletonQueryWizard", () => { credentials, extLogger, mockDatabaseManagerWithItems, - token, storagePath, ); }); diff --git a/extensions/ql-vscode/test/vscode-tests/global.helper.ts b/extensions/ql-vscode/test/vscode-tests/global.helper.ts index 61251f0a1c1..0863b62c4e2 100644 --- a/extensions/ql-vscode/test/vscode-tests/global.helper.ts +++ b/extensions/ql-vscode/test/vscode-tests/global.helper.ts @@ -1,12 +1,7 @@ import { join } from "path"; import { load, dump } from "js-yaml"; import { realpathSync, readFileSync, writeFileSync } from "fs-extra"; -import { - CancellationToken, - CancellationTokenSource, - Uri, - extensions, -} from "vscode"; +import { Uri, extensions } from "vscode"; import { DatabaseItem, DatabaseManager, @@ -14,7 +9,6 @@ import { import { CodeQLCliServer } from "../../src/codeql-cli/cli"; import { removeWorkspaceRefs } from "../../src/variant-analysis/run-remote-query"; import { CodeQLExtensionInterface } from "../../src/extension"; -import { ProgressCallback } from "../../src/common/vscode/progress"; import { importArchiveDatabase } from "../../src/databases/database-fetcher"; import { createMockCommandManager } from "../__mocks__/commandsMock"; @@ -49,7 +43,6 @@ export async function ensureTestDatabase( (_p) => { /**/ }, - new CancellationTokenSource().token, cli, ); @@ -77,10 +70,7 @@ export async function getActivatedExtension(): Promise } export async function cleanDatabases(databaseManager: DatabaseManager) { - await databaseManager.removeAllDatabases( - {} as ProgressCallback, - {} as CancellationToken, - ); + await databaseManager.removeAllDatabases(); } /** diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts index a28197ac2cb..37af6e29c7c 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts @@ -141,11 +141,7 @@ describe("local databases", () => { onDidChangeDatabaseItem.mockClear(); // now remove the item - await databaseManager.removeDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await databaseManager.removeDatabaseItem(mockDbItem); expect((databaseManager as any)._databaseItems).toEqual([]); expect(updateSpy).toBeCalledWith("databaseList", []); expect(onDidChangeDatabaseItem).toBeCalledWith({ @@ -243,11 +239,7 @@ describe("local databases", () => { updateSpy.mockClear(); - await databaseManager.removeDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await databaseManager.removeDatabaseItem(mockDbItem); expect(databaseManager.databaseItems).toEqual([]); expect(updateSpy).toBeCalledWith("databaseList", []); @@ -279,11 +271,7 @@ describe("local databases", () => { (databaseManager as any).ctx.storageUri = Uri.file("hucairz"); extensionContextStoragePath = "hucairz"; - await databaseManager.removeDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await databaseManager.removeDatabaseItem(mockDbItem); expect(databaseManager.databaseItems).toEqual([]); expect(updateSpy).toBeCalledWith("databaseList", []); @@ -309,11 +297,7 @@ describe("local databases", () => { // Should have registered this database expect(registerSpy).toBeCalledWith({}, {}, mockDbItem); - await databaseManager.removeDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await databaseManager.removeDatabaseItem(mockDbItem); // Should have deregistered this database expect(deregisterSpy).toBeCalledWith({}, {}, mockDbItem); @@ -753,31 +737,19 @@ describe("local databases", () => { }); it("should resolve the database contents", async () => { - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(resolveDatabaseContentsSpy).toBeCalledTimes(2); }); it("should set the database as the currently selected one", async () => { - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(setCurrentDatabaseItemSpy).toBeCalledTimes(1); }); it("should add database source archive folder", async () => { - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(addDatabaseSourceArchiveFolderSpy).toBeCalledTimes(1); }); @@ -792,8 +764,6 @@ describe("local databases", () => { const nameOverride = "CodeQL Tutorial Database"; await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, mockDbItem.databaseUri, makeSelected, nameOverride, @@ -808,11 +778,7 @@ describe("local databases", () => { it("should create a skeleton QL pack", async () => { jest.spyOn(Setting.prototype, "getValue").mockReturnValue(true); - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(createSkeletonPacksSpy).toBeCalledTimes(1); }); @@ -823,11 +789,7 @@ describe("local databases", () => { it("should not create a skeleton QL pack", async () => { jest.spyOn(Setting.prototype, "getValue").mockReturnValue(false); - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(createSkeletonPacksSpy).toBeCalledTimes(0); }); }); @@ -836,11 +798,7 @@ describe("local databases", () => { it("should not create a skeleton QL pack", async () => { jest.spyOn(Setting.prototype, "getValue").mockReturnValue(undefined); - await databaseManager.openDatabase( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem.databaseUri, - ); + await databaseManager.openDatabase(mockDbItem.databaseUri); expect(createSkeletonPacksSpy).toBeCalledTimes(0); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts index 2511c1fa436..c460b3d5ccc 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts @@ -242,11 +242,7 @@ describe("run-queries", () => { }, } as any; - await runner.registerDatabase( - mockProgress as any, - mockCancel as any, - dbItem, - ); + await runner.registerDatabase(dbItem); expect(qs.sendRequest).toHaveBeenCalledTimes(1); expect(qs.sendRequest).toHaveBeenCalledWith( @@ -277,11 +273,7 @@ describe("run-queries", () => { }, } as any; - await runner.deregisterDatabase( - mockProgress as any, - mockCancel as any, - dbItem, - ); + await runner.deregisterDatabase(dbItem); expect(qs.sendRequest).toHaveBeenCalledTimes(1); expect(qs.sendRequest).toHaveBeenCalledWith( From 4d8506b3f56221677d689fcfac4c712aaf16042a Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Jun 2023 09:42:59 +0100 Subject: [PATCH 2/3] Add back in manual progress update --- .../data-extensions-editor/data-extensions-editor-view.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts b/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts index aba17154158..f6fa457c91b 100644 --- a/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts +++ b/extensions/ql-vscode/src/data-extensions-editor/data-extensions-editor-view.ts @@ -353,6 +353,11 @@ export class DataExtensionsEditorView extends AbstractWebview< // After the flow model has been generated, we can remove the temporary database // which we used for generating the flow model. + await this.showProgress({ + step: 3900, + maxStep: 4000, + message: "Removing temporary database", + }); await this.databaseManager.removeDatabaseItem(database); await this.clearProgress(); From f99957435ddc2b9c0d0bc87335e0f81107d2e1e1 Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 20 Jun 2023 11:09:13 +0100 Subject: [PATCH 3/3] Fix expected args in tests --- .../cli-integration/legacy-query.test.ts | 9 +--- .../cli-integration/new-query.test.ts | 9 +--- .../minimal-workspace/local-databases.test.ts | 43 ++++------------- .../query-testing/test-runner.test.ts | 8 +--- .../no-workspace/run-queries.test.ts | 46 +++++++------------ 5 files changed, 28 insertions(+), 87 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/legacy-query.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/legacy-query.test.ts index 9a933a43767..c57f31cfdb9 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/legacy-query.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/legacy-query.test.ts @@ -142,14 +142,7 @@ describeWithCodeQL()("using the legacy query server", () => { const parsedResults = new Checkpoint(); it("should register the database if necessary", async () => { - await qs.sendRequest( - messages.registerDatabases, - { databases: [db] }, - token, - (() => { - /**/ - }) as any, - ); + await qs.sendRequest(messages.registerDatabases, { databases: [db] }); }); it(`should be able to compile query ${queryName}`, async () => { diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/new-query.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/new-query.test.ts index c3e578ebbe9..5dc838101c1 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/new-query.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/new-query.test.ts @@ -152,14 +152,7 @@ describeWithCodeQL()("using the new query server", () => { return; } - await qs.sendRequest( - messages.registerDatabases, - { databases: [db] }, - token, - (() => { - /**/ - }) as any, - ); + await qs.sendRequest(messages.registerDatabases, { databases: [db] }); }); it(`should be able to run query ${queryName}`, async () => { diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts index 37af6e29c7c..4601973cbb9 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/local-databases.test.ts @@ -1,7 +1,7 @@ import * as tmp from "tmp"; import * as fs from "fs-extra"; import { join } from "path"; -import { CancellationToken, ExtensionContext, Uri, workspace } from "vscode"; +import { ExtensionContext, Uri, workspace } from "vscode"; import { DatabaseContentsWithDbScheme, @@ -12,7 +12,6 @@ import { FullDatabaseOptions, } from "../../../src/databases/local-databases"; import { Logger } from "../../../src/common"; -import { ProgressCallback } from "../../../src/common/vscode/progress"; import { CodeQLCliServer, DbInfo } from "../../../src/codeql-cli/cli"; import { encodeArchiveBasePath, @@ -119,11 +118,7 @@ describe("local databases", () => { const mockDbItem = createMockDB(dir); const onDidChangeDatabaseItem = jest.fn(); databaseManager.onDidChangeDatabaseItem(onDidChangeDatabaseItem); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); expect((databaseManager as any)._databaseItems).toEqual([mockDbItem]); expect(updateSpy).toBeCalledWith("databaseList", [ @@ -155,11 +150,7 @@ describe("local databases", () => { const mockDbItem = createMockDB(dir); const onDidChangeDatabaseItem = jest.fn(); databaseManager.onDidChangeDatabaseItem(onDidChangeDatabaseItem); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); await databaseManager.renameDatabaseItem(mockDbItem, "new name"); @@ -184,11 +175,7 @@ describe("local databases", () => { databaseManager.onDidChangeDatabaseItem(onDidChangeDatabaseItem); const mockDbItem = createMockDB(dir); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); expect(databaseManager.databaseItems).toEqual([mockDbItem]); expect(updateSpy).toBeCalledWith("databaseList", [ @@ -231,11 +218,7 @@ describe("local databases", () => { .spyOn(mockDbItem, "belongsToSourceArchiveExplorerUri") .mockReturnValue(true); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); updateSpy.mockClear(); @@ -260,11 +243,7 @@ describe("local databases", () => { jest .spyOn(mockDbItem, "belongsToSourceArchiveExplorerUri") .mockReturnValue(true); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); updateSpy.mockClear(); // pretend that the database location is not controlled by the extension @@ -289,18 +268,14 @@ describe("local databases", () => { // registration messages. const mockDbItem = createMockDB(dir); - await (databaseManager as any).addDatabaseItem( - {} as ProgressCallback, - {} as CancellationToken, - mockDbItem, - ); + await (databaseManager as any).addDatabaseItem(mockDbItem); // Should have registered this database - expect(registerSpy).toBeCalledWith({}, {}, mockDbItem); + expect(registerSpy).toBeCalledWith(mockDbItem); await databaseManager.removeDatabaseItem(mockDbItem); // Should have deregistered this database - expect(deregisterSpy).toBeCalledWith({}, {}, mockDbItem); + expect(deregisterSpy).toBeCalledWith(mockDbItem); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts index f4d9a3902ea..ef18b2b011d 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts @@ -155,16 +155,10 @@ describe("test-runner", () => { ).toBeGreaterThan(openDatabaseSpy.mock.invocationCallOrder[0]); expect(removeDatabaseItemSpy).toBeCalledTimes(1); - expect(removeDatabaseItemSpy).toBeCalledWith( - expect.anything(), - expect.anything(), - preTestDatabaseItem, - ); + expect(removeDatabaseItemSpy).toBeCalledWith(preTestDatabaseItem); expect(openDatabaseSpy).toBeCalledTimes(1); expect(openDatabaseSpy).toBeCalledWith( - expect.anything(), - expect.anything(), preTestDatabaseItem.databaseUri, false, ); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts index c460b3d5ccc..c0691d3565e 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/run-queries.test.ts @@ -232,8 +232,6 @@ describe("run-queries", () => { it("should register", async () => { const qs = createMockQueryServerClient(); const runner = new LegacyQueryRunner(qs); - const mockProgress = "progress-monitor"; - const mockCancel = "cancel-token"; const datasetUri = Uri.file("dataset-uri"); const dbItem: DatabaseItem = { @@ -245,26 +243,19 @@ describe("run-queries", () => { await runner.registerDatabase(dbItem); expect(qs.sendRequest).toHaveBeenCalledTimes(1); - expect(qs.sendRequest).toHaveBeenCalledWith( - registerDatabases, - { - databases: [ - { - dbDir: datasetUri.fsPath, - workingSet: "default", - }, - ], - }, - mockCancel, - mockProgress, - ); + expect(qs.sendRequest).toHaveBeenCalledWith(registerDatabases, { + databases: [ + { + dbDir: datasetUri.fsPath, + workingSet: "default", + }, + ], + }); }); it("should deregister", async () => { const qs = createMockQueryServerClient(); const runner = new LegacyQueryRunner(qs); - const mockProgress = "progress-monitor"; - const mockCancel = "cancel-token"; const datasetUri = Uri.file("dataset-uri"); const dbItem: DatabaseItem = { @@ -276,19 +267,14 @@ describe("run-queries", () => { await runner.deregisterDatabase(dbItem); expect(qs.sendRequest).toHaveBeenCalledTimes(1); - expect(qs.sendRequest).toHaveBeenCalledWith( - deregisterDatabases, - { - databases: [ - { - dbDir: datasetUri.fsPath, - workingSet: "default", - }, - ], - }, - mockCancel, - mockProgress, - ); + expect(qs.sendRequest).toHaveBeenCalledWith(deregisterDatabases, { + databases: [ + { + dbDir: datasetUri.fsPath, + workingSet: "default", + }, + ], + }); }); });