Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions extensions/ql-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -977,6 +981,13 @@
"when": "viewItem == testWithSource"
}
],
"testing/item/context": [
{
"command": "codeQLTests.acceptOutputContextTestItem",
"group": "qltest@1",
"when": "controllerId == codeql && testId =~ /^test /"
}
],
"explorer/context": [
{
"command": "codeQL.setCurrentDatabase",
Expand Down Expand Up @@ -1325,6 +1336,10 @@
{
"command": "codeQL.createQuery",
"when": "config.codeQL.canary"
},
{
"command": "codeQLTests.acceptOutputContextTestItem",
"when": "false"
}
],
"editor/context": [
Expand Down
16 changes: 11 additions & 5 deletions extensions/ql-vscode/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -149,6 +149,7 @@ export interface TestCompleted {
compilationMs: number;
evaluationMs: number;
expected: string;
actual?: string;
diff: string[] | undefined;
failureDescription?: string;
failureStage?: string;
Expand Down Expand Up @@ -439,14 +440,19 @@ export class CodeQLCliServer implements Disposable {
command: string[],
commandArgs: string[],
cancellationToken?: CancellationToken,
logger?: Logger,
logger?: BaseLogger,
): AsyncGenerator<string, void, unknown> {
// Add format argument first, in case commandArgs contains positional parameters.
const args = [...command, "--format", "jsonz", ...commandArgs];

// 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;
Expand Down Expand Up @@ -497,7 +503,7 @@ export class CodeQLCliServer implements Disposable {
logger,
}: {
cancellationToken?: CancellationToken;
logger?: Logger;
logger?: BaseLogger;
} = {},
): AsyncGenerator<EventType, void, unknown> {
for await (const event of this.runAsyncCodeQlCliCommandInternal(
Expand Down Expand Up @@ -776,7 +782,7 @@ export class CodeQLCliServer implements Disposable {
logger,
}: {
cancellationToken?: CancellationToken;
logger?: Logger;
logger?: BaseLogger;
},
): AsyncGenerator<TestCompleted, void, unknown> {
const subcommandArgs = this.cliConfig.additionalTestArguments.concat([
Expand Down Expand Up @@ -1661,7 +1667,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<void> {
async function logStream(stream: Readable, logger: BaseLogger): Promise<void> {
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);
Expand Down
3 changes: 3 additions & 0 deletions extensions/ql-vscode/src/common/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,9 @@ export type SummaryLanguageSupportCommands = {
export type TestUICommands = {
"codeQLTests.showOutputDifferences": (node: TestTreeNode) => Promise<void>;
"codeQLTests.acceptOutput": (node: TestTreeNode) => Promise<void>;
"codeQLTests.acceptOutputContextTestItem": (
node: TestTreeNode,
) => Promise<void>;
};

export type MockGitHubApiServerCommands = {
Expand Down
44 changes: 28 additions & 16 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { CodeQLCliServer } from "./cli";
import {
CliConfigListener,
DistributionConfigListener,
isCanary,
joinOrderWarningThreshold,
QueryHistoryConfigListener,
QueryServerConfigListener,
Expand Down Expand Up @@ -114,14 +115,16 @@ import {
BaseCommands,
PreActivationCommands,
QueryServerCommands,
TestUICommands,
} from "./common/commands";
import { LocalQueries } from "./local-queries";
import { getAstCfgCommands } from "./ast-cfg-commands";
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
Expand Down Expand Up @@ -879,25 +882,34 @@ async function activateWithInstalledDistribution(
);

void extLogger.log("Initializing QLTest interface.");
const testExplorerExtension = extensions.getExtension<TestHub>(
testExplorerExtensionId,
);
let testUiCommands: Partial<TestUICommands> = {};
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()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As much as possible, we try to dis/enable all canary features without a restart. It looks like this is not following the pattern. I don't know how easy it would be to do, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. There's no way to "unregister" a TestController AFAICT, but maybe just deleting all of its TestItems would be equivalent. I'll have to see if there's a way to unregister the legacy test adapter, but it might not be worth the effort.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, there is a way to unregister the legacy test stuff. However, handling of the commands we expose is trickier. We share the same command IDs between the two implementations, but they route to slightly different implementations depending on which test UI we're using.

Given how easy the transition to the new UI was, and how annoying our dependency on that third-party extension is, I suspect we'll move this out of canary pretty soon anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's going to take significant time and still be imperfect, I don't think it's worth taking on for a transitional feature.

testManager = new TestManager(app, testRunner, cliServer);
ctx.subscriptions.push(testManager);
} else {
const testExplorerExtension = extensions.getExtension<TestHub>(
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,
Expand Down
Loading