From 937e5042f5b37d672846fda86b1540abb74efca2 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 21 Feb 2023 15:16:50 +0000 Subject: [PATCH 1/3] Fix `Unhandled arguments` bug When we added an extra optional param `isTutorialDatabase` to `openDatabase` [1] , the tests all passed but there is at least one call [2] where this triggers an `Unhandled arguments` error. This is because the method call doesn't know which optional param we're passing. Let's revert back to the original method signature where we had just one optional parameter (the database name) and set it to a special "CodeQL Tutorial Database" value. We can then use this to understand that the extension is running in the codespace and we want to skip creating a skeleton pack for it since it's the tutorial database. [1]: https://github.com/github/vscode-codeql/blob/5e51bb57f5f76b57768fb6a3543f153f5752c23c/extensions/ql-vscode/src/local-databases.ts#L610 [2]: https://github.com/github/vscode-codeql/blob/5e51bb57f5f76b57768fb6a3543f153f5752c23c/extensions/ql-vscode/src/databaseFetcher.ts#L257-L262 --- extensions/ql-vscode/src/local-databases-ui.ts | 2 -- extensions/ql-vscode/src/local-databases.ts | 3 +-- .../vscode-tests/minimal-workspace/local-databases.test.ts | 3 --- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/local-databases-ui.ts b/extensions/ql-vscode/src/local-databases-ui.ts index f80b0f8394a..f996244d35c 100644 --- a/extensions/ql-vscode/src/local-databases-ui.ts +++ b/extensions/ql-vscode/src/local-databases-ui.ts @@ -379,14 +379,12 @@ export class DatabaseUI extends DisposableObject { ); let databaseItem = this.databaseManager.findDatabaseItem(uri); - const isTutorialDatabase = true; if (databaseItem === undefined) { databaseItem = await this.databaseManager.openDatabase( progress, token, uri, "CodeQL Tutorial Database", - isTutorialDatabase, ); } await this.databaseManager.setCurrentDatabaseItem(databaseItem); diff --git a/extensions/ql-vscode/src/local-databases.ts b/extensions/ql-vscode/src/local-databases.ts index 8ba756df77f..36ed8f88fef 100644 --- a/extensions/ql-vscode/src/local-databases.ts +++ b/extensions/ql-vscode/src/local-databases.ts @@ -607,7 +607,6 @@ export class DatabaseManager extends DisposableObject { token: vscode.CancellationToken, uri: vscode.Uri, displayName?: string, - isTutorialDatabase?: boolean, ): Promise { const contents = await DatabaseResolver.resolveDatabaseContents(uri); // Ignore the source archive for QLTest databases by default. @@ -631,7 +630,7 @@ export class DatabaseManager extends DisposableObject { await this.addDatabaseItem(progress, token, databaseItem); await this.addDatabaseSourceArchiveFolder(databaseItem); - if (isCodespacesTemplate() && !isTutorialDatabase) { + if (isCodespacesTemplate() && displayName !== "CodeQL Tutorial Database") { await this.createSkeletonPacks(databaseItem); } 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 df020a5b910..9b09abc8118 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 @@ -715,14 +715,11 @@ describe("local databases", () => { it("should not offer to create a skeleton QL pack", async () => { jest.spyOn(Setting.prototype, "getValue").mockReturnValue(true); - const isTutorialDatabase = true; - await databaseManager.openDatabase( {} as ProgressCallback, {} as CancellationToken, mockDbItem.databaseUri, "CodeQL Tutorial Database", - isTutorialDatabase, ); expect(createSkeletonPacksSpy).toBeCalledTimes(0); From 2d040c24329583d0640cfffdfdbd565d930feaa5 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 21 Feb 2023 17:04:27 +0000 Subject: [PATCH 2/3] Add comment and test to enforce the name of the tutorial database I'm extracting the call to `openDatabase` where we set a custom display name into a separate method: `openTutorialDatabase`. I've added comments to it to specify the display name is significant. I've also added a test that will fail if you ever change the name of the tutorial database. --- .../ql-vscode/src/local-databases-ui.ts | 23 ++++++--- .../no-workspace/local-databases-ui.test.ts | 49 ++++++++++++++++++- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/local-databases-ui.ts b/extensions/ql-vscode/src/local-databases-ui.ts index f996244d35c..c68bae4e77f 100644 --- a/extensions/ql-vscode/src/local-databases-ui.ts +++ b/extensions/ql-vscode/src/local-databases-ui.ts @@ -380,12 +380,7 @@ export class DatabaseUI extends DisposableObject { let databaseItem = this.databaseManager.findDatabaseItem(uri); if (databaseItem === undefined) { - databaseItem = await this.databaseManager.openDatabase( - progress, - token, - uri, - "CodeQL Tutorial Database", - ); + databaseItem = await this.openTutorialDatabase(progress, token, uri); } await this.databaseManager.setCurrentDatabaseItem(databaseItem); } @@ -399,6 +394,22 @@ export class DatabaseUI extends DisposableObject { } }; + // Opens the tutorial database for the CodeQL Tour. + // We set the database name as "CodeQL Tutorial Database" in purpose. This means + // we won't attempt to create a skeleton QL pack for this database. + openTutorialDatabase = async ( + progress: ProgressCallback, + token: CancellationToken, + uri: Uri, + ): Promise => { + return await this.databaseManager.openDatabase( + progress, + token, + uri, + "CodeQL Tutorial Database", + ); + }; + handleRemoveOrphanedDatabases = async (): Promise => { void extLogger.log("Removing orphaned databases from workspace storage."); let dbDirs = undefined; diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/local-databases-ui.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/local-databases-ui.test.ts index 61b56592b0b..6e2add5258f 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/local-databases-ui.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/local-databases-ui.test.ts @@ -7,12 +7,14 @@ import { createFileSync, pathExistsSync, } from "fs-extra"; -import { Uri } from "vscode"; +import { CancellationToken, Uri } from "vscode"; import { DatabaseUI } from "../../../src/local-databases-ui"; import { testDisposeHandler } from "../test-dispose-handler"; import { createMockApp } from "../../__mocks__/appMock"; import { QueryLanguage } from "../../../src/common/query-language"; +import { App } from "../../../src/common/app"; +import { ProgressCallback } from "../../../src/commandRunner"; describe("local-databases-ui", () => { describe("fixDbUri", () => { @@ -116,6 +118,51 @@ describe("local-databases-ui", () => { databaseUI.dispose(testDisposeHandler); }); + describe("openTutorialDatabase", () => { + let app: App; + let databaseUI: DatabaseUI; + let openDatabaseSpy: jest.SpyInstance; + let databaseManager: any; + + beforeEach(async () => { + app = createMockApp({}); + openDatabaseSpy = jest.fn(); + + databaseManager = { + databaseItems: [], + openDatabase: openDatabaseSpy, + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + }; + databaseUI = new DatabaseUI( + app, + databaseManager as any, + {} as any, + "", + "", + ); + }); + + it("should call openDatase with 'CodeQL Tutorial Database' as displayName", async () => { + await databaseUI.openTutorialDatabase( + {} as ProgressCallback, + {} as CancellationToken, + Uri.parse(""), + ); + + expect(openDatabaseSpy).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.anything(), + "CodeQL Tutorial Database", + ); + }); + }); + function createDatabase( storageDir: string, dbName: string, From 5269e0b45075045f997c322cc7932454ac2676c5 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Tue, 21 Feb 2023 17:42:37 +0000 Subject: [PATCH 3/3] Don't use unhandle rejection listener in codespace This was implemented here: [1]. It is throwing up a lot of `Unhandled error: Invalid arguments` errors without any way to see a stack trace for them. Since we're not able to debug this from the codespace, I'm choosing to turn it off in the codespace template as it's preventing us from running the Code Tour successfully. --- extensions/ql-vscode/src/extension.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index c9d5d700996..c62a27783b3 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -37,6 +37,7 @@ import { CliConfigListener, DistributionConfigListener, isCanary, + isCodespacesTemplate, joinOrderWarningThreshold, MAX_QUERIES, QueryHistoryConfigListener, @@ -234,7 +235,10 @@ export async function activate( const distributionConfigListener = new DistributionConfigListener(); await initializeLogging(ctx); await initializeTelemetry(extension, ctx); - addUnhandledRejectionListener(); + if (!isCodespacesTemplate()) { + addUnhandledRejectionListener(); + } + install(); const codelensProvider = new QuickEvalCodeLensProvider();