From bfc102a3eee12d4ef6f9752eec92af71dda55387 Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Tue, 16 Jan 2024 13:48:53 +0000 Subject: [PATCH 1/3] Add test for "promptForDatabase" --- .../src/databases/local-databases-ui.ts | 8 +- .../databases/local-databases-ui.test.ts | 135 ++++++++++++------ 2 files changed, 93 insertions(+), 50 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 674c1ecd3d1..69594ff1606 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -235,7 +235,7 @@ async function chooseDatabaseDir(byFolder: boolean): Promise { return getFirst(chosen); } -interface DatabaseSelectionQuickPickItem extends QuickPickItem { +export interface DatabaseSelectionQuickPickItem extends QuickPickItem { databaseKind: "new" | "existing"; } @@ -827,7 +827,7 @@ export class DatabaseUI extends DisposableObject { return this.databaseManager.currentDatabaseItem; } - private async promptForDatabase(): Promise { + public async promptForDatabase(): Promise { const quickPickItems: DatabaseSelectionQuickPickItem[] = [ { label: "$(database) Existing database", @@ -862,7 +862,7 @@ export class DatabaseUI extends DisposableObject { } } - private async selectExistingDatabase() { + public async selectExistingDatabase() { const dbItems: DatabaseQuickPickItem[] = this.databaseManager.databaseItems.map((dbItem) => ({ label: dbItem.name, @@ -884,7 +884,7 @@ export class DatabaseUI extends DisposableObject { ); } - private async importNewDatabase() { + public async importNewDatabase() { const importOptions: DatabaseImportQuickPickItems[] = [ { label: "$(github) GitHub", diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts index 05c324651ee..934876a89a5 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts @@ -7,14 +7,57 @@ import { createFileSync, pathExistsSync, } from "fs-extra"; -import { Uri } from "vscode"; +import { Uri, window } from "vscode"; +import type { DatabaseSelectionQuickPickItem } from "../../../../src/databases/local-databases-ui"; import { DatabaseUI } from "../../../../src/databases/local-databases-ui"; import { testDisposeHandler } from "../../test-dispose-handler"; import { createMockApp } from "../../../__mocks__/appMock"; import { QueryLanguage } from "../../../../src/common/query-language"; +import { mockedQuickPickItem, mockedObject } from "../../utils/mocking.helpers"; describe("local-databases-ui", () => { + const storageDir = dirSync({ unsafeCleanup: true }).name; + const db1 = createDatabase(storageDir, "db1-imported", QueryLanguage.Cpp); + const db2 = createDatabase(storageDir, "db2-notimported", QueryLanguage.Cpp); + const db3 = createDatabase(storageDir, "db3-invalidlanguage", "hucairz"); + + // these two should be deleted + const db4 = createDatabase( + storageDir, + "db2-notimported-with-db-info", + QueryLanguage.Cpp, + ".dbinfo", + ); + const db5 = createDatabase( + storageDir, + "db2-notimported-with-codeql-database.yml", + QueryLanguage.Cpp, + "codeql-database.yml", + ); + + const app = createMockApp({}); + const databaseUI = new DatabaseUI( + app, + { + databaseItems: [{ databaseUri: Uri.file(db1) }], + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + setCurrentDatabaseItem: () => {}, + } as any, + { + onLanguageContextChanged: () => { + /**/ + }, + } as any, + {} as any, + storageDir, + storageDir, + ); describe("fixDbUri", () => { const fixDbUri = (DatabaseUI.prototype as any).fixDbUri; it("should choose current directory normally", async () => { @@ -64,51 +107,6 @@ describe("local-databases-ui", () => { }); it("should delete orphaned databases", async () => { - const storageDir = dirSync({ unsafeCleanup: true }).name; - const db1 = createDatabase(storageDir, "db1-imported", QueryLanguage.Cpp); - const db2 = createDatabase( - storageDir, - "db2-notimported", - QueryLanguage.Cpp, - ); - const db3 = createDatabase(storageDir, "db3-invalidlanguage", "hucairz"); - - // these two should be deleted - const db4 = createDatabase( - storageDir, - "db2-notimported-with-db-info", - QueryLanguage.Cpp, - ".dbinfo", - ); - const db5 = createDatabase( - storageDir, - "db2-notimported-with-codeql-database.yml", - QueryLanguage.Cpp, - "codeql-database.yml", - ); - - const app = createMockApp({}); - const databaseUI = new DatabaseUI( - app, - { - databaseItems: [{ databaseUri: Uri.file(db1) }], - onDidChangeDatabaseItem: () => { - /**/ - }, - onDidChangeCurrentDatabaseItem: () => { - /**/ - }, - } as any, - { - onLanguageContextChanged: () => { - /**/ - }, - } as any, - {} as any, - storageDir, - storageDir, - ); - await databaseUI.handleRemoveOrphanedDatabases(); expect(pathExistsSync(db1)).toBe(true); @@ -121,6 +119,51 @@ describe("local-databases-ui", () => { databaseUI.dispose(testDisposeHandler); }); + describe("promptForDatabase", () => { + it("should prompt for a new or existing database", async () => { + const showQuickPickSpy = jest + .spyOn(window, "showQuickPick") + .mockResolvedValue( + mockedQuickPickItem( + mockedObject({ + databaseKind: "existing", + }), + ), + ); + + const selectExistingDatabaseSpy = jest + .spyOn(databaseUI, "selectExistingDatabase") + .mockResolvedValue(undefined); + + const importNewDatabaseSpy = jest + .spyOn(databaseUI, "importNewDatabase") + .mockResolvedValue(undefined); + + await databaseUI.promptForDatabase(); + expect(showQuickPickSpy).toHaveBeenCalledWith( + [ + { + label: "$(database) Existing database", + detail: "Select an existing database from your workspace", + alwaysShow: true, + databaseKind: "existing", + }, + { + label: "$(arrow-down) New database", + detail: + "Import a new database from the cloud or your local machine", + alwaysShow: true, + databaseKind: "new", + }, + ], + { ignoreFocusOut: true, placeHolder: "Select an option" }, + ); + + expect(selectExistingDatabaseSpy).toHaveBeenCalled(); + expect(importNewDatabaseSpy).not.toHaveBeenCalled(); + }); + }); + function createDatabase( storageDir: string, dbName: string, From a888e72d331f25ed612cad1d17218a03980dccca Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:40:57 +0000 Subject: [PATCH 2/3] Test public `getDatabaseItem` method --- .../src/databases/local-databases-ui.ts | 6 +- .../databases/local-databases-ui.test.ts | 110 +++++++++++++----- 2 files changed, 86 insertions(+), 30 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index 69594ff1606..bb6807abf4e 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -827,7 +827,7 @@ export class DatabaseUI extends DisposableObject { return this.databaseManager.currentDatabaseItem; } - public async promptForDatabase(): Promise { + private async promptForDatabase(): Promise { const quickPickItems: DatabaseSelectionQuickPickItem[] = [ { label: "$(database) Existing database", @@ -862,7 +862,7 @@ export class DatabaseUI extends DisposableObject { } } - public async selectExistingDatabase() { + private async selectExistingDatabase() { const dbItems: DatabaseQuickPickItem[] = this.databaseManager.databaseItems.map((dbItem) => ({ label: dbItem.name, @@ -884,7 +884,7 @@ export class DatabaseUI extends DisposableObject { ); } - public async importNewDatabase() { + private async importNewDatabase() { const importOptions: DatabaseImportQuickPickItems[] = [ { label: "$(github) GitHub", diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts index 934876a89a5..b8a42f3558f 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts @@ -7,7 +7,7 @@ import { createFileSync, pathExistsSync, } from "fs-extra"; -import { Uri, window } from "vscode"; +import { CancellationTokenSource, Uri, window } from "vscode"; import type { DatabaseSelectionQuickPickItem } from "../../../../src/databases/local-databases-ui"; import { DatabaseUI } from "../../../../src/databases/local-databases-ui"; @@ -37,27 +37,7 @@ describe("local-databases-ui", () => { ); const app = createMockApp({}); - const databaseUI = new DatabaseUI( - app, - { - databaseItems: [{ databaseUri: Uri.file(db1) }], - onDidChangeDatabaseItem: () => { - /**/ - }, - onDidChangeCurrentDatabaseItem: () => { - /**/ - }, - setCurrentDatabaseItem: () => {}, - } as any, - { - onLanguageContextChanged: () => { - /**/ - }, - } as any, - {} as any, - storageDir, - storageDir, - ); + describe("fixDbUri", () => { const fixDbUri = (DatabaseUI.prototype as any).fixDbUri; it("should choose current directory normally", async () => { @@ -107,6 +87,27 @@ describe("local-databases-ui", () => { }); it("should delete orphaned databases", async () => { + const databaseUI = new DatabaseUI( + app, + { + databaseItems: [{ databaseUri: Uri.file(db1) }], + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + setCurrentDatabaseItem: () => {}, + } as any, + { + onLanguageContextChanged: () => { + /**/ + }, + } as any, + {} as any, + storageDir, + storageDir, + ); await databaseUI.handleRemoveOrphanedDatabases(); expect(pathExistsSync(db1)).toBe(true); @@ -119,8 +120,62 @@ describe("local-databases-ui", () => { databaseUI.dispose(testDisposeHandler); }); - describe("promptForDatabase", () => { - it("should prompt for a new or existing database", async () => { + describe("getDatabaseItem", () => { + const progress = jest.fn(); + const token = new CancellationTokenSource().token; + it("should return current database if there is one", async () => { + const databaseUI = new DatabaseUI( + app, + { + databaseItems: [{ databaseUri: Uri.file(db1) }], + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + setCurrentDatabaseItem: () => {}, + currentDatabaseItem: { databaseUri: Uri.file(db1) }, + } as any, + { + onLanguageContextChanged: () => { + /**/ + }, + } as any, + {} as any, + storageDir, + storageDir, + ); + + const databaseItem = await databaseUI.getDatabaseItem(progress, token); + + expect(databaseItem).toBeDefined(); + }); + + it("should prompt for a database if there is no current one", async () => { + const databaseUI = new DatabaseUI( + app, + { + databaseItems: [{ databaseUri: Uri.file(db1) }], + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + setCurrentDatabaseItem: () => {}, + currentDatabaseItem: undefined, + } as any, + { + onLanguageContextChanged: () => { + /**/ + }, + } as any, + {} as any, + storageDir, + storageDir, + ); + const showQuickPickSpy = jest .spyOn(window, "showQuickPick") .mockResolvedValue( @@ -132,14 +187,15 @@ describe("local-databases-ui", () => { ); const selectExistingDatabaseSpy = jest - .spyOn(databaseUI, "selectExistingDatabase") + .spyOn(databaseUI as any, "selectExistingDatabase") .mockResolvedValue(undefined); const importNewDatabaseSpy = jest - .spyOn(databaseUI, "importNewDatabase") + .spyOn(databaseUI as any, "importNewDatabase") .mockResolvedValue(undefined); - await databaseUI.promptForDatabase(); + await databaseUI.getDatabaseItem(progress, token); + expect(showQuickPickSpy).toHaveBeenCalledWith( [ { From 212f57763d3ccf4800234806cfbc4f80e3b5a1d6 Mon Sep 17 00:00:00 2001 From: shati-patel <42641846+shati-patel@users.noreply.github.com> Date: Thu, 18 Jan 2024 15:09:17 +0000 Subject: [PATCH 3/3] More comprehensive tests of `getDatabaseItem` --- .../src/databases/local-databases-ui.ts | 2 +- .../databases/local-databases-ui.test.ts | 131 +++++++++++------- 2 files changed, 81 insertions(+), 52 deletions(-) diff --git a/extensions/ql-vscode/src/databases/local-databases-ui.ts b/extensions/ql-vscode/src/databases/local-databases-ui.ts index bb6807abf4e..f1b802ed8d8 100644 --- a/extensions/ql-vscode/src/databases/local-databases-ui.ts +++ b/extensions/ql-vscode/src/databases/local-databases-ui.ts @@ -243,7 +243,7 @@ export interface DatabaseQuickPickItem extends QuickPickItem { databaseItem: DatabaseItem; } -interface DatabaseImportQuickPickItems extends QuickPickItem { +export interface DatabaseImportQuickPickItems extends QuickPickItem { importType: "URL" | "github" | "archive" | "folder"; } diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts index b8a42f3558f..1d602078763 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/databases/local-databases-ui.test.ts @@ -8,7 +8,12 @@ import { pathExistsSync, } from "fs-extra"; import { CancellationTokenSource, Uri, window } from "vscode"; -import type { DatabaseSelectionQuickPickItem } from "../../../../src/databases/local-databases-ui"; + +import type { + DatabaseImportQuickPickItems, + DatabaseQuickPickItem, + DatabaseSelectionQuickPickItem, +} from "../../../../src/databases/local-databases-ui"; import { DatabaseUI } from "../../../../src/databases/local-databases-ui"; import { testDisposeHandler } from "../../test-dispose-handler"; @@ -123,7 +128,7 @@ describe("local-databases-ui", () => { describe("getDatabaseItem", () => { const progress = jest.fn(); const token = new CancellationTokenSource().token; - it("should return current database if there is one", async () => { + describe("when there is a current database", () => { const databaseUI = new DatabaseUI( app, { @@ -147,25 +152,32 @@ describe("local-databases-ui", () => { storageDir, ); - const databaseItem = await databaseUI.getDatabaseItem(progress, token); + it("should return current database", async () => { + const databaseItem = await databaseUI.getDatabaseItem(progress, token); - expect(databaseItem).toBeDefined(); + expect(databaseItem).toEqual({ databaseUri: Uri.file(db1) }); + }); }); - it("should prompt for a database if there is no current one", async () => { + describe("when there is no current database", () => { + const databaseManager = { + databaseItems: [ + { databaseUri: Uri.file(db1) }, + { databaseUri: Uri.file(db2) }, + ], + onDidChangeDatabaseItem: () => { + /**/ + }, + onDidChangeCurrentDatabaseItem: () => { + /**/ + }, + setCurrentDatabaseItem: () => {}, + currentDatabaseItem: undefined, + } as any; + const databaseUI = new DatabaseUI( app, - { - databaseItems: [{ databaseUri: Uri.file(db1) }], - onDidChangeDatabaseItem: () => { - /**/ - }, - onDidChangeCurrentDatabaseItem: () => { - /**/ - }, - setCurrentDatabaseItem: () => {}, - currentDatabaseItem: undefined, - } as any, + databaseManager, { onLanguageContextChanged: () => { /**/ @@ -176,47 +188,64 @@ describe("local-databases-ui", () => { storageDir, ); - const showQuickPickSpy = jest - .spyOn(window, "showQuickPick") - .mockResolvedValue( - mockedQuickPickItem( - mockedObject({ - databaseKind: "existing", - }), - ), + it("should prompt for a database and select existing one", async () => { + const showQuickPickSpy = jest + .spyOn(window, "showQuickPick") + .mockResolvedValueOnce( + mockedQuickPickItem( + mockedObject({ + databaseKind: "existing", + }), + ), + ) + .mockResolvedValueOnce( + mockedQuickPickItem( + mockedObject({ + databaseItem: { databaseUri: Uri.file(db2) }, + }), + ), + ); + + const setCurrentDatabaseItemSpy = jest.spyOn( + databaseManager, + "setCurrentDatabaseItem", ); - const selectExistingDatabaseSpy = jest - .spyOn(databaseUI as any, "selectExistingDatabase") - .mockResolvedValue(undefined); + await databaseUI.getDatabaseItem(progress, token); - const importNewDatabaseSpy = jest - .spyOn(databaseUI as any, "importNewDatabase") - .mockResolvedValue(undefined); + expect(showQuickPickSpy).toHaveBeenCalledTimes(2); + expect(setCurrentDatabaseItemSpy).toHaveBeenCalledWith({ + databaseUri: Uri.file(db2), + }); + }); - await databaseUI.getDatabaseItem(progress, token); + it("should prompt for a database and import a new one", async () => { + const showQuickPickSpy = jest + .spyOn(window, "showQuickPick") + .mockResolvedValueOnce( + mockedQuickPickItem( + mockedObject({ + databaseKind: "new", + }), + ), + ) + .mockResolvedValueOnce( + mockedQuickPickItem( + mockedObject({ + importType: "github", + }), + ), + ); - expect(showQuickPickSpy).toHaveBeenCalledWith( - [ - { - label: "$(database) Existing database", - detail: "Select an existing database from your workspace", - alwaysShow: true, - databaseKind: "existing", - }, - { - label: "$(arrow-down) New database", - detail: - "Import a new database from the cloud or your local machine", - alwaysShow: true, - databaseKind: "new", - }, - ], - { ignoreFocusOut: true, placeHolder: "Select an option" }, - ); + const handleChooseDatabaseGithubSpy = jest + .spyOn(databaseUI as any, "handleChooseDatabaseGithub") + .mockResolvedValue(undefined); + + await databaseUI.getDatabaseItem(progress, token); - expect(selectExistingDatabaseSpy).toHaveBeenCalled(); - expect(importNewDatabaseSpy).not.toHaveBeenCalled(); + expect(showQuickPickSpy).toHaveBeenCalledTimes(2); + expect(handleChooseDatabaseGithubSpy).toHaveBeenCalledTimes(1); + }); }); });