From e1dae0bf012d3759843f2473c45c46709f0ad465 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 12 Apr 2023 15:49:52 -0400 Subject: [PATCH 1/9] Avoid identifier collision with new VS Code version --- extensions/ql-vscode/src/databases/ui/db-panel.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/extensions/ql-vscode/src/databases/ui/db-panel.ts b/extensions/ql-vscode/src/databases/ui/db-panel.ts index 80a0eec5b0c..7ea33368139 100644 --- a/extensions/ql-vscode/src/databases/ui/db-panel.ts +++ b/extensions/ql-vscode/src/databases/ui/db-panel.ts @@ -34,11 +34,11 @@ import { DatabasePanelCommands } from "../../common/commands"; import { App } from "../../common/app"; export interface RemoteDatabaseQuickPickItem extends QuickPickItem { - kind: string; + remoteDatabaseKind: string; } export interface AddListQuickPickItem extends QuickPickItem { - kind: DbListKind; + databaseKind: DbListKind; } export class DbPanel extends DisposableObject { @@ -113,19 +113,19 @@ export class DbPanel extends DisposableObject { ) { await this.addNewRemoteRepo(highlightedItem.parentListName); } else { - const quickPickItems = [ + const quickPickItems: RemoteDatabaseQuickPickItem[] = [ { label: "$(repo) From a GitHub repository", detail: "Add a variant analysis repository from GitHub", alwaysShow: true, - kind: "repo", + remoteDatabaseKind: "repo", }, { label: "$(organization) All repositories of a GitHub org or owner", detail: "Add a variant analysis list of repositories from a GitHub organization/owner", alwaysShow: true, - kind: "owner", + remoteDatabaseKind: "owner", }, ]; const databaseKind = @@ -142,9 +142,9 @@ export class DbPanel extends DisposableObject { // We set 'true' to make this a silent exception. throw new UserCancellationException("No repository selected", true); } - if (databaseKind.kind === "repo") { + if (databaseKind.remoteDatabaseKind === "repo") { await this.addNewRemoteRepo(); - } else if (databaseKind.kind === "owner") { + } else if (databaseKind.remoteDatabaseKind === "owner") { await this.addNewRemoteOwner(); } } From 4804220971fcded6907a30736eeb443f356d1a02 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 12 Apr 2023 15:50:27 -0400 Subject: [PATCH 2/9] Use native VS Code test UI in canary --- extensions/ql-vscode/package-lock.json | 14 +- extensions/ql-vscode/package.json | 9 +- extensions/ql-vscode/src/cli.ts | 19 +- extensions/ql-vscode/src/extension.ts | 44 ++- extensions/ql-vscode/src/test-adapter.ts | 204 ++-------- extensions/ql-vscode/src/test-manager-base.ts | 76 ++++ extensions/ql-vscode/src/test-manager.ts | 371 ++++++++++++++++++ extensions/ql-vscode/src/test-runner.ts | 136 +++++++ extensions/ql-vscode/src/test-ui.ts | 67 +--- 9 files changed, 688 insertions(+), 252 deletions(-) create mode 100644 extensions/ql-vscode/src/test-manager-base.ts create mode 100644 extensions/ql-vscode/src/test-manager.ts create mode 100644 extensions/ql-vscode/src/test-runner.ts diff --git a/extensions/ql-vscode/package-lock.json b/extensions/ql-vscode/package-lock.json index 896a2b67756..cd9637b99a5 100644 --- a/extensions/ql-vscode/package-lock.json +++ b/extensions/ql-vscode/package-lock.json @@ -93,7 +93,7 @@ "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", - "@types/vscode": "^1.59.0", + "@types/vscode": "^1.67.0", "@types/webpack": "^5.28.0", "@types/webpack-env": "^1.18.0", "@types/xml2js": "~0.4.4", @@ -17194,9 +17194,9 @@ } }, "node_modules/@types/vscode": { - "version": "1.63.1", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.63.1.tgz", - "integrity": "sha512-Z+ZqjRcnGfHP86dvx/BtSwWyZPKQ/LBdmAVImY82TphyjOw2KgTKcp7Nx92oNwCTsHzlshwexAG/WiY2JuUm3g==", + "version": "1.77.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.77.0.tgz", + "integrity": "sha512-MWFN5R7a33n8eJZJmdVlifjig3LWUNRrPeO1xemIcZ0ae0TEQuRc7G2xV0LUX78RZFECY1plYBn+dP/Acc3L0Q==", "dev": true }, "node_modules/@types/webpack": { @@ -58329,9 +58329,9 @@ } }, "@types/vscode": { - "version": "1.63.1", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.63.1.tgz", - "integrity": "sha512-Z+ZqjRcnGfHP86dvx/BtSwWyZPKQ/LBdmAVImY82TphyjOw2KgTKcp7Nx92oNwCTsHzlshwexAG/WiY2JuUm3g==", + "version": "1.77.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.77.0.tgz", + "integrity": "sha512-MWFN5R7a33n8eJZJmdVlifjig3LWUNRrPeO1xemIcZ0ae0TEQuRc7G2xV0LUX78RZFECY1plYBn+dP/Acc3L0Q==", "dev": true }, "@types/webpack": { diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index 819e78f9a5d..dc75fc5b9d9 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -973,6 +973,13 @@ "when": "viewItem == testWithSource" } ], + "testing/item/context": [ + { + "command": "codeQLTests.acceptOutput", + "group": "qltest@1", + "when": "controllerId == codeql && testId =~ /^test /" + } + ], "explorer/context": [ { "command": "codeQL.setCurrentDatabase", @@ -1523,7 +1530,7 @@ "@types/through2": "^2.0.36", "@types/tmp": "^0.1.0", "@types/unzipper": "~0.10.1", - "@types/vscode": "^1.59.0", + "@types/vscode": "^1.67.0", "@types/webpack": "^5.28.0", "@types/webpack-env": "^1.18.0", "@types/xml2js": "~0.4.4", diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 45c6a7f6c56..2d88e693aee 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -23,7 +23,7 @@ import { getErrorStack, } from "./pure/helpers-pure"; import { QueryMetadata, SortDirection } from "./pure/interface-types"; -import { Logger, ProgressReporter } from "./common"; +import { BaseLogger, Logger, ProgressReporter } from "./common"; import { CompilationMessage } from "./pure/legacy-messages"; import { sarifParser } from "./sarif-parser"; import { walkDirectory } from "./helpers"; @@ -134,6 +134,7 @@ export interface TestCompleted { compilationMs: number; evaluationMs: number; expected: string; + actual?: string; diff: string[] | undefined; failureDescription?: string; failureStage?: string; @@ -424,7 +425,7 @@ export class CodeQLCliServer implements Disposable { command: string[], commandArgs: string[], cancellationToken?: CancellationToken, - logger?: Logger, + logger?: BaseLogger, ): AsyncGenerator { // Add format argument first, in case commandArgs contains positional parameters. const args = [...command, "--format", "jsonz", ...commandArgs]; @@ -453,9 +454,13 @@ export class CodeQLCliServer implements Disposable { ])) { yield event; } - - await childPromise; } finally { + try { + await childPromise; + } catch (_e) { + // We need to await this to avoid an unhandled rejection, but we want to propagate the + // original exception. + } if (cancellationRegistration !== undefined) { cancellationRegistration.dispose(); } @@ -482,7 +487,7 @@ export class CodeQLCliServer implements Disposable { logger, }: { cancellationToken?: CancellationToken; - logger?: Logger; + logger?: BaseLogger; } = {}, ): AsyncGenerator { for await (const event of this.runAsyncCodeQlCliCommandInternal( @@ -761,7 +766,7 @@ export class CodeQLCliServer implements Disposable { logger, }: { cancellationToken?: CancellationToken; - logger?: Logger; + logger?: BaseLogger; }, ): AsyncGenerator { const subcommandArgs = this.cliConfig.additionalTestArguments.concat([ @@ -1623,7 +1628,7 @@ const lineEndings = ["\r\n", "\r", "\n"]; * @param stream The stream to log. * @param logger The logger that will consume the stream output. */ -async function logStream(stream: Readable, logger: Logger): Promise { +async function logStream(stream: Readable, logger: BaseLogger): Promise { for await (const line of splitStreamAtSeparators(stream, lineEndings)) { // Await the result of log here in order to ensure the logs are written in the correct order. await logger.log(line); diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 77547d85ee2..b8bc258c0be 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -30,6 +30,7 @@ import { CodeQLCliServer } from "./cli"; import { CliConfigListener, DistributionConfigListener, + isCanary, joinOrderWarningThreshold, QueryHistoryConfigListener, QueryServerConfigListener, @@ -113,7 +114,6 @@ import { BaseCommands, PreActivationCommands, QueryServerCommands, - TestUICommands, } from "./common/commands"; import { LocalQueries } from "./local-queries"; import { getAstCfgCommands } from "./ast-cfg-commands"; @@ -121,6 +121,9 @@ import { getQueryEditorCommands } from "./query-editor"; import { App } from "./common/app"; import { registerCommandWithErrorHandling } from "./common/vscode/commands"; import { DataExtensionsEditorModule } from "./data-extensions-editor/data-extensions-editor-module"; +import { TestManager } from "./test-manager"; +import { TestRunner } from "./test-runner"; +import { TestManagerBase } from "./test-manager-base"; /** * extension.ts @@ -876,25 +879,34 @@ async function activateWithInstalledDistribution( ); void extLogger.log("Initializing QLTest interface."); - const testExplorerExtension = extensions.getExtension( - testExplorerExtensionId, - ); - let testUiCommands: Partial = {}; - if (testExplorerExtension) { - const testHub = testExplorerExtension.exports; - const testAdapterFactory = new QLTestAdapterFactory( - testHub, - cliServer, - dbm, - ); - ctx.subscriptions.push(testAdapterFactory); - const testUIService = new TestUIService(app, testHub); - ctx.subscriptions.push(testUIService); + const testRunner = new TestRunner(dbm, cliServer); + ctx.subscriptions.push(testRunner); + + let testManager: TestManagerBase | undefined = undefined; + if (isCanary()) { + testManager = new TestManager(app, testRunner, cliServer); + ctx.subscriptions.push(testManager); + } else { + const testExplorerExtension = extensions.getExtension( + testExplorerExtensionId, + ); + if (testExplorerExtension) { + const testHub = testExplorerExtension.exports; + const testAdapterFactory = new QLTestAdapterFactory( + testHub, + testRunner, + cliServer, + ); + ctx.subscriptions.push(testAdapterFactory); - testUiCommands = testUIService.getCommands(); + testManager = new TestUIService(app, testHub); + ctx.subscriptions.push(testManager); + } } + const testUiCommands = testManager?.getCommands() ?? {}; + const astViewer = new AstViewer(); const astTemplateProvider = new TemplatePrintAstProvider( cliServer, diff --git a/extensions/ql-vscode/src/test-adapter.ts b/extensions/ql-vscode/src/test-adapter.ts index 326f93456e6..38a6e383726 100644 --- a/extensions/ql-vscode/src/test-adapter.ts +++ b/extensions/ql-vscode/src/test-adapter.ts @@ -1,4 +1,3 @@ -import { access } from "fs-extra"; import { dirname, extname } from "path"; import * as vscode from "vscode"; import { @@ -20,23 +19,11 @@ import { QLTestDirectory, QLTestDiscovery, } from "./qltest-discovery"; -import { - Event, - EventEmitter, - CancellationTokenSource, - CancellationToken, -} from "vscode"; +import { Event, EventEmitter, CancellationTokenSource } from "vscode"; import { DisposableObject } from "./pure/disposable-object"; -import { CodeQLCliServer } from "./cli"; -import { - getOnDiskWorkspaceFolders, - showAndLogExceptionWithTelemetry, - showAndLogWarningMessage, -} from "./helpers"; +import { CodeQLCliServer, TestCompleted } from "./cli"; import { testLogger } from "./common"; -import { DatabaseItem, DatabaseManager } from "./local-databases"; -import { asError, getErrorMessage } from "./pure/helpers-pure"; -import { redactableError } from "./pure/errors"; +import { TestRunner } from "./test-runner"; /** * Get the full path of the `.expected` file for the specified QL test. @@ -77,8 +64,8 @@ function getTestOutputFile(testPath: string, extension: string): string { export class QLTestAdapterFactory extends DisposableObject { constructor( testHub: TestHub, + testRunner: TestRunner, cliServer: CodeQLCliServer, - databaseManager: DatabaseManager, ) { super(); @@ -87,7 +74,7 @@ export class QLTestAdapterFactory extends DisposableObject { new TestAdapterRegistrar( testHub, (workspaceFolder) => - new QLTestAdapter(workspaceFolder, cliServer, databaseManager), + new QLTestAdapter(workspaceFolder, testRunner, cliServer), ), ); } @@ -120,8 +107,8 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter { constructor( public readonly workspaceFolder: vscode.WorkspaceFolder, - private readonly cliServer: CodeQLCliServer, - private readonly databaseManager: DatabaseManager, + private readonly testRunner: TestRunner, + cliServer: CodeQLCliServer, ) { super(); @@ -232,110 +219,14 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter { tests, } as TestRunStartedEvent); - const currentDatabaseUri = - this.databaseManager.currentDatabaseItem?.databaseUri; - const databasesUnderTest: DatabaseItem[] = []; - for (const database of this.databaseManager.databaseItems) { - for (const test of tests) { - if (await database.isAffectedByTest(test)) { - databasesUnderTest.push(database); - break; - } - } - } - - await this.removeDatabasesBeforeTests(databasesUnderTest, token); - try { - await this.runTests(tests, token); - } catch (e) { - // CodeQL testing can throw exception even in normal scenarios. For example, if the test run - // produces no output (which is normal), the testing command would throw an exception on - // unexpected EOF during json parsing. So nothing needs to be done here - all the relevant - // error information (if any) should have already been written to the test logger. - } - await this.reopenDatabasesAfterTests( - databasesUnderTest, - currentDatabaseUri, - token, + await this.testRunner.run(tests, testLogger, token, (event) => + this.processTestEvent(event), ); this._testStates.fire({ type: "finished" } as TestRunFinishedEvent); this.clearTask(); } - private async removeDatabasesBeforeTests( - databasesUnderTest: DatabaseItem[], - token: vscode.CancellationToken, - ): Promise { - for (const database of databasesUnderTest) { - try { - await this.databaseManager.removeDatabaseItem( - (_) => { - /* no progress reporting */ - }, - token, - 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. - // So we need to display the error message ourselves and then rethrow. - void showAndLogExceptionWithTelemetry( - redactableError(asError(e))`Cannot remove database ${ - database.name - }: ${getErrorMessage(e)}`, - ); - throw e; - } - } - } - - private async reopenDatabasesAfterTests( - databasesUnderTest: DatabaseItem[], - currentDatabaseUri: vscode.Uri | undefined, - token: vscode.CancellationToken, - ): Promise { - for (const closedDatabase of databasesUnderTest) { - const uri = closedDatabase.databaseUri; - if (await this.isFileAccessible(uri)) { - try { - const reopenedDatabase = await this.databaseManager.openDatabase( - (_) => { - /* no progress reporting */ - }, - token, - uri, - ); - await this.databaseManager.renameDatabaseItem( - reopenedDatabase, - closedDatabase.name, - ); - if (currentDatabaseUri?.toString() === uri.toString()) { - await this.databaseManager.setCurrentDatabaseItem( - reopenedDatabase, - true, - ); - } - } 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. - // So we need to display the error message ourselves and then rethrow. - void showAndLogWarningMessage(`Cannot reopen database ${uri}: ${e}`); - throw e; - } - } - } - } - - private async isFileAccessible(uri: vscode.Uri): Promise { - try { - await access(uri.fsPath); - return true; - } catch { - return false; - } - } - private clearTask(): void { if (this.runningTask !== undefined) { const runningTask = this.runningTask; @@ -352,49 +243,40 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter { } } - private async runTests( - tests: string[], - cancellationToken: CancellationToken, - ): Promise { - const workspacePaths = getOnDiskWorkspaceFolders(); - for await (const event of this.cliServer.runTests(tests, workspacePaths, { - cancellationToken, - logger: testLogger, - })) { - const state = event.pass - ? "passed" - : event.messages?.length - ? "errored" - : "failed"; - let message: string | undefined; - if (event.failureDescription || event.diff?.length) { - message = - event.failureStage === "RESULT" - ? [ - "", - `${state}: ${event.test}`, - event.failureDescription || event.diff?.join("\n"), - "", - ].join("\n") - : [ - "", - `${event.failureStage?.toLowerCase()} error: ${event.test}`, - event.failureDescription || - `${event.messages[0].severity}: ${event.messages[0].message}`, - "", - ].join("\n"); - void testLogger.log(message); - } - this._testStates.fire({ - type: "test", - state, - test: event.test, - message, - decorations: event.messages?.map((msg) => ({ - line: msg.position.line, - message: msg.message, - })), - }); + private async processTestEvent(event: TestCompleted): Promise { + const state = event.pass + ? "passed" + : event.messages?.length + ? "errored" + : "failed"; + let message: string | undefined; + if (event.failureDescription || event.diff?.length) { + message = + event.failureStage === "RESULT" + ? [ + "", + `${state}: ${event.test}`, + event.failureDescription || event.diff?.join("\n"), + "", + ].join("\n") + : [ + "", + `${event.failureStage?.toLowerCase()} error: ${event.test}`, + event.failureDescription || + `${event.messages[0].severity}: ${event.messages[0].message}`, + "", + ].join("\n"); + void testLogger.log(message); } + this._testStates.fire({ + type: "test", + state, + test: event.test, + message, + decorations: event.messages?.map((msg) => ({ + line: msg.position.line, + message: msg.message, + })), + }); } } diff --git a/extensions/ql-vscode/src/test-manager-base.ts b/extensions/ql-vscode/src/test-manager-base.ts new file mode 100644 index 00000000000..c2a1058c9fb --- /dev/null +++ b/extensions/ql-vscode/src/test-manager-base.ts @@ -0,0 +1,76 @@ +import { copy, createFile, lstat, pathExists } from "fs-extra"; +import { TestUICommands } from "./common/commands"; +import { DisposableObject } from "./pure/disposable-object"; +import { getActualFile, getExpectedFile } from "./test-adapter"; +import { TestItem, TextDocumentShowOptions, Uri, window } from "vscode"; +import { showAndLogWarningMessage } from "./helpers"; +import { basename } from "path"; +import { App } from "./common/app"; +import { TestTreeNode } from "./test-tree-node"; + +export type TestNode = TestTreeNode | TestItem; + +/** + * Base class for both the legacy and new test services. Implements commands that are common to + * both. + */ +export abstract class TestManagerBase extends DisposableObject { + protected constructor(private readonly app: App) { + super(); + } + + public getCommands(): TestUICommands { + return { + "codeQLTests.showOutputDifferences": + this.showOutputDifferences.bind(this), + "codeQLTests.acceptOutput": this.acceptOutput.bind(this), + }; + } + + /** Override to compute the path of the test file from the selected node. */ + protected abstract getTestPath(node: TestNode): string; + + private async acceptOutput(node: TestNode): Promise { + const testPath = this.getTestPath(node); + const stat = await lstat(testPath); + if (stat.isFile()) { + const expectedPath = getExpectedFile(testPath); + const actualPath = getActualFile(testPath); + await copy(actualPath, expectedPath, { overwrite: true }); + } + } + + private async showOutputDifferences(node: TestNode): Promise { + const testId = this.getTestPath(node); + const stat = await lstat(testId); + if (stat.isFile()) { + const expectedPath = getExpectedFile(testId); + const expectedUri = Uri.file(expectedPath); + const actualPath = getActualFile(testId); + const options: TextDocumentShowOptions = { + preserveFocus: true, + preview: true, + }; + + if (!(await pathExists(expectedPath))) { + void showAndLogWarningMessage( + `'${basename(expectedPath)}' does not exist. Creating an empty file.`, + ); + await createFile(expectedPath); + } + + if (await pathExists(actualPath)) { + const actualUri = Uri.file(actualPath); + await this.app.commands.execute( + "vscode.diff", + expectedUri, + actualUri, + `Expected vs. Actual for ${basename(testId)}`, + options, + ); + } else { + await window.showTextDocument(expectedUri, options); + } + } + } +} diff --git a/extensions/ql-vscode/src/test-manager.ts b/extensions/ql-vscode/src/test-manager.ts new file mode 100644 index 00000000000..70f21750382 --- /dev/null +++ b/extensions/ql-vscode/src/test-manager.ts @@ -0,0 +1,371 @@ +import { readFile } from "fs-extra"; +import { + CancellationToken, + Location, + Range, + TestController, + TestItem, + TestMessage, + TestRun, + TestRunProfileKind, + TestRunRequest, + Uri, + WorkspaceFolder, + WorkspaceFoldersChangeEvent, + tests, + workspace, +} from "vscode"; +import { DisposableObject } from "./pure/disposable-object"; +import { + QLTestDirectory, + QLTestDiscovery, + QLTestFile, + QLTestNode, +} from "./qltest-discovery"; +import { CodeQLCliServer } from "./cli"; +import { getErrorMessage } from "./pure/helpers-pure"; +import { BaseLogger, LogOptions } from "./common"; +import { TestRunner } from "./test-runner"; +import { TestManagerBase } from "./test-manager-base"; +import { App } from "./common/app"; + +/** + * Returns the complete text content of the specified file. If there is an error reading the file, + * an error message is added to `testMessages` and this function returns undefined. + */ +async function tryReadFileContents( + path: string, + testMessages: TestMessage[], +): Promise { + try { + return await readFile(path, { encoding: "utf-8" }); + } catch (e) { + testMessages.push( + new TestMessage( + `Error reading from file '${path}': ${getErrorMessage(e)}`, + ), + ); + return undefined; + } +} + +function forEachTest(testItem: TestItem, op: (test: TestItem) => void): void { + if (testItem.children.size > 0) { + // This is a directory, so recurse into the children. + for (const [, child] of testItem.children) { + forEachTest(child, op); + } + } else { + // This is a leaf node, so it's a test. + op(testItem); + } +} + +/** + * Implementation of `BaseLogger` that logs to the output of a `TestRun`. + */ +class TestRunLogger implements BaseLogger { + public constructor(private readonly testRun: TestRun) {} + + public async log(message: string, options?: LogOptions): Promise { + // "\r\n" because that's what the test terminal wants. + const lineEnding = options?.trailingNewline === false ? "" : "\r\n"; + this.testRun.appendOutput(message + lineEnding); + } +} + +/** + * Handles test dicovery for a specific workspace folder, and reports back to `TestManager`. + */ +class WorkspaceFolderHandler extends DisposableObject { + private readonly testDiscovery: QLTestDiscovery; + + public constructor( + private readonly workspaceFolder: WorkspaceFolder, + private readonly testUI: TestManager, + cliServer: CodeQLCliServer, + ) { + super(); + + this.testDiscovery = new QLTestDiscovery(workspaceFolder, cliServer); + this.push( + this.testDiscovery.onDidChangeTests(this.handleDidChangeTests, this), + ); + this.testDiscovery.refresh(); + } + + private handleDidChangeTests(): void { + const testDirectory = this.testDiscovery.testDirectory; + + this.testUI.updateTestsForWorkspaceFolder( + this.workspaceFolder, + testDirectory, + ); + } +} + +/** + * Service that populates the VS Code "Test Explorer" panel for CodeQL, and handles running and + * debugging of tests. + */ +export class TestManager extends TestManagerBase { + private readonly testController: TestController = tests.createTestController( + "codeql", + "Fancy CodeQL Tests", + ); + + /** + * Maps from each workspace folder being tracked to the `WorkspaceFolderHandler` responsible for + * tracking it. + */ + private readonly workspaceFolderHandlers = new Map< + WorkspaceFolder, + WorkspaceFolderHandler + >(); + + public constructor( + app: App, + private readonly testRunner: TestRunner, + private readonly cliServer: CodeQLCliServer, + ) { + super(app); + + this.testController.createRunProfile( + "Run", + TestRunProfileKind.Run, + this.run.bind(this), + ); + + // Start by tracking whatever folders are currently in the workspace. + this.startTrackingWorkspaceFolders(workspace.workspaceFolders ?? []); + + // Listen for changes to the set of workspace folders. + workspace.onDidChangeWorkspaceFolders( + this.handleDidChangeWorkspaceFolders, + this, + ); + } + + public dispose(): void { + this.workspaceFolderHandlers.clear(); // These will be disposed in the `super.dispose()` call. + super.dispose(); + } + + protected getTestPath(node: TestItem): string { + if (node.uri === undefined || node.uri.scheme !== "file") { + throw new Error("Selected test is not a CodeQL test."); + } + return node.uri!.fsPath; + } + + /** Start tracking tests in the specified workspace folders. */ + private startTrackingWorkspaceFolders( + workspaceFolders: readonly WorkspaceFolder[], + ): void { + for (const workspaceFolder of workspaceFolders) { + const workspaceFolderHandler = new WorkspaceFolderHandler( + workspaceFolder, + this, + this.cliServer, + ); + this.track(workspaceFolderHandler); + this.workspaceFolderHandlers.set(workspaceFolder, workspaceFolderHandler); + } + } + + /** Stop tracking tests in the specified workspace folders. */ + private stopTrackingWorkspaceFolders( + workspaceFolders: readonly WorkspaceFolder[], + ): void { + for (const workspaceFolder of workspaceFolders) { + const workspaceFolderHandler = + this.workspaceFolderHandlers.get(workspaceFolder); + if (workspaceFolderHandler !== undefined) { + // Delete the root item for this workspace folder, if any. + this.testController.items.delete(workspaceFolder.uri.toString()); + this.disposeAndStopTracking(workspaceFolderHandler); + this.workspaceFolderHandlers.delete(workspaceFolder); + } + } + } + + private handleDidChangeWorkspaceFolders( + e: WorkspaceFoldersChangeEvent, + ): void { + this.startTrackingWorkspaceFolders(e.added); + this.stopTrackingWorkspaceFolders(e.removed); + } + + /** + * Update the test controller when we discover changes to the tests in the workspace folder. + */ + public updateTestsForWorkspaceFolder( + workspaceFolder: WorkspaceFolder, + testDirectory: QLTestDirectory | undefined, + ): void { + if (testDirectory !== undefined) { + // Adding an item with the same ID as an existing item will replace it, which is exactly what + // we want. + // Test discovery returns a root `QLTestDirectory` representing the workspace folder itself, + // named after the `WorkspaceFolder` object's `name` property. We can map this directly to a + // `TestItem`. + this.testController.items.add( + this.createTestItemTree(testDirectory, true), + ); + } else { + // No tests, so delete any existing item. + this.testController.items.delete(workspaceFolder.uri.toString()); + } + } + + /** + * Creates a tree of `TestItem`s from the root `QlTestNode` provided by test discovery. + */ + private createTestItemTree(node: QLTestNode, isRoot: boolean): TestItem { + // Prefix the ID to identify it as a directory or a test + const itemType = node instanceof QLTestDirectory ? "dir" : "test"; + const testItem = this.testController.createTestItem( + // For the root of a workspace folder, use the full path as the ID. Otherwise, use the node's + // name as the ID, since it's shorter but still unique. + `${itemType} ${isRoot ? node.path : node.name}`, + node.name, + Uri.file(node.path), + ); + + for (const childNode of node.children) { + const childItem = this.createTestItemTree(childNode, false); + if (childNode instanceof QLTestFile) { + childItem.range = new Range(0, 0, 0, 0); + } + testItem.children.add(childItem); + } + + return testItem; + } + + /** + * Run the tests specified by the `TestRunRequest` parameter. + */ + private async run( + request: TestRunRequest, + token: CancellationToken, + ): Promise { + const testsToRun = this.computeTestsToRun(request); + const testRun = this.testController.createTestRun(request, undefined, true); + try { + const tests: string[] = []; + testsToRun.forEach((t) => { + testRun.enqueued(t); + tests.push(t.uri!.fsPath); + }); + + const logger = new TestRunLogger(testRun); + + await this.testRunner.run(tests, logger, token, async (event) => { + // Pass the test path from the event through `Uri` and back via `fsPath` so that it matches + // the canonicalization of the URI that we used to create the `TestItem`. + const testItem = testsToRun.get(Uri.file(event.test).fsPath); + if (testItem === undefined) { + throw new Error( + `Unexpected result from unknown test '${event.test}'.`, + ); + } + + const duration = event.compilationMs + event.evaluationMs; + if (event.pass) { + testRun.passed(testItem, duration); + } else { + // Construct a list of `TestMessage`s to report for the failure. + const testMessages: TestMessage[] = []; + if (event.failureDescription !== undefined) { + testMessages.push(new TestMessage(event.failureDescription)); + } + if (event.diff?.length && event.actual !== undefined) { + // Actual results differ from expected results. Read both sets of results and create a + // diff to put in the message. + const expected = await tryReadFileContents( + event.expected, + testMessages, + ); + const actual = await tryReadFileContents( + event.actual, + testMessages, + ); + if (expected !== undefined && actual !== undefined) { + testMessages.push( + TestMessage.diff( + "Actual output differs from expected", + expected, + actual, + ), + ); + } + } + if (event.messages?.length > 0) { + // The test didn't make it far enough to produce results. Transform any error messages + // into `TestMessage`s and report the test as "errored". + const testMessages = event.messages.map((m) => { + const location = new Location( + Uri.file(m.position.fileName), + new Range( + m.position.line - 1, + m.position.column - 1, + m.position.endLine - 1, + m.position.endColumn - 1, + ), + ); + const testMessage = new TestMessage(m.message); + testMessage.location = location; + return testMessage; + }); + testRun.errored(testItem, testMessages, duration); + } else { + // Results didn't match expectations. Report the test as "failed". + if (testMessages.length === 0) { + // If we managed to get here without creating any `TestMessage`s, create a default one + // here. Any failed test needs at least one message. + testMessages.push(new TestMessage("Test failed")); + } + testRun.failed(testItem, testMessages, duration); + } + } + }); + } finally { + testRun.end(); + } + } + + /** + * Computes the set of tests to run as specified in the `TestRunRequest` object. + */ + private computeTestsToRun(request: TestRunRequest): Map { + const testsToRun = new Map(); + if (request.include !== undefined) { + // Include these tests, recursively expanding test directories into their list of contained + // tests. + for (const includedTestItem of request.include) { + forEachTest(includedTestItem, (testItem) => + testsToRun.set(testItem.uri!.fsPath, testItem), + ); + } + } else { + // Include all of the tests. + for (const [, includedTestItem] of this.testController.items) { + forEachTest(includedTestItem, (testItem) => + testsToRun.set(testItem.uri!.fsPath, testItem), + ); + } + } + if (request.exclude !== undefined) { + // Exclude the specified tests from the set we've computed so far, again recursively expanding + // test directories into their list of contained tests. + for (const excludedTestItem of request.exclude) { + forEachTest(excludedTestItem, (testItem) => + testsToRun.delete(testItem.uri!.fsPath), + ); + } + } + + return testsToRun; + } +} diff --git a/extensions/ql-vscode/src/test-runner.ts b/extensions/ql-vscode/src/test-runner.ts new file mode 100644 index 00000000000..f691497b6ff --- /dev/null +++ b/extensions/ql-vscode/src/test-runner.ts @@ -0,0 +1,136 @@ +import { CancellationToken, Uri } from "vscode"; +import { CodeQLCliServer, TestCompleted } from "./cli"; +import { DatabaseItem, DatabaseManager } from "./local-databases"; +import { + getOnDiskWorkspaceFolders, + showAndLogExceptionWithTelemetry, + showAndLogWarningMessage, +} from "./helpers"; +import { asError, getErrorMessage } from "./pure/helpers-pure"; +import { redactableError } from "./pure/errors"; +import { access } from "fs-extra"; +import { BaseLogger } from "./common"; +import { DisposableObject } from "./pure/disposable-object"; + +async function isFileAccessible(uri: Uri): Promise { + try { + await access(uri.fsPath); + return true; + } catch { + return false; + } +} + +export class TestRunner extends DisposableObject { + public constructor( + private readonly databaseManager: DatabaseManager, + private readonly cliServer: CodeQLCliServer, + ) { + super(); + } + + public async run( + tests: string[], + logger: BaseLogger, + token: CancellationToken, + eventHandler: (event: TestCompleted) => Promise, + ): Promise { + const currentDatabaseUri = + this.databaseManager.currentDatabaseItem?.databaseUri; + const databasesUnderTest: DatabaseItem[] = []; + for (const database of this.databaseManager.databaseItems) { + for (const test of tests) { + if (await database.isAffectedByTest(test)) { + databasesUnderTest.push(database); + break; + } + } + } + + await this.removeDatabasesBeforeTests(databasesUnderTest, token); + try { + const workspacePaths = getOnDiskWorkspaceFolders(); + for await (const event of this.cliServer.runTests(tests, workspacePaths, { + cancellationToken: token, + logger, + })) { + await eventHandler(event); + } + } catch (e) { + // CodeQL testing can throw exception even in normal scenarios. For example, if the test run + // produces no output (which is normal), the testing command would throw an exception on + // unexpected EOF during json parsing. So nothing needs to be done here - all the relevant + // error information (if any) should have already been written to the test logger. + } finally { + 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, + ); + } 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. + // So we need to display the error message ourselves and then rethrow. + void showAndLogExceptionWithTelemetry( + redactableError(asError(e))`Cannot remove database ${ + database.name + }: ${getErrorMessage(e)}`, + ); + throw e; + } + } + } + + 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, + ); + await this.databaseManager.renameDatabaseItem( + reopenedDatabase, + closedDatabase.name, + ); + if (currentDatabaseUri?.toString() === uri.toString()) { + await this.databaseManager.setCurrentDatabaseItem( + reopenedDatabase, + true, + ); + } + } 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. + // So we need to display the error message ourselves and then rethrow. + void showAndLogWarningMessage(`Cannot reopen database ${uri}: ${e}`); + throw e; + } + } + } + } +} diff --git a/extensions/ql-vscode/src/test-ui.ts b/extensions/ql-vscode/src/test-ui.ts index 45b8f41a75d..aad2f1e47e7 100644 --- a/extensions/ql-vscode/src/test-ui.ts +++ b/extensions/ql-vscode/src/test-ui.ts @@ -1,6 +1,3 @@ -import { lstat, copy, pathExists, createFile } from "fs-extra"; -import { basename } from "path"; -import { Uri, TextDocumentShowOptions, window } from "vscode"; import { TestHub, TestController, @@ -10,13 +7,11 @@ import { TestEvent, TestSuiteEvent, } from "vscode-test-adapter-api"; - -import { showAndLogWarningMessage } from "./helpers"; import { TestTreeNode } from "./test-tree-node"; import { DisposableObject } from "./pure/disposable-object"; -import { QLTestAdapter, getExpectedFile, getActualFile } from "./test-adapter"; -import { TestUICommands } from "./common/commands"; +import { QLTestAdapter } from "./test-adapter"; import { App } from "./common/app"; +import { TestManagerBase } from "./test-manager-base"; type VSCodeTestEvent = | TestRunStartedEvent @@ -42,23 +37,15 @@ class QLTestListener extends DisposableObject { /** * Service that implements all UI and commands for QL tests. */ -export class TestUIService extends DisposableObject implements TestController { +export class TestUIService extends TestManagerBase implements TestController { private readonly listeners: Map = new Map(); - constructor(private readonly app: App, private readonly testHub: TestHub) { - super(); + public constructor(app: App, private readonly testHub: TestHub) { + super(app); testHub.registerTestController(this); } - public getCommands(): TestUICommands { - return { - "codeQLTests.showOutputDifferences": - this.showOutputDifferences.bind(this), - "codeQLTests.acceptOutput": this.acceptOutput.bind(this), - }; - } - public dispose(): void { this.testHub.unregisterTestController(this); @@ -75,47 +62,7 @@ export class TestUIService extends DisposableObject implements TestController { } } - private async acceptOutput(node: TestTreeNode): Promise { - const testId = node.info.id; - const stat = await lstat(testId); - if (stat.isFile()) { - const expectedPath = getExpectedFile(testId); - const actualPath = getActualFile(testId); - await copy(actualPath, expectedPath, { overwrite: true }); - } - } - - private async showOutputDifferences(node: TestTreeNode): Promise { - const testId = node.info.id; - const stat = await lstat(testId); - if (stat.isFile()) { - const expectedPath = getExpectedFile(testId); - const expectedUri = Uri.file(expectedPath); - const actualPath = getActualFile(testId); - const options: TextDocumentShowOptions = { - preserveFocus: true, - preview: true, - }; - - if (!(await pathExists(expectedPath))) { - void showAndLogWarningMessage( - `'${basename(expectedPath)}' does not exist. Creating an empty file.`, - ); - await createFile(expectedPath); - } - - if (await pathExists(actualPath)) { - const actualUri = Uri.file(actualPath); - await this.app.commands.execute( - "vscode.diff", - expectedUri, - actualUri, - `Expected vs. Actual for ${basename(testId)}`, - options, - ); - } else { - await window.showTextDocument(expectedUri, options); - } - } + protected getTestPath(node: TestTreeNode): string { + return node.info.id; } } From e986b07bc76d7339b54f8317e1809c9e390c5af2 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 12 Apr 2023 16:45:53 -0400 Subject: [PATCH 3/9] Add missing mock method after `@types/vscode` upgrade --- .../test/vscode-tests/no-workspace/helpers.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/helpers.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/helpers.test.ts index 98919219dfa..8eae5cd03a5 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/helpers.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/helpers.test.ts @@ -272,6 +272,13 @@ describe("helpers", () => { class MockEnvironmentVariableCollection implements EnvironmentVariableCollection { + [Symbol.iterator](): Iterator< + [variable: string, mutator: EnvironmentVariableMutator], + any, + undefined + > { + throw new Error("Method not implemented."); + } persistent = false; replace(_variable: string, _value: string): void { throw new Error("Method not implemented."); From 397b5852c15cae9ceae93ab6d61c12098dcb82c1 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 12 Apr 2023 16:54:07 -0400 Subject: [PATCH 4/9] Fix test adapter tests --- .../no-workspace/test-adapter.test.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts index ccc528155ee..42c26815ee1 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts @@ -9,6 +9,7 @@ import { FullDatabaseOptions, } from "../../../src/local-databases"; import { mockedObject } from "../utils/mocking.helpers"; +import { TestRunner } from "../../../src/test-runner"; jest.mock("fs-extra", () => { const original = jest.requireActual("fs-extra"); @@ -19,8 +20,10 @@ jest.mock("fs-extra", () => { }); describe("test-adapter", () => { + let testRunner: TestRunner; let adapter: QLTestAdapter; let fakeDatabaseManager: DatabaseManager; + let fakeCliServer: CodeQLCliServer; let currentDatabaseItem: DatabaseItem | undefined; let databaseItems: DatabaseItem[] = []; const openDatabaseSpy = jest.fn(); @@ -73,17 +76,21 @@ describe("test-adapter", () => { jest.spyOn(preTestDatabaseItem, "isAffectedByTest").mockResolvedValue(true); + fakeCliServer = mockedObject({ + runTests: runTestsSpy, + resolveQlpacks: resolveQlpacksSpy, + resolveTests: resolveTestsSpy, + }); + + testRunner = new TestRunner(fakeDatabaseManager, fakeCliServer); + adapter = new QLTestAdapter( mockedObject({ name: "ABC", uri: Uri.parse("file:/ab/c"), }), - mockedObject({ - runTests: runTestsSpy, - resolveQlpacks: resolveQlpacksSpy, - resolveTests: resolveTestsSpy, - }), - fakeDatabaseManager, + testRunner, + fakeCliServer, ); }); From cb93c84611d7d746b3dec6d8f22c53822b7f0c1a Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 13 Apr 2023 17:00:15 -0400 Subject: [PATCH 5/9] Test tests --- extensions/ql-vscode/src/test-manager.ts | 14 +- .../no-workspace/test-adapter.test.ts | 269 ++++++++---------- .../no-workspace/test-runner-helpers.ts | 96 +++++++ .../no-workspace/test-runner.test.ts | 189 ++++++++++++ 4 files changed, 409 insertions(+), 159 deletions(-) create mode 100644 extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner-helpers.ts create mode 100644 extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner.test.ts diff --git a/extensions/ql-vscode/src/test-manager.ts b/extensions/ql-vscode/src/test-manager.ts index 70f21750382..e8357746838 100644 --- a/extensions/ql-vscode/src/test-manager.ts +++ b/extensions/ql-vscode/src/test-manager.ts @@ -109,11 +109,6 @@ class WorkspaceFolderHandler extends DisposableObject { * debugging of tests. */ export class TestManager extends TestManagerBase { - private readonly testController: TestController = tests.createTestController( - "codeql", - "Fancy CodeQL Tests", - ); - /** * Maps from each workspace folder being tracked to the `WorkspaceFolderHandler` responsible for * tracking it. @@ -127,6 +122,11 @@ export class TestManager extends TestManagerBase { app: App, private readonly testRunner: TestRunner, private readonly cliServer: CodeQLCliServer, + // Having this as a parameter with a default value makes passing in a mock easier. + private readonly testController: TestController = tests.createTestController( + "codeql", + "Fancy CodeQL Tests", + ), ) { super(app); @@ -245,8 +245,10 @@ export class TestManager extends TestManagerBase { /** * Run the tests specified by the `TestRunRequest` parameter. + * + * Public because this is used in unit tests. */ - private async run( + public async run( request: TestRunRequest, token: CancellationToken, ): Promise { diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts index 42c26815ee1..3bc9f61206f 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-adapter.test.ts @@ -1,90 +1,46 @@ -import { Uri, WorkspaceFolder } from "vscode"; +import { + CancellationTokenSource, + Range, + TestItem, + TestItemCollection, + TestRun, + TestRunRequest, + Uri, + WorkspaceFolder, + tests, +} from "vscode"; import { QLTestAdapter } from "../../../src/test-adapter"; import { CodeQLCliServer } from "../../../src/cli"; -import { - DatabaseItem, - DatabaseItemImpl, - DatabaseManager, - FullDatabaseOptions, -} from "../../../src/local-databases"; +import { DatabaseManager } from "../../../src/local-databases"; import { mockedObject } from "../utils/mocking.helpers"; import { TestRunner } from "../../../src/test-runner"; +import { + createMockCliServerForTestRun, + mockEmptyDatabaseManager, + mockTestsInfo, +} from "./test-runner-helpers"; +import { TestManager } from "../../../src/test-manager"; +import { createMockApp } from "../../__mocks__/appMock"; -jest.mock("fs-extra", () => { - const original = jest.requireActual("fs-extra"); - return { - ...original, - access: jest.fn(), - }; -}); +type IdTestItemPair = [id: string, testItem: TestItem]; describe("test-adapter", () => { let testRunner: TestRunner; - let adapter: QLTestAdapter; let fakeDatabaseManager: DatabaseManager; let fakeCliServer: CodeQLCliServer; - let currentDatabaseItem: DatabaseItem | undefined; - let databaseItems: DatabaseItem[] = []; - const openDatabaseSpy = jest.fn(); - const removeDatabaseItemSpy = jest.fn(); - const renameDatabaseItemSpy = jest.fn(); - const setCurrentDatabaseItemSpy = jest.fn(); - const runTestsSpy = jest.fn(); - const resolveTestsSpy = jest.fn(); - const resolveQlpacksSpy = jest.fn(); - - const preTestDatabaseItem = new DatabaseItemImpl( - Uri.file("/path/to/test/dir/dir.testproj"), - undefined, - mockedObject({ displayName: "custom display name" }), - (_) => { - /* no change event listener */ - }, - ); - const postTestDatabaseItem = new DatabaseItemImpl( - Uri.file("/path/to/test/dir/dir.testproj"), - undefined, - mockedObject({ displayName: "default name" }), - (_) => { - /* no change event listener */ - }, - ); beforeEach(() => { - mockRunTests(); - openDatabaseSpy.mockResolvedValue(postTestDatabaseItem); - removeDatabaseItemSpy.mockResolvedValue(undefined); - renameDatabaseItemSpy.mockResolvedValue(undefined); - setCurrentDatabaseItemSpy.mockResolvedValue(undefined); - resolveQlpacksSpy.mockResolvedValue({}); - resolveTestsSpy.mockResolvedValue([]); - fakeDatabaseManager = mockedObject( - { - openDatabase: openDatabaseSpy, - removeDatabaseItem: removeDatabaseItemSpy, - renameDatabaseItem: renameDatabaseItemSpy, - setCurrentDatabaseItem: setCurrentDatabaseItemSpy, - }, - { - dynamicProperties: { - currentDatabaseItem: () => currentDatabaseItem, - databaseItems: () => databaseItems, - }, - }, - ); + fakeDatabaseManager = mockEmptyDatabaseManager(); - jest.spyOn(preTestDatabaseItem, "isAffectedByTest").mockResolvedValue(true); - - fakeCliServer = mockedObject({ - runTests: runTestsSpy, - resolveQlpacks: resolveQlpacksSpy, - resolveTests: resolveTestsSpy, - }); + const mockCli = createMockCliServerForTestRun(); + fakeCliServer = mockCli.cliServer; testRunner = new TestRunner(fakeDatabaseManager, fakeCliServer); + }); - adapter = new QLTestAdapter( + it("legacy test adapter should run some tests", async () => { + const adapter = new QLTestAdapter( mockedObject({ name: "ABC", uri: Uri.parse("file:/ab/c"), @@ -92,121 +48,128 @@ describe("test-adapter", () => { testRunner, fakeCliServer, ); - }); - it("should run some tests", async () => { const listenerSpy = jest.fn(); adapter.testStates(listenerSpy); - const testsPath = Uri.parse("file:/ab/c").fsPath; - const dPath = Uri.parse("file:/ab/c/d.ql").fsPath; - const gPath = Uri.parse("file:/ab/c/e/f/g.ql").fsPath; - const hPath = Uri.parse("file:/ab/c/e/f/h.ql").fsPath; - - await adapter.run([testsPath]); + await adapter.run([mockTestsInfo.testsPath]); expect(listenerSpy).toBeCalledTimes(5); expect(listenerSpy).toHaveBeenNthCalledWith(1, { type: "started", - tests: [testsPath], + tests: [mockTestsInfo.testsPath], }); expect(listenerSpy).toHaveBeenNthCalledWith(2, { type: "test", state: "passed", - test: dPath, + test: mockTestsInfo.dPath, message: undefined, decorations: [], }); expect(listenerSpy).toHaveBeenNthCalledWith(3, { type: "test", state: "errored", - test: gPath, - message: `\ncompilation error: ${gPath}\nERROR: abc\n`, + test: mockTestsInfo.gPath, + message: `\ncompilation error: ${mockTestsInfo.gPath}\nERROR: abc\n`, decorations: [{ line: 1, message: "abc" }], }); expect(listenerSpy).toHaveBeenNthCalledWith(4, { type: "test", state: "failed", - test: hPath, - message: `\nfailed: ${hPath}\njkh\ntuv\n`, + test: mockTestsInfo.hPath, + message: `\nfailed: ${mockTestsInfo.hPath}\njkh\ntuv\n`, decorations: [], }); expect(listenerSpy).toHaveBeenNthCalledWith(5, { type: "finished" }); }); - it("should reregister testproj databases around test run", async () => { - currentDatabaseItem = preTestDatabaseItem; - databaseItems = [preTestDatabaseItem]; - await adapter.run(["/path/to/test/dir"]); - - expect(removeDatabaseItemSpy.mock.invocationCallOrder[0]).toBeLessThan( - runTestsSpy.mock.invocationCallOrder[0], - ); - expect(openDatabaseSpy.mock.invocationCallOrder[0]).toBeGreaterThan( - runTestsSpy.mock.invocationCallOrder[0], - ); - expect(renameDatabaseItemSpy.mock.invocationCallOrder[0]).toBeGreaterThan( - openDatabaseSpy.mock.invocationCallOrder[0], - ); - expect( - setCurrentDatabaseItemSpy.mock.invocationCallOrder[0], - ).toBeGreaterThan(openDatabaseSpy.mock.invocationCallOrder[0]); - - expect(removeDatabaseItemSpy).toBeCalledTimes(1); - expect(removeDatabaseItemSpy).toBeCalledWith( - expect.anything(), - expect.anything(), - preTestDatabaseItem, + it("native test manager should run some tests", async () => { + const enqueuedSpy = jest.fn(); + const passedSpy = jest.fn(); + const erroredSpy = jest.fn(); + const failedSpy = jest.fn(); + const endSpy = jest.fn(); + + const testController = tests.createTestController("codeql", "CodeQL Tests"); + testController.createTestRun = jest.fn().mockImplementation(() => + mockedObject({ + enqueued: enqueuedSpy, + passed: passedSpy, + errored: erroredSpy, + failed: failedSpy, + end: endSpy, + }), ); - - expect(openDatabaseSpy).toBeCalledTimes(1); - expect(openDatabaseSpy).toBeCalledWith( - expect.anything(), - expect.anything(), - preTestDatabaseItem.databaseUri, + const testManager = new TestManager( + createMockApp({}), + testRunner, + fakeCliServer, + testController, ); - expect(renameDatabaseItemSpy).toBeCalledTimes(1); - expect(renameDatabaseItemSpy).toBeCalledWith( - postTestDatabaseItem, - preTestDatabaseItem.name, + const childItems: TestItem[] = [ + { + children: { size: 0 } as TestItemCollection, + id: `test ${mockTestsInfo.dPath}`, + uri: Uri.file(mockTestsInfo.dPath), + } as TestItem, + { + children: { size: 0 } as TestItemCollection, + id: `test ${mockTestsInfo.gPath}`, + uri: Uri.file(mockTestsInfo.gPath), + } as TestItem, + { + children: { size: 0 } as TestItemCollection, + id: `test ${mockTestsInfo.hPath}`, + uri: Uri.file(mockTestsInfo.hPath), + } as TestItem, + ]; + const childElements: IdTestItemPair[] = childItems.map((childItem) => [ + childItem.id, + childItem, + ]); + const childIteratorFunc: () => Iterator = () => + childElements[Symbol.iterator](); + + const rootItem = { + id: `dir ${mockTestsInfo.testsPath}`, + uri: Uri.file(mockTestsInfo.testsPath), + children: { + size: 3, + [Symbol.iterator]: childIteratorFunc, + } as TestItemCollection, + } as TestItem; + + const request = new TestRunRequest([rootItem]); + await testManager.run(request, new CancellationTokenSource().token); + + expect(enqueuedSpy).toBeCalledTimes(3); + expect(passedSpy).toBeCalledTimes(1); + expect(passedSpy).toHaveBeenCalledWith(childItems[0], 3000); + expect(erroredSpy).toHaveBeenCalledTimes(1); + expect(erroredSpy).toHaveBeenCalledWith( + childItems[1], + [ + { + location: { + range: new Range(0, 0, 1, 1), + uri: Uri.file(mockTestsInfo.gPath), + }, + message: "abc", + }, + ], + 4000, ); - - expect(setCurrentDatabaseItemSpy).toBeCalledTimes(1); - expect(setCurrentDatabaseItemSpy).toBeCalledWith( - postTestDatabaseItem, - true, + expect(failedSpy).toHaveBeenCalledWith( + childItems[2], + [ + { + message: "Test failed", + }, + ], + 11000, ); + expect(failedSpy).toBeCalledTimes(1); + expect(endSpy).toBeCalledTimes(1); }); - - function mockRunTests() { - // runTests is an async generator function. This is not directly supported in sinon - // However, we can pretend the same thing by just returning an async array. - runTestsSpy.mockReturnValue( - (async function* () { - yield Promise.resolve({ - test: Uri.parse("file:/ab/c/d.ql").fsPath, - pass: true, - messages: [], - }); - yield Promise.resolve({ - test: Uri.parse("file:/ab/c/e/f/g.ql").fsPath, - pass: false, - diff: ["pqr", "xyz"], - // a compile error - failureStage: "COMPILATION", - messages: [ - { position: { line: 1 }, message: "abc", severity: "ERROR" }, - ], - }); - yield Promise.resolve({ - test: Uri.parse("file:/ab/c/e/f/h.ql").fsPath, - pass: false, - diff: ["jkh", "tuv"], - failureStage: "RESULT", - messages: [], - }); - })(), - ); - } }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner-helpers.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner-helpers.ts new file mode 100644 index 00000000000..b41d4584123 --- /dev/null +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner-helpers.ts @@ -0,0 +1,96 @@ +import { Uri } from "vscode"; +import { mockedObject } from "../utils/mocking.helpers"; +import { CodeQLCliServer } from "../../../src/cli"; +import { DatabaseManager } from "../../../src/local-databases"; + +/** + * Fake QL tests used by various tests. + */ +export const mockTestsInfo = { + testsPath: Uri.parse("file:/ab/c").fsPath, + dPath: Uri.parse("file:/ab/c/d.ql").fsPath, + gPath: Uri.parse("file:/ab/c/e/f/g.ql").fsPath, + hPath: Uri.parse("file:/ab/c/e/f/h.ql").fsPath, +}; + +/** + * Create a mock of a `DatabaseManager` with no databases loaded. + */ +export function mockEmptyDatabaseManager(): DatabaseManager { + return mockedObject({ + currentDatabaseItem: undefined, + databaseItems: [], + }); +} + +/** + * Creates a `CodeQLCliServer` that "runs" the mock tests. Also returns the spy + * hook for the `runTests` function on the CLI server. + */ +export function createMockCliServerForTestRun() { + const resolveQlpacksSpy = jest.fn(); + resolveQlpacksSpy.mockResolvedValue({}); + + const resolveTestsSpy = jest.fn(); + resolveTestsSpy.mockResolvedValue([]); + + const runTestsSpy = mockRunTests(); + return { + cliServer: mockedObject({ + runTests: runTestsSpy, + resolveQlpacks: resolveQlpacksSpy, + resolveTests: resolveTestsSpy, + }), + runTestsSpy, + }; +} + +function mockRunTests(): jest.Mock { + const runTestsSpy = jest.fn(); + // runTests is an async generator function. This is not directly supported in sinon + // However, we can pretend the same thing by just returning an async array. + runTestsSpy.mockReturnValue( + (async function* () { + yield Promise.resolve({ + test: mockTestsInfo.dPath, + pass: true, + messages: [], + compilationMs: 1000, + evaluationMs: 2000, + }); + yield Promise.resolve({ + test: mockTestsInfo.gPath, + pass: false, + diff: ["pqr", "xyz"], + // a compile error + failureStage: "COMPILATION", + compilationMs: 4000, + evaluationMs: 0, + messages: [ + { + position: { + fileName: mockTestsInfo.gPath, + line: 1, + column: 1, + endLine: 2, + endColumn: 2, + }, + message: "abc", + severity: "ERROR", + }, + ], + }); + yield Promise.resolve({ + test: mockTestsInfo.hPath, + pass: false, + diff: ["jkh", "tuv"], + failureStage: "RESULT", + compilationMs: 5000, + evaluationMs: 6000, + messages: [], + }); + })(), + ); + + return runTestsSpy; +} diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner.test.ts new file mode 100644 index 00000000000..d1cd60a7d62 --- /dev/null +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/test-runner.test.ts @@ -0,0 +1,189 @@ +import { CancellationTokenSource, Uri } from "vscode"; +import { CodeQLCliServer } from "../../../src/cli"; +import { + DatabaseItem, + DatabaseItemImpl, + DatabaseManager, + FullDatabaseOptions, +} from "../../../src/local-databases"; +import { mockedObject } from "../utils/mocking.helpers"; +import { TestRunner } from "../../../src/test-runner"; +import { createMockLogger } from "../../__mocks__/loggerMock"; +import { + createMockCliServerForTestRun, + mockTestsInfo, +} from "./test-runner-helpers"; + +jest.mock("fs-extra", () => { + const original = jest.requireActual("fs-extra"); + return { + ...original, + access: jest.fn(), + }; +}); + +describe("test-runner", () => { + let testRunner: TestRunner; + let fakeDatabaseManager: DatabaseManager; + let fakeCliServer: CodeQLCliServer; + let currentDatabaseItem: DatabaseItem | undefined; + let databaseItems: DatabaseItem[] = []; + const openDatabaseSpy = jest.fn(); + const removeDatabaseItemSpy = jest.fn(); + const renameDatabaseItemSpy = jest.fn(); + const setCurrentDatabaseItemSpy = jest.fn(); + let runTestsSpy: jest.Mock; + const resolveTestsSpy = jest.fn(); + const resolveQlpacksSpy = jest.fn(); + + const preTestDatabaseItem = new DatabaseItemImpl( + Uri.file("/path/to/test/dir/dir.testproj"), + undefined, + mockedObject({ displayName: "custom display name" }), + (_) => { + /* no change event listener */ + }, + ); + const postTestDatabaseItem = new DatabaseItemImpl( + Uri.file("/path/to/test/dir/dir.testproj"), + undefined, + mockedObject({ displayName: "default name" }), + (_) => { + /* no change event listener */ + }, + ); + + beforeEach(() => { + openDatabaseSpy.mockResolvedValue(postTestDatabaseItem); + removeDatabaseItemSpy.mockResolvedValue(undefined); + renameDatabaseItemSpy.mockResolvedValue(undefined); + setCurrentDatabaseItemSpy.mockResolvedValue(undefined); + resolveQlpacksSpy.mockResolvedValue({}); + resolveTestsSpy.mockResolvedValue([]); + fakeDatabaseManager = mockedObject( + { + openDatabase: openDatabaseSpy, + removeDatabaseItem: removeDatabaseItemSpy, + renameDatabaseItem: renameDatabaseItemSpy, + setCurrentDatabaseItem: setCurrentDatabaseItemSpy, + }, + { + dynamicProperties: { + currentDatabaseItem: () => currentDatabaseItem, + databaseItems: () => databaseItems, + }, + }, + ); + + jest.spyOn(preTestDatabaseItem, "isAffectedByTest").mockResolvedValue(true); + + const mockCli = createMockCliServerForTestRun(); + fakeCliServer = mockCli.cliServer; + runTestsSpy = mockCli.runTestsSpy; + + testRunner = new TestRunner(fakeDatabaseManager, fakeCliServer); + }); + + it("should run some tests", async () => { + const eventHandlerSpy = jest.fn(); + + await testRunner.run( + [mockTestsInfo.dPath, mockTestsInfo.gPath, mockTestsInfo.hPath], + createMockLogger(), + new CancellationTokenSource().token, + eventHandlerSpy, + ); + + expect(eventHandlerSpy).toBeCalledTimes(3); + + expect(eventHandlerSpy).toHaveBeenNthCalledWith(1, { + test: mockTestsInfo.dPath, + pass: true, + compilationMs: 1000, + evaluationMs: 2000, + messages: [], + }); + expect(eventHandlerSpy).toHaveBeenNthCalledWith(2, { + test: mockTestsInfo.gPath, + pass: false, + compilationMs: 4000, + evaluationMs: 0, + diff: ["pqr", "xyz"], + failureStage: "COMPILATION", + messages: [ + { + message: "abc", + position: { + line: 1, + column: 1, + endLine: 2, + endColumn: 2, + fileName: mockTestsInfo.gPath, + }, + severity: "ERROR", + }, + ], + }); + expect(eventHandlerSpy).toHaveBeenNthCalledWith(3, { + test: mockTestsInfo.hPath, + pass: false, + compilationMs: 5000, + evaluationMs: 6000, + diff: ["jkh", "tuv"], + failureStage: "RESULT", + messages: [], + }); + }); + + it("should reregister testproj databases around test run", async () => { + currentDatabaseItem = preTestDatabaseItem; + databaseItems = [preTestDatabaseItem]; + await testRunner.run( + ["/path/to/test/dir"], + createMockLogger(), + new CancellationTokenSource().token, + async () => { + /***/ + }, + ); + + expect(removeDatabaseItemSpy.mock.invocationCallOrder[0]).toBeLessThan( + runTestsSpy.mock.invocationCallOrder[0], + ); + expect(openDatabaseSpy.mock.invocationCallOrder[0]).toBeGreaterThan( + runTestsSpy.mock.invocationCallOrder[0], + ); + expect(renameDatabaseItemSpy.mock.invocationCallOrder[0]).toBeGreaterThan( + openDatabaseSpy.mock.invocationCallOrder[0], + ); + expect( + setCurrentDatabaseItemSpy.mock.invocationCallOrder[0], + ).toBeGreaterThan(openDatabaseSpy.mock.invocationCallOrder[0]); + + expect(removeDatabaseItemSpy).toBeCalledTimes(1); + expect(removeDatabaseItemSpy).toBeCalledWith( + expect.anything(), + expect.anything(), + preTestDatabaseItem, + ); + + expect(openDatabaseSpy).toBeCalledTimes(1); + expect(openDatabaseSpy).toBeCalledWith( + expect.anything(), + expect.anything(), + preTestDatabaseItem.databaseUri, + ); + + expect(renameDatabaseItemSpy).toBeCalledTimes(1); + expect(renameDatabaseItemSpy).toBeCalledWith( + postTestDatabaseItem, + preTestDatabaseItem.name, + ); + + expect(setCurrentDatabaseItemSpy).toBeCalledTimes(1); + expect(setCurrentDatabaseItemSpy).toBeCalledWith( + postTestDatabaseItem, + true, + ); + }); +}); From eca8cce6b9b9ff259da5dc5aea5095620faeb776 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 13 Apr 2023 17:03:08 -0400 Subject: [PATCH 6/9] Update extensions/ql-vscode/src/test-manager.ts Co-authored-by: Andrew Eisenberg --- extensions/ql-vscode/src/test-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/test-manager.ts b/extensions/ql-vscode/src/test-manager.ts index e8357746838..db7f04737d2 100644 --- a/extensions/ql-vscode/src/test-manager.ts +++ b/extensions/ql-vscode/src/test-manager.ts @@ -75,7 +75,7 @@ class TestRunLogger implements BaseLogger { } /** - * Handles test dicovery for a specific workspace folder, and reports back to `TestManager`. + * Handles test discovery for a specific workspace folder, and reports back to `TestManager`. */ class WorkspaceFolderHandler extends DisposableObject { private readonly testDiscovery: QLTestDiscovery; From 8f29e1c8124e0192da0c79f3871dfcad4a7493e6 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 13 Apr 2023 17:26:44 -0400 Subject: [PATCH 7/9] Add separate command for "Accept Output" on test item context menu --- extensions/ql-vscode/package.json | 10 +++++++++- extensions/ql-vscode/src/common/commands.ts | 3 +++ extensions/ql-vscode/src/test-manager-base.ts | 6 ++---- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index aa9a8ba2581..a21561dd3fc 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -682,6 +682,10 @@ "command": "codeQLTests.acceptOutput", "title": "Accept Test Output" }, + { + "command": "codeQLTests.acceptOutputContextTestItem", + "title": "Accept Test Output" + }, { "command": "codeQLAstViewer.gotoCode", "title": "Go To Code" @@ -979,7 +983,7 @@ ], "testing/item/context": [ { - "command": "codeQLTests.acceptOutput", + "command": "codeQLTests.acceptOutputContextTestItem", "group": "qltest@1", "when": "controllerId == codeql && testId =~ /^test /" } @@ -1332,6 +1336,10 @@ { "command": "codeQL.createQuery", "when": "config.codeQL.canary" + }, + { + "command": "codeQLTests.acceptOutputContextTestItem", + "when": "false" } ], "editor/context": [ diff --git a/extensions/ql-vscode/src/common/commands.ts b/extensions/ql-vscode/src/common/commands.ts index 47447c0c77d..1ef09cf0341 100644 --- a/extensions/ql-vscode/src/common/commands.ts +++ b/extensions/ql-vscode/src/common/commands.ts @@ -298,6 +298,9 @@ export type SummaryLanguageSupportCommands = { export type TestUICommands = { "codeQLTests.showOutputDifferences": (node: TestTreeNode) => Promise; "codeQLTests.acceptOutput": (node: TestTreeNode) => Promise; + "codeQLTests.acceptOutputContextTestItem": ( + node: TestTreeNode, + ) => Promise; }; export type MockGitHubApiServerCommands = { diff --git a/extensions/ql-vscode/src/test-manager-base.ts b/extensions/ql-vscode/src/test-manager-base.ts index c2a1058c9fb..c611c07de2c 100644 --- a/extensions/ql-vscode/src/test-manager-base.ts +++ b/extensions/ql-vscode/src/test-manager-base.ts @@ -3,7 +3,6 @@ import { TestUICommands } from "./common/commands"; import { DisposableObject } from "./pure/disposable-object"; import { getActualFile, getExpectedFile } from "./test-adapter"; import { TestItem, TextDocumentShowOptions, Uri, window } from "vscode"; -import { showAndLogWarningMessage } from "./helpers"; import { basename } from "path"; import { App } from "./common/app"; import { TestTreeNode } from "./test-tree-node"; @@ -24,6 +23,7 @@ export abstract class TestManagerBase extends DisposableObject { "codeQLTests.showOutputDifferences": this.showOutputDifferences.bind(this), "codeQLTests.acceptOutput": this.acceptOutput.bind(this), + "codeQLTests.acceptOutputContextTestItem": this.acceptOutput.bind(this), }; } @@ -53,9 +53,7 @@ export abstract class TestManagerBase extends DisposableObject { }; if (!(await pathExists(expectedPath))) { - void showAndLogWarningMessage( - `'${basename(expectedPath)}' does not exist. Creating an empty file.`, - ); + // Just create a new file. await createFile(expectedPath); } From da3f482a97d320b0414efb89f91f7e1bc92e663d Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Thu, 13 Apr 2023 18:03:07 -0400 Subject: [PATCH 8/9] Reconsider the fanciness of CodeQL tests --- extensions/ql-vscode/src/test-manager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/test-manager.ts b/extensions/ql-vscode/src/test-manager.ts index db7f04737d2..ce2ed5c11ba 100644 --- a/extensions/ql-vscode/src/test-manager.ts +++ b/extensions/ql-vscode/src/test-manager.ts @@ -125,7 +125,7 @@ export class TestManager extends TestManagerBase { // Having this as a parameter with a default value makes passing in a mock easier. private readonly testController: TestController = tests.createTestController( "codeql", - "Fancy CodeQL Tests", + "CodeQL Tests", ), ) { super(app); From 8c2c25e85f181bee055f4be9e08cb6a03de9def8 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 14 Apr 2023 15:24:08 -0400 Subject: [PATCH 9/9] Fix PR feedback --- extensions/ql-vscode/src/cli.ts | 13 +++++++------ extensions/ql-vscode/src/test-adapter.ts | 4 +++- extensions/ql-vscode/src/test-manager.ts | 8 ++++---- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 6a42a13e82c..2d19cc8303c 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -448,6 +448,11 @@ export class CodeQLCliServer implements Disposable { // Spawn the CodeQL process const codeqlPath = await this.getCodeQlPath(); const childPromise = spawn(codeqlPath, args); + // Avoid a runtime message about unhandled rejection. + childPromise.catch(() => { + /**/ + }); + const child = childPromise.childProcess; let cancellationRegistration: Disposable | undefined = undefined; @@ -469,13 +474,9 @@ export class CodeQLCliServer implements Disposable { ])) { yield event; } + + await childPromise; } finally { - try { - await childPromise; - } catch (_e) { - // We need to await this to avoid an unhandled rejection, but we want to propagate the - // original exception. - } if (cancellationRegistration !== undefined) { cancellationRegistration.dispose(); } diff --git a/extensions/ql-vscode/src/test-adapter.ts b/extensions/ql-vscode/src/test-adapter.ts index 38a6e383726..14bfd8fac5d 100644 --- a/extensions/ql-vscode/src/test-adapter.ts +++ b/extensions/ql-vscode/src/test-adapter.ts @@ -261,7 +261,9 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter { ].join("\n") : [ "", - `${event.failureStage?.toLowerCase()} error: ${event.test}`, + `${event.failureStage?.toLowerCase() ?? "unknown stage"} error: ${ + event.test + }`, event.failureDescription || `${event.messages[0].severity}: ${event.messages[0].message}`, "", diff --git a/extensions/ql-vscode/src/test-manager.ts b/extensions/ql-vscode/src/test-manager.ts index ce2ed5c11ba..17933fa8222 100644 --- a/extensions/ql-vscode/src/test-manager.ts +++ b/extensions/ql-vscode/src/test-manager.ts @@ -155,7 +155,7 @@ export class TestManager extends TestManagerBase { if (node.uri === undefined || node.uri.scheme !== "file") { throw new Error("Selected test is not a CodeQL test."); } - return node.uri!.fsPath; + return node.uri.fsPath; } /** Start tracking tests in the specified workspace folders. */ @@ -256,9 +256,9 @@ export class TestManager extends TestManagerBase { const testRun = this.testController.createTestRun(request, undefined, true); try { const tests: string[] = []; - testsToRun.forEach((t) => { - testRun.enqueued(t); - tests.push(t.uri!.fsPath); + testsToRun.forEach((testItem, testPath) => { + testRun.enqueued(testItem); + tests.push(testPath); }); const logger = new TestRunLogger(testRun);