From f8d0f689a6b72db6c9271b9440c182ffcd557014 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Sun, 5 Feb 2023 23:50:56 +0000 Subject: [PATCH 1/2] Move functions into class so they can be mocked We'd like to add test coverage for the openDatabase function (which is public). At the moment, this relies on `resolveDatabaseContents` which is just a standalone function. This means we're unable to mock it using Jest. So let's move it into its own class. This method in turn depends on a `resolveDatabase` function, which we've also moved into the new class. The only usages I could find for there functions were from within the `databases.ts` file. --- extensions/ql-vscode/src/databases.ts | 110 +++++++++++++------------- 1 file changed, 57 insertions(+), 53 deletions(-) diff --git a/extensions/ql-vscode/src/databases.ts b/extensions/ql-vscode/src/databases.ts index a8cfba97afb..e6651b8e094 100644 --- a/extensions/ql-vscode/src/databases.ts +++ b/extensions/ql-vscode/src/databases.ts @@ -154,67 +154,69 @@ export async function findSourceArchive( return undefined; } -async function resolveDatabase( - databasePath: string, -): Promise { - const name = basename(databasePath); - - // Look for dataset and source archive. - const datasetUri = await findDataset(databasePath); - const sourceArchiveUri = await findSourceArchive(databasePath); - - return { - kind: DatabaseKind.Database, - name, - datasetUri, - sourceArchiveUri, - }; -} - /** Gets the relative paths of all `.dbscheme` files in the given directory. */ async function getDbSchemeFiles(dbDirectory: string): Promise { return await glob("*.dbscheme", { cwd: dbDirectory }); } -async function resolveDatabaseContents( - uri: vscode.Uri, -): Promise { - if (uri.scheme !== "file") { - throw new Error( - `Database URI scheme '${uri.scheme}' not supported; only 'file' URIs are supported.`, - ); - } - const databasePath = uri.fsPath; - if (!(await pathExists(databasePath))) { - throw new InvalidDatabaseError( - `Database '${databasePath}' does not exist.`, - ); - } +export class DatabaseResolver { + public static async resolveDatabaseContents( + uri: vscode.Uri, + ): Promise { + if (uri.scheme !== "file") { + throw new Error( + `Database URI scheme '${uri.scheme}' not supported; only 'file' URIs are supported.`, + ); + } + const databasePath = uri.fsPath; + if (!(await pathExists(databasePath))) { + throw new InvalidDatabaseError( + `Database '${databasePath}' does not exist.`, + ); + } - const contents = await resolveDatabase(databasePath); + const contents = await this.resolveDatabase(databasePath); - if (contents === undefined) { - throw new InvalidDatabaseError( - `'${databasePath}' is not a valid database.`, - ); + if (contents === undefined) { + throw new InvalidDatabaseError( + `'${databasePath}' is not a valid database.`, + ); + } + + // Look for a single dbscheme file within the database. + // This should be found in the dataset directory, regardless of the form of database. + const dbPath = contents.datasetUri.fsPath; + const dbSchemeFiles = await getDbSchemeFiles(dbPath); + if (dbSchemeFiles.length === 0) { + throw new InvalidDatabaseError( + `Database '${databasePath}' does not contain a CodeQL dbscheme under '${dbPath}'.`, + ); + } else if (dbSchemeFiles.length > 1) { + throw new InvalidDatabaseError( + `Database '${databasePath}' contains multiple CodeQL dbschemes under '${dbPath}'.`, + ); + } else { + contents.dbSchemeUri = vscode.Uri.file(resolve(dbPath, dbSchemeFiles[0])); + } + return contents; } - // Look for a single dbscheme file within the database. - // This should be found in the dataset directory, regardless of the form of database. - const dbPath = contents.datasetUri.fsPath; - const dbSchemeFiles = await getDbSchemeFiles(dbPath); - if (dbSchemeFiles.length === 0) { - throw new InvalidDatabaseError( - `Database '${databasePath}' does not contain a CodeQL dbscheme under '${dbPath}'.`, - ); - } else if (dbSchemeFiles.length > 1) { - throw new InvalidDatabaseError( - `Database '${databasePath}' contains multiple CodeQL dbschemes under '${dbPath}'.`, - ); - } else { - contents.dbSchemeUri = vscode.Uri.file(resolve(dbPath, dbSchemeFiles[0])); + public static async resolveDatabase( + databasePath: string, + ): Promise { + const name = basename(databasePath); + + // Look for dataset and source archive. + const datasetUri = await findDataset(databasePath); + const sourceArchiveUri = await findSourceArchive(databasePath); + + return { + kind: DatabaseKind.Database, + name, + datasetUri, + sourceArchiveUri, + }; } - return contents; } /** An item in the list of available databases */ @@ -370,7 +372,9 @@ export class DatabaseItemImpl implements DatabaseItem { public async refresh(): Promise { try { try { - this._contents = await resolveDatabaseContents(this.databaseUri); + this._contents = await DatabaseResolver.resolveDatabaseContents( + this.databaseUri, + ); this._error = undefined; } catch (e) { this._contents = undefined; @@ -602,7 +606,7 @@ export class DatabaseManager extends DisposableObject { uri: vscode.Uri, displayName?: string, ): Promise { - const contents = await resolveDatabaseContents(uri); + const contents = await DatabaseResolver.resolveDatabaseContents(uri); // Ignore the source archive for QLTest databases by default. const isQLTestDatabase = extname(uri.fsPath) === ".testproj"; const fullOptions: FullDatabaseOptions = { From 72f3847a3e3170cd208aed1158dcda6638574d99 Mon Sep 17 00:00:00 2001 From: Elena Tanasoiu Date: Sun, 5 Feb 2023 23:53:33 +0000 Subject: [PATCH 2/2] Add tests for openDatabase function --- .../minimal-workspace/databases.test.ts | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases.test.ts b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases.test.ts index 37d804b8d68..068be73850e 100644 --- a/extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/minimal-workspace/databases.test.ts @@ -10,6 +10,7 @@ import { DatabaseContents, FullDatabaseOptions, findSourceArchive, + DatabaseResolver, } from "../../../src/databases"; import { Logger } from "../../../src/common"; import { ProgressCallback } from "../../../src/commandRunner"; @@ -21,6 +22,7 @@ import { import { testDisposeHandler } from "../test-dispose-handler"; import { QueryRunner } from "../../../src/queryRunner"; import * as helpers from "../../../src/helpers"; +import { Setting } from "../../../src/config"; describe("databases", () => { const MOCK_DB_OPTIONS: FullDatabaseOptions = { @@ -623,6 +625,98 @@ describe("databases", () => { }); }); + describe("openDatabase", () => { + let createSkeletonPacksSpy: jest.SpyInstance; + let resolveDatabaseContentsSpy: jest.SpyInstance; + let addDatabaseSourceArchiveFolderSpy: jest.SpyInstance; + let mockDbItem: DatabaseItemImpl; + + beforeEach(() => { + createSkeletonPacksSpy = jest + .spyOn(databaseManager, "createSkeletonPacks") + .mockImplementation(async () => { + /* no-op */ + }); + + resolveDatabaseContentsSpy = jest + .spyOn(DatabaseResolver, "resolveDatabaseContents") + .mockResolvedValue({} as DatabaseContents); + + addDatabaseSourceArchiveFolderSpy = jest.spyOn( + databaseManager, + "addDatabaseSourceArchiveFolder", + ); + + jest.mock("fs", () => ({ + promises: { + pathExists: jest.fn().mockResolvedValue(true), + }, + })); + + mockDbItem = createMockDB(); + }); + + it("should resolve the database contents", async () => { + await databaseManager.openDatabase( + {} as ProgressCallback, + {} as CancellationToken, + mockDbItem.databaseUri, + ); + + expect(resolveDatabaseContentsSpy).toBeCalledTimes(1); + }); + + it("should add database source archive folder", async () => { + await databaseManager.openDatabase( + {} as ProgressCallback, + {} as CancellationToken, + mockDbItem.databaseUri, + ); + + expect(addDatabaseSourceArchiveFolderSpy).toBeCalledTimes(1); + }); + + describe("when codeQL.codespacesTemplate is set to true", () => { + it("should create a skeleton QL pack", async () => { + jest.spyOn(Setting.prototype, "getValue").mockReturnValue(true); + + await databaseManager.openDatabase( + {} as ProgressCallback, + {} as CancellationToken, + mockDbItem.databaseUri, + ); + + expect(createSkeletonPacksSpy).toBeCalledTimes(1); + }); + }); + + describe("when codeQL.codespacesTemplate is set to false", () => { + 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, + ); + expect(createSkeletonPacksSpy).toBeCalledTimes(0); + }); + }); + + describe("when codeQL.codespacesTemplate is not set", () => { + 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, + ); + expect(createSkeletonPacksSpy).toBeCalledTimes(0); + }); + }); + }); + function createMockDB( mockDbOptions = MOCK_DB_OPTIONS, // source archive location must be a real(-ish) location since