Merge pull request #120 from github/default_queries_fix

Default queries fix + reset env vars in tests

src/config-utils.test.ts

@@ -191,6 +191,7 @@ test("load non-empty input", async t => {
 
     fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
     setInput('config-file', 'input');
+    setInput('languages', 'javascript');
 
     const actualConfig = await configUtils.initConfig();
 
@@ -199,6 +200,53 @@ test("load non-empty input", async t => {
   });
 });
 
+test("default queries are used", async t => {
+  return await util.withTmpDir(async tmpDir => {
+    process.env['RUNNER_TEMP'] = tmpDir;
+    process.env['GITHUB_WORKSPACE'] = tmpDir;
+
+    // Check that the default behaviour is to add the default queries.
+    // In this case if a config file is specified but does not include
+    // the disable-default-queries field.
+    // We determine this by whether CodeQL.resolveQueries is called
+    // with the correct arguments.
+
+    const resolveQueriesArgs: {queries: string[], extraSearchPath: string | undefined}[] = [];
+    CodeQL.setCodeQL({
+      resolveQueries: async function(queries: string[], extraSearchPath: string | undefined) {
+        resolveQueriesArgs.push({queries, extraSearchPath});
+        return {
+          byLanguage: {
+            'javascript': {},
+          },
+          noDeclaredLanguage: {},
+          multipleDeclaredLanguages: {},
+        };
+      },
+    });
+
+    // The important point of this config is that it doesn't specify
+    // the disable-default-queries field.
+    // Any other details are hopefully irrelevant for this tetst.
+    const inputFileContents = `
+      paths:
+        - foo`;
+
+    fs.mkdirSync(path.join(tmpDir, 'foo'));
+
+    fs.writeFileSync(path.join(tmpDir, 'input'), inputFileContents, 'utf8');
+    setInput('config-file', 'input');
+    setInput('languages', 'javascript');
+
+    await configUtils.initConfig();
+
+    // Check resolve queries was called correctly
+    t.deepEqual(resolveQueriesArgs.length, 1);
+    t.deepEqual(resolveQueriesArgs[0].queries, ['javascript-code-scanning.qls']);
+    t.deepEqual(resolveQueriesArgs[0].extraSearchPath, undefined);
+  });
+});
+
 test("API client used when reading remote config", async t => {
   return await util.withTmpDir(async tmpDir => {
     process.env['RUNNER_TEMP'] = tmpDir;
@@ -235,6 +283,8 @@ test("API client used when reading remote config", async t => {
     fs.mkdirSync(path.join(tmpDir, 'foo/bar'), { recursive: true });
 
     setInput('config-file', 'octo-org/codeql-config/config.yaml@main');
+    setInput('languages', 'javascript');
+
     await configUtils.initConfig();
     t.assert(spyGetContents.called);
   });
@@ -290,9 +340,20 @@ function doInvalidInputTest(
       process.env['RUNNER_TEMP'] = tmpDir;
       process.env['GITHUB_WORKSPACE'] = tmpDir;
 
+      CodeQL.setCodeQL({
+        resolveQueries: async function() {
+          return {
+            byLanguage: {},
+            noDeclaredLanguage: {},
+            multipleDeclaredLanguages: {},
+          };
+        },
+      });
+
       const inputFile = path.join(tmpDir, 'input');
       fs.writeFileSync(inputFile, inputFileContents, 'utf8');
       setInput('config-file', 'input');
+      setInput('languages', 'javascript');
 
       try {
         await configUtils.initConfig();

src/config-utils.ts

@@ -502,13 +502,15 @@ async function loadConfig(configFile: string): Promise<Config> {
   const pathsIgnore: string[] = [];
   const paths: string[] = [];
 
+  let disableDefaultQueries = false;
   if (DISABLE_DEFAULT_QUERIES_PROPERTY in parsedYAML) {
     if (typeof parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY] !== "boolean") {
       throw new Error(getDisableDefaultQueriesInvalid(configFile));
     }
-    if (!parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY]) {
-      await addDefaultQueries(languages, queries);
-    }
+    disableDefaultQueries = parsedYAML[DISABLE_DEFAULT_QUERIES_PROPERTY];
+  }
+  if (!disableDefaultQueries) {
+    await addDefaultQueries(languages, queries);
   }
 
   if (QUERIES_PROPERTY in parsedYAML) {

src/testing-utils.ts

@@ -3,7 +3,7 @@ import sinon from 'sinon';
 
 import * as CodeQL from './codeql';
 
-type TestContext = {stdoutWrite: any, stderrWrite: any, testOutput: string};
+type TestContext = {stdoutWrite: any, stderrWrite: any, testOutput: string, env: NodeJS.ProcessEnv};
 
 function wrapOutput(context: TestContext) {
   // Function signature taken from Socket.write.
@@ -49,6 +49,12 @@ export function setupTests(test: TestInterface<any>) {
     const processStderrWrite = process.stderr.write.bind(process.stderr);
     t.context.stderrWrite = processStderrWrite;
     process.stderr.write = wrapOutput(t.context) as any;
+
+    // Many tests modify environment variables. Take a copy now so that
+    // we reset them after the test to keep tests independent of each other.
+    // process.env only has strings fields, so a shallow copy is fine.
+    t.context.env = {};
+    Object.assign(t.context.env, process.env);
   });
 
   typedTest.afterEach.always(t => {
@@ -62,5 +68,8 @@ export function setupTests(test: TestInterface<any>) {
 
     // Undo any modifications made by sinon
     sinon.restore();
+
+    // Undo any modifications to the env
+    process.env = t.context.env;
   });
 }