Don't log the resolve queries CLI command (#2454)

This commit is contained in:
Shati Patel
2023-06-01 09:49:35 +01:00
committed by GitHub
parent bc342022cb
commit 5387546e93
3 changed files with 61 additions and 11 deletions

View File

@@ -218,7 +218,7 @@ export class CodeQLCliServer implements Disposable {
private readonly app: App,
private distributionProvider: DistributionProvider,
private cliConfig: CliConfig,
private logger: Logger,
public readonly logger: Logger,
) {
this.commandQueue = [];
this.commandInProcess = false;
@@ -330,6 +330,7 @@ export class CodeQLCliServer implements Disposable {
commandArgs: string[],
description: string,
onLine?: OnLineCallback,
silent?: boolean,
): Promise<string> {
const stderrBuffers: Buffer[] = [];
if (this.commandInProcess) {
@@ -349,7 +350,12 @@ export class CodeQLCliServer implements Disposable {
// Compute the full args array
const args = command.concat(LOGGING_FLAGS).concat(commandArgs);
const argsString = args.join(" ");
void this.logger.log(`${description} using CodeQL CLI: ${argsString}...`);
// If we are running silently, we don't want to print anything to the console.
if (!silent) {
void this.logger.log(
`${description} using CodeQL CLI: ${argsString}...`,
);
}
try {
await new Promise<void>((resolve, reject) => {
// Start listening to stdout
@@ -395,24 +401,30 @@ export class CodeQLCliServer implements Disposable {
const fullBuffer = Buffer.concat(stdoutBuffers);
// Make sure we remove the terminator;
const data = fullBuffer.toString("utf8", 0, fullBuffer.length - 1);
if (!silent) {
void this.logger.log("CLI command succeeded.");
}
return data;
} catch (err) {
// Kill the process if it isn't already dead.
this.killProcessIfRunning();
// Report the error (if there is a stderr then use that otherwise just report the error cod or nodejs error)
// Report the error (if there is a stderr then use that otherwise just report the error code or nodejs error)
const newError =
stderrBuffers.length === 0
? new Error(`${description} failed: ${err}`)
? new Error(
`${description} failed with args:${EOL} ${argsString}${EOL}${err}`,
)
: new Error(
`${description} failed: ${Buffer.concat(stderrBuffers).toString(
"utf8",
)}`,
`${description} failed with args:${EOL} ${argsString}${EOL}${Buffer.concat(
stderrBuffers,
).toString("utf8")}`,
);
newError.stack += getErrorStack(err);
throw newError;
} finally {
if (!silent) {
void this.logger.log(Buffer.concat(stderrBuffers).toString("utf8"));
}
// Remove the listeners we set up.
process.stdout.removeAllListeners("data");
process.stderr.removeAllListeners("data");
@@ -549,9 +561,11 @@ export class CodeQLCliServer implements Disposable {
{
progressReporter,
onLine,
silent = false,
}: {
progressReporter?: ProgressReporter;
onLine?: OnLineCallback;
silent?: boolean;
} = {},
): Promise<string> {
if (progressReporter) {
@@ -567,6 +581,7 @@ export class CodeQLCliServer implements Disposable {
commandArgs,
description,
onLine,
silent,
).then(resolve, reject);
} catch (err) {
reject(err);
@@ -600,10 +615,12 @@ export class CodeQLCliServer implements Disposable {
addFormat = true,
progressReporter,
onLine,
silent = false,
}: {
addFormat?: boolean;
progressReporter?: ProgressReporter;
onLine?: OnLineCallback;
silent?: boolean;
} = {},
): Promise<OutputType> {
let args: string[] = [];
@@ -614,6 +631,7 @@ export class CodeQLCliServer implements Disposable {
const result = await this.runCodeQlCliCommand(command, args, description, {
progressReporter,
onLine,
silent,
});
try {
return JSON.parse(result) as OutputType;
@@ -739,14 +757,19 @@ export class CodeQLCliServer implements Disposable {
/**
* Finds all available queries in a given directory.
* @param queryDir Root of directory tree to search for queries.
* @param silent If true, don't print logs to the CodeQL extension log.
* @returns The list of queries that were found.
*/
public async resolveQueries(queryDir: string): Promise<ResolvedQueries> {
public async resolveQueries(
queryDir: string,
silent?: boolean,
): Promise<ResolvedQueries> {
const subcommandArgs = [queryDir];
return await this.runJsonCodeQlCliCommand<ResolvedQueries>(
["resolve", "queries"],
subcommandArgs,
"Resolving queries",
{ silent },
);
}

View File

@@ -114,7 +114,12 @@ export class QueryDiscovery
const fullPath = workspaceFolder.uri.fsPath;
const name = workspaceFolder.name;
const resolvedQueries = await this.cliServer.resolveQueries(fullPath);
// We don't want to log each invocation of resolveQueries, since it clutters up the log.
const silent = true;
const resolvedQueries = await this.cliServer.resolveQueries(
fullPath,
silent,
);
if (resolvedQueries.length === 0) {
return undefined;
}

View File

@@ -15,6 +15,7 @@ import {
import { KeyType, resolveQueries } from "../../../src/language-support";
import { faker } from "@faker-js/faker";
import { getActivatedExtension } from "../global.helper";
import { BaseLogger } from "../../../src/common";
/**
* Perform proper integration tests by running the CLI
@@ -23,10 +24,14 @@ describe("Use cli", () => {
let cli: CodeQLCliServer;
let supportedLanguages: string[];
let logSpy: jest.SpiedFunction<BaseLogger["log"]>;
beforeEach(async () => {
const extension = await getActivatedExtension();
cli = extension.cliServer;
supportedLanguages = await cli.getSupportedLanguages();
logSpy = jest.spyOn(cli.logger, "log");
});
if (process.env.CLI_VERSION && process.env.CLI_VERSION !== "nightly") {
@@ -42,6 +47,23 @@ describe("Use cli", () => {
expect(result).toEqual(["-J-Xmx4096M", "--off-heap-ram=4096"]);
});
describe("silent logging", () => {
it("should log command output", async () => {
const queryDir = getOnDiskWorkspaceFolders()[0];
await cli.resolveQueries(queryDir);
expect(logSpy).toHaveBeenCalled();
});
it("shouldn't log command output if the `silent` flag is set", async () => {
const queryDir = getOnDiskWorkspaceFolders()[0];
const silent = true;
await cli.resolveQueries(queryDir, silent);
expect(logSpy).not.toHaveBeenCalled();
});
});
itWithCodeQL()("should resolve query packs", async () => {
const qlpacks = await cli.resolveQlpacks(getOnDiskWorkspaceFolders());
// Depending on the version of the CLI, the qlpacks may have different names