Skip to content

Commit 9769e4a

Browse files
robertbrignullrobertbrignull
authored
Merge pull request #120 from github/default_queries_fix
Default queries fix + reset env vars in tests
2 parents 58a0034 + 315a9f4 commit 9769e4a

File tree

9 files changed

+142
-10
lines changed

9 files changed

+142
-10
lines changed

‎lib/config-utils.js‎

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/config-utils.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/config-utils.test.js‎

Lines changed: 51 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/config-utils.test.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/testing-utils.js‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/testing-utils.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/config-utils.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ test("load non-empty input", async t => {
191191

192192
fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
193193
setInput('config-file', 'input');
194+
setInput('languages', 'javascript');
194195

195196
const actualConfig = await configUtils.initConfig();
196197

@@ -199,6 +200,53 @@ test("load non-empty input", async t => {
199200
});
200201
});
201202

203+
test("default queries are used", async t => {
204+
return await util.withTmpDir(async tmpDir => {
205+
process.env['RUNNER_TEMP'] = tmpDir;
206+
process.env['GITHUB_WORKSPACE'] = tmpDir;
207+
208+
// Check that the default behaviour is to add the default queries.
209+
// In this case if a config file is specified but does not include
210+
// the disable-default-queries field.
211+
// We determine this by whether CodeQL.resolveQueries is called
212+
// with the correct arguments.
213+
214+
const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
215+
CodeQL.setCodeQL({
216+
resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
217+
resolveQueriesArgs.push({queries, extraSearchPath});
218+
return {
219+
byLanguage: {
220+
'javascript': {},
221+
},
222+
noDeclaredLanguage: {},
223+
multipleDeclaredLanguages: {},
224+
};
225+
},
226+
});
227+
228+
// The important point of this config is that it doesn't specify
229+
// the disable-default-queries field.
230+
// Any other details are hopefully irrelevant for this tetst.
231+
const inputFileContents = `
232+
paths:
233+
- foo`;
234+
235+
fs.mkdirSync(path.join(tmpDir, 'foo'));
236+
237+
fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
238+
setInput('config-file', 'input');
239+
setInput('languages', 'javascript');
240+
241+
await configUtils.initConfig();
242+
243+
// Check resolve queries was called correctly
244+
t.deepEqual(resolveQueriesArgs.length, 1);
245+
t.deepEqual(resolveQueriesArgs[0].queries, ['javascript-code-scanning.qls']);
246+
t.deepEqual(resolveQueriesArgs[0].extraSearchPath, undefined);
247+
});
248+
});
249+
202250
test("API client used when reading remote config", async t => {
203251
return await util.withTmpDir(async tmpDir => {
204252
process.env['RUNNER_TEMP'] = tmpDir;
@@ -235,6 +283,8 @@ test("API client used when reading remote config", async t => {
235283
fs.mkdirSync(path.join(tmpDir, 'foo/bar'), { recursive: true });
236284

237285
setInput('config-file', 'octo-org/codeql-config/config.yaml@main');
286+
setInput('languages', 'javascript');
287+
238288
await configUtils.initConfig();
239289
t.assert(spyGetContents.called);
240290
});
@@ -290,9 +340,20 @@ function doInvalidInputTest(
290340
process.env['RUNNER_TEMP'] = tmpDir;
291341
process.env['GITHUB_WORKSPACE'] = tmpDir;
292342

343+
CodeQL.setCodeQL({
344+
resolveQueries: async function() {
345+
return {
346+
byLanguage: {},
347+
noDeclaredLanguage: {},
348+
multipleDeclaredLanguages: {},
349+
};
350+
},
351+
});
352+
293353
const inputFile = path.join(tmpDir, 'input');
294354
fs.writeFileSync(inputFile, inputFileContents, 'utf8');
295355
setInput('config-file', 'input');
356+
setInput('languages', 'javascript');
296357

297358
try {
298359
await configUtils.initConfig();

‎src/config-utils.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,13 +502,15 @@ async function loadConfig(configFile: string): Promise<Config> {
502502
const pathsIgnore: string[] = [];
503503
const paths: string[] = [];
504504

505+
let disableDefaultQueries = false;
505506
if (DISABLE_DEFAULT_QUERIES_PROPERTY in parsedYAML) {
506507
if (typeof parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY] !== "boolean") {
507508
throw new Error(getDisableDefaultQueriesInvalid(configFile));
508509
}
509-
if (!parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY]) {
510-
await addDefaultQueries(languages, queries);
511-
}
510+
disableDefaultQueries = parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY];
511+
}
512+
if (!disableDefaultQueries) {
513+
await addDefaultQueries(languages, queries);
512514
}
513515

514516
if (QUERIES_PROPERTY in parsedYAML) {

‎src/testing-utils.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import sinon from 'sinon';
33

44
import * as CodeQL from './codeql';
55

6-
type TestContext = {stdoutWrite: any, stderrWrite: any, testOutput: string};
6+
type TestContext = {stdoutWrite: any, stderrWrite: any, testOutput: string, env: NodeJS.ProcessEnv};
77

88
function wrapOutput(context: TestContext) {
99
// Function signature taken from Socket.write.
@@ -49,6 +49,12 @@ export function setupTests(test: TestInterface<any>) {
4949
const processStderrWrite = process.stderr.write.bind(process.stderr);
5050
t.context.stderrWrite = processStderrWrite;
5151
process.stderr.write = wrapOutput(t.context) as any;
52+
53+
// Many tests modify environment variables. Take a copy now so that
54+
// we reset them after the test to keep tests independent of each other.
55+
// process.env only has strings fields, so a shallow copy is fine.
56+
t.context.env = {};
57+
Object.assign(t.context.env, process.env);
5258
});
5359

5460
typedTest.afterEach.always(t => {
@@ -62,5 +68,8 @@ export function setupTests(test: TestInterface<any>) {
6268

6369
// Undo any modifications made by sinon
6470
sinon.restore();
71+
72+
// Undo any modifications to the env
73+
process.env = t.context.env;
6574
});
6675
}

0 commit comments

Comments
 (0)