Add pack download to its own log group

Also, make the baseline count message less awkward sounding.

.github/workflows/pr-checks.yml

@@ -136,7 +136,7 @@ jobs:
         EXPECTED_RULES="javascript/example/empty-or-one-block javascript/example/empty-or-one-block javascript/example/two-block"
 
         # use tr to replace newlines with spaces and xargs to trim leading and trailing whitespace
-        RULES="$(cat javascript-custom.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
+        RULES="$(cat javascript.sarif | jq -r '.runs[0].results[].ruleId' | sort | tr "\n" " " | xargs)"
         echo "Found matching rules '$RULES'"
         if [ "$RULES" != "$EXPECTED_RULES" ]; then
           echo "Did not match expected rules '$EXPECTED_RULES'."

lib/analyze.js

@@ -85,7 +85,6 @@ async function runQueries(sarifFolder, memoryFlag, addSnippetsFlag, threadsFlag,
     // that here.
     config.paths, config.pathsIgnore, config.languages, logger);
     for (const language of config.languages) {
-        logger.startGroup(`Running queries for ${language}`);
         const queries = config.queries[language];
         const packsWithVersion = config.packs[language] || [];
         const hasBuiltinQueries = ((_a = queries) === null || _a === void 0 ? void 0 : _a.builtin.length) > 0;
@@ -96,12 +95,15 @@ async function runQueries(sarifFolder, memoryFlag, addSnippetsFlag, threadsFlag,
         }
         try {
             if (hasPackWithCustomQueries) {
+                logger.startGroup(`Downloading custom packs for ${language}`);
                 const codeql = codeql_1.getCodeQL(config.codeQLCmd);
                 const results = await codeql.packDownload(packsWithVersion);
                 logger.info(`Downloaded packs: ${results.packs
                     .map((r) => `${r.name}@${r.version || "latest"}`)
                     .join(", ")}`);
+                logger.endGroup();
             }
+            logger.startGroup(`Running queries for ${language}`);
             const querySuitePaths = [];
             if (queries["builtin"].length > 0) {
                 const startTimeBuiltIn = new Date().getTime();
@@ -222,7 +224,7 @@ async function injectLinesOfCode(sarifFile, language, locPromise) {
 }
 function printLinesOfCodeSummary(logger, language, lineCounts) {
     if (language in lineCounts) {
-        logger.info(`Counted ${lineCounts[language]} lines of code for ${language} as a baseline.`);
+        logger.info(`Counted a baseline of ${lineCounts[language]} lines of code for ${language}.`);
     }
 }
 //# sourceMappingURL=analyze.js.map

lib/analyze.test.js

@@ -119,9 +119,17 @@ ava_1.default("status report fields and search path setting", async (t) => {
             };
             const builtinStatusReport = await analyze_1.runQueries(tmpDir, memoryFlag, addSnippetsFlag, threadsFlag, undefined, config, logging_1.getRunnerLogger(true));
             const hasPacks = language in packs;
-            t.deepEqual(Object.keys(builtinStatusReport).length, hasPacks ? 2 : 1);
+            const statusReportKeys = Object.keys(builtinStatusReport).sort();
-            t.true(`analyze_builtin_queries_${language}_duration_ms` in builtinStatusReport);
+            if (hasPacks) {
-            t.true(`interpret_results_${language}_duration_ms` in builtinStatusReport);
+                t.deepEqual(statusReportKeys.length, 3, statusReportKeys.toString());
+                t.deepEqual(statusReportKeys[0], `analyze_builtin_queries_${language}_duration_ms`);
+                t.deepEqual(statusReportKeys[1], `analyze_custom_queries_${language}_duration_ms`);
+                t.deepEqual(statusReportKeys[2], `interpret_results_${language}_duration_ms`);
+            }
+            else {
+                t.deepEqual(statusReportKeys[0], `analyze_builtin_queries_${language}_duration_ms`);
+                t.deepEqual(statusReportKeys[1], `interpret_results_${language}_duration_ms`);
+            }
             config.queries[language] = {
                 builtin: [],
                 custom: [

lib/config-utils.test.js

@@ -763,7 +763,7 @@ function invalidPackNameMacro(t, name) {
     parsePacksErrorMacro(t, { [languages_1.Language.cpp]: [name] }, [languages_1.Language.cpp], new RegExp(`The configuration file "/a/b" is invalid: property "packs" "${name}" is not a valid pack`));
 }
 invalidPackNameMacro.title = (_, arg) => `Invalid pack string: ${arg}`;
-ava_1.default("no packs", parsePacksMacro, undefined, [], {});
+ava_1.default("no packs", parsePacksMacro, {}, [], {});
 ava_1.default("two packs", parsePacksMacro, ["a/b", "c/d@1.2.3"], [languages_1.Language.cpp], {
     [languages_1.Language.cpp]: [
         { packName: "a/b", version: undefined },

src/analyze.test.ts

@@ -136,13 +136,31 @@ test("status report fields and search path setting", async (t) => {
         getRunnerLogger(true)
       );
       const hasPacks = language in packs;
-      t.deepEqual(Object.keys(builtinStatusReport).length, hasPacks ? 2 : 1);
+      const statusReportKeys = Object.keys(builtinStatusReport).sort();
-      t.true(
+      if (hasPacks) {
-        `analyze_builtin_queries_${language}_duration_ms` in builtinStatusReport
+        t.deepEqual(statusReportKeys.length, 3, statusReportKeys.toString());
-      );
+        t.deepEqual(
-      t.true(
+          statusReportKeys[0],
-        `interpret_results_${language}_duration_ms` in builtinStatusReport
+          `analyze_builtin_queries_${language}_duration_ms`
-      );
+        );
+        t.deepEqual(
+          statusReportKeys[1],
+          `analyze_custom_queries_${language}_duration_ms`
+        );
+        t.deepEqual(
+          statusReportKeys[2],
+          `interpret_results_${language}_duration_ms`
+        );
+      } else {
+        t.deepEqual(
+          statusReportKeys[0],
+          `analyze_builtin_queries_${language}_duration_ms`
+        );
+        t.deepEqual(
+          statusReportKeys[1],
+          `interpret_results_${language}_duration_ms`
+        );
+      }
 
       config.queries[language] = {
         builtin: [],

src/analyze.ts

@@ -176,14 +176,13 @@ export async function runQueries(
   );
 
   for (const language of config.languages) {
-    logger.startGroup(`Running queries for ${language}`);
-
     const queries = config.queries[language];
     const packsWithVersion = config.packs[language] || [];
 
     const hasBuiltinQueries = queries?.builtin.length > 0;
     const hasCustomQueries = queries?.custom.length > 0;
     const hasPackWithCustomQueries = packsWithVersion.length > 0;
+
     if (!hasBuiltinQueries && !hasCustomQueries && !hasPackWithCustomQueries) {
       throw new Error(
         `Unable to analyse ${language} as no queries were selected for this language`
@@ -192,6 +191,8 @@ export async function runQueries(
 
     try {
       if (hasPackWithCustomQueries) {
+        logger.startGroup(`Downloading custom packs for ${language}`);
+
         const codeql = getCodeQL(config.codeQLCmd);
         const results = await codeql.packDownload(packsWithVersion);
         logger.info(
@@ -199,8 +200,11 @@ export async function runQueries(
             .map((r) => `${r.name}@${r.version || "latest"}`)
             .join(", ")}`
         );
+
+        logger.endGroup();
       }
 
+      logger.startGroup(`Running queries for ${language}`);
       const querySuitePaths: string[] = [];
       if (queries["builtin"].length > 0) {
         const startTimeBuiltIn = new Date().getTime();
@@ -419,7 +423,7 @@ function printLinesOfCodeSummary(
 ) {
   if (language in lineCounts) {
     logger.info(
-      `Counted ${lineCounts[language]} lines of code for ${language} as a baseline.`
+      `Counted a baseline of ${lineCounts[language]} lines of code for ${language}.`
     );
   }
 }

src/config-utils.test.ts

@@ -1316,7 +1316,7 @@ test("path sanitisation", (t) => {
  */
 function parsePacksMacro(
   t: ExecutionContext<unknown>,
-  packsByLanguage,
+  packsByLanguage: string[] | Record<string, string[]>,
   languages: Language[],
   expected
 ) {
@@ -1362,7 +1362,7 @@ function invalidPackNameMacro(t: ExecutionContext<unknown>, name: string) {
 invalidPackNameMacro.title = (_: string, arg: string) =>
   `Invalid pack string: ${arg}`;
 
-test("no packs", parsePacksMacro, undefined, [], {});
+test("no packs", parsePacksMacro, {}, [], {});
 test("two packs", parsePacksMacro, ["a/b", "c/d@1.2.3"], [Language.cpp], {
   [Language.cpp]: [
     { packName: "a/b", version: undefined },