Merge pull request #4219 from github/mbg/ci/improve-pr-checks-triggers

Improve `pr-checks.yml` triggers and conditions

.github/copilot-instructions.md

@@ -11,9 +11,12 @@ The CodeQL Action is used in GitHub Actions workflows to run CodeQL scans using
 
 ## Review instructions
 
-- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Word your comments constructively as suggestions for improvements. Do not word suggestions as commands.
+- Prioritise identifying critical issues, such as logic bugs, over comments on documentation or typos.
+- When wording review comments, be helpful and friendly. Assume that the PR author has written the code with the best of intentions. Never word suggestions as commands or using the imperative. Examples of what you should never write: "Preserve that non-fatal behaviour here" or "Please preserve that non-fatal behaviour here".
+- When writing feedback, assume that the PR author is competent and is able to understand whether the point you are making is valid or not, why you are raising it, and how to best address it. While you can suggest how to address a point you are making, don't phrase it as a requirement or assume that it is the only solution.
+- Always word your comments constructively as suggestions for improvements, for example: "It would make sense to ..." or "Could you add ...?" or "Would it make sense to ...?"
 - If you want to comment on a change that you believe will fail a CI check, do not present the CI failure you expect as a fact. Instead, write that you think a change "may" lead to a failure in CI. Suggest that, if such a failure manifests, the changes you are commenting on may be the place responsible for the failure and are worth looking at.
-- If a suggestion you make is suitable for a follow-up, such as a refactoring that doesn't change the behaviour or fixing a typo in a comment, mention that it can be addressed in a later PR rather than blocking this one.
+- If a suggestion you make is suitable for a follow-up, such as a refactoring that doesn't change the behaviour or fixing a typo in a comment, mention that it can be addressed in a later PR rather than blocking this one or prefix your comment with "Minor: ".
 - If a change is a net improvement, for example because it improves on an existing limitation of existing code, do not complain about pre-existing problems that remain. You may comment on them, but you should make it clear that the thing you are commenting on is not new by writing e.g. "Not new in this PR, but [..]" followed by your description of the issue and a suggestion that it could be improved at the same time with e.g. "Consider whether this is worth addressing as part of this PR as well."
 
 ## Generated code
@@ -22,6 +25,16 @@ The main codebase of the CodeQL Action is written in TypeScript and compiled to
 
 GitHub Actions workflows in the `.github/workflows` directory whose filenames start with two underscores (e.g. `__all-platform-bundle.yml`) are automatically generated using the `pr-checks/sync.sh` script from template files in the `pr-checks/checks` directory. Therefore, you do not need to review files in the `.github/workflows` directory that start with two underscores. However, you should review changes to the `pr-checks` directory as well as workflows in the `.github/workflows` directory that do not start with underscores.
 
+## Feature flags
+
+Some functionality in the code may be gated behind feature flags, which are controlled server-side. If the functionality that is gated behind them is user-facing, then this should be documented in a change note at the time that the gated functionality is added. See `CHANGELOG.md` for existing notes that mention "experimental" for examples. These typically include information about when users may expect to see the feature rolled out.
+
+When a feature flag that gates some functionality is removed, this does not need to be documented in a change note since the feature is already rolled out and the notes for the release which introduced the feature already describe it. The only exception is functionality which will start having an effect on GHES as a result of the feature flag removal:
+
+- Most functionality gated behind feature flags does not affect GHES.
+- We can make functionality available on GHES by setting `defaultValue` to `true` first and waiting for this to have had an effect on GHES. No change note is needed if a feature flag is removed where `defaultValue` was `true`.
+- When removing a feature flag where `defaultValue` was `false` and the functionality starts to affect GHES, a change note may be appropriate.
+
 ## Release process
 
 Changes that are merged to the `main` branch are not used in production straight away. Instead, changes from `main` must first be released using the release process, which has to be manually triggered by a staff member. Therefore, it is possible for further changes to be made to the code after some changes have been merged to `main` but before they are released. Once the release process automation is started, a series of PRs are automatically generated:

.github/workflows/pr-checks.yml

@@ -2,6 +2,9 @@ name: PR Checks
 
 on:
   push:
+    branches:
+      - "main"
+      - "releases/v*"
   pull_request:
   merge_group:
     types: [checks_requested]
@@ -145,12 +148,11 @@ jobs:
 
       - name: Fetch base commit
         id: fetch-base
-        # Forks and Dependabot PRs don't have permission to write comments, so skip the repo size
-        # check in those cases.
+        # Skip this (and subsequent steps) if the PR originates from a fork, since this workflow
+        # won't have the necessary permissions to post comments on the PR.
         if: >-
           github.event_name == 'pull_request' &&
-          github.event.pull_request.head.repo.full_name == github.repository &&
-          github.event.pull_request.user.login != 'dependabot[bot]'
+          github.event.pull_request.head.repo.full_name == github.repository
         env:
           BASE_SHA: ${{ github.event.pull_request.base.sha }}
           HEAD_SHA: ${{ github.event.pull_request.head.sha }}
@@ -209,7 +211,7 @@ jobs:
     if: >-
       github.event_name == 'pull_request' &&
       github.event.pull_request.head.repo.full_name == github.repository &&
-      github.event.pull_request.user.login != 'dependabot[bot]' &&
+      github.actor != 'dependabot[bot]' &&
       needs.other-checks.result == 'success'
     permissions:
       contents: read

lib/entry-points.js

@@ -170452,24 +170452,12 @@ function fixCodeQualityCategory(logger2, category, env = getEnv()) {
 
 // src/logging.ts
 var core5 = __toESM(require_core());
-var joinMessageStrings = (message) => {
-  if (Array.isArray(message)) {
-    return message.join(" ");
-  } else {
-    return message;
-  }
-};
-var autoJoinMessageStrings = (fn) => {
-  return (message) => {
-    return fn(joinMessageStrings(message));
-  };
-};
 function getActionsLogger() {
   return {
-    debug: autoJoinMessageStrings(core5.debug),
-    info: autoJoinMessageStrings(core5.info),
-    warning: autoJoinMessageStrings(core5.warning),
-    error: autoJoinMessageStrings(core5.error),
+    debug: core5.debug,
+    info: core5.info,
+    warning: core5.warning,
+    error: core5.error,
     isDebug: core5.isDebug,
     startGroup: core5.startGroup,
     endGroup: core5.endGroup

src/actions-util.test.ts

@@ -11,6 +11,7 @@ import {
 } from "./actions-util";
 import { computeAutomationID } from "./api-client";
 import { EnvVar } from "./environment";
+import { getRunnerLogger } from "./logging";
 import { RecordingLogger, setupTests } from "./testing-utils";
 import { initializeEnvironment, withTmpDir } from "./util";
 
@@ -237,7 +238,7 @@ test.serial("fixCodeQualityCategory", (t) => {
       GITHUB_EVENT_NAME: "dynamic",
     },
     () => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
 
       // Categories that should get adjusted.
       t.is(fixCodeQualityCategory(logger, "/language:c#"), "/language:csharp");

src/analyses.test.ts

@@ -15,6 +15,7 @@ import {
   supportedAnalysisKinds,
 } from "./analyses";
 import { EnvVar } from "./environment";
+import { getRunnerLogger } from "./logging";
 import {
   createFeatures,
   RecordingLogger,
@@ -64,7 +65,7 @@ test.serial(
       .withArgs("analysis-kinds")
       .returns("code-scanning,code-quality");
     const result = await getAnalysisKinds(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       features,
       true,
     );
@@ -138,7 +139,7 @@ test.serial(
     const optionalInputStub = sinon.stub(actionsUtil, "getOptionalInput");
     optionalInputStub.withArgs("quality-queries").returns("code-quality");
     const result = await getAnalysisKinds(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       features,
       true,
     );
@@ -154,7 +155,7 @@ test.serial(
     const requiredInputStub = sinon.stub(actionsUtil, "getRequiredInput");
     requiredInputStub.withArgs("analysis-kinds").returns("no-such-thing");
     await t.throwsAsync(
-      getAnalysisKinds(new RecordingLogger(), features, true),
+      getAnalysisKinds(getRunnerLogger(true), features, true),
     );
   },
 );
@@ -180,7 +181,7 @@ for (let i = 0; i < analysisKinds.length; i++) {
             .withArgs("analysis-kinds")
             .returns([analysisKind, otherAnalysis].join(","));
           const result = await getAnalysisKinds(
-            new RecordingLogger(),
+            getRunnerLogger(true),
             features,
             true,
           );
@@ -198,7 +199,7 @@ for (let i = 0; i < analysisKinds.length; i++) {
             .withArgs("analysis-kinds")
             .returns([analysisKind, otherAnalysis].join(","));
           await t.throwsAsync(
-            getAnalysisKinds(new RecordingLogger(), features, true),
+            getAnalysisKinds(getRunnerLogger(true), features, true),
             {
               instanceOf: ConfigurationError,
               message: `${analysisKind} and ${otherAnalysis} cannot be enabled at the same time`,

src/analyze.test.ts

@@ -17,12 +17,12 @@ import { defaultSuites } from "./config/db-config";
 import { Feature } from "./feature-flags";
 import * as gitUtils from "./git-utils";
 import { BuiltInLanguage } from "./languages";
+import { getRunnerLogger } from "./logging";
 import {
   setupTests,
   setupActionsVars,
   createFeatures,
   createTestConfig,
-  RecordingLogger,
   callee,
 } from "./testing-utils";
 import * as uploadLib from "./upload-lib";
@@ -195,7 +195,7 @@ test.serial("status report fields", async (t) => {
         undefined,
         codeql,
         config,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         createFeatures([Feature.QaTelemetryEnabled]),
       );
       t.deepEqual(Object.keys(statusReport).sort(), [

src/artifact-scanner.test.ts

@@ -10,12 +10,12 @@ import {
   scanArtifactsForTokens,
   TokenType,
 } from "./artifact-scanner";
+import { getRunnerLogger } from "./logging";
 import {
   checkExpectedLogMessages,
   getRecordingLogger,
   LoggedMessage,
   makeTestToken,
-  RecordingLogger,
 } from "./testing-utils";
 
 test("makeTestToken", (t) => {
@@ -121,7 +121,7 @@ for (const { type, value, checkPattern } of testTokens) {
 }
 
 test("scanArtifactsForTokens handles files without tokens", async (t) => {
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
   const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "scanner-test-"));
 
   try {

src/cli/output-cache.test.ts

@@ -3,14 +3,15 @@ import path from "path";
 
 import test from "ava";
 
-import { RecordingLogger, setupTests } from "../testing-utils";
+import { getRunnerLogger } from "../logging";
+import { setupTests } from "../testing-utils";
 import * as util from "../util";
 
 import { getCachedCodeQlVersion } from "./output-cache";
 
 setupTests(test);
 
-const logger = new RecordingLogger();
+const logger = getRunnerLogger(true);
 
 test.serial(
   "getCachedCodeQlVersion reuses a version persisted by an earlier step",

src/codeql.test.ts

@@ -22,6 +22,7 @@ import type { Config } from "./config-utils";
 import * as defaults from "./defaults.json";
 import { DocUrl } from "./doc-url";
 import { BuiltInLanguage } from "./languages";
+import { getRunnerLogger } from "./logging";
 import { ToolsSource } from "./setup-codeql";
 import {
   setupTests,
@@ -33,8 +34,6 @@ import {
   makeVersionInfo,
   createTestConfig,
   makeMacro,
-  getCodeQLForTesting,
-  RecordingLogger,
 } from "./testing-utils";
 import { ToolsDownloadStatusReport } from "./tools-download";
 import * as util from "./util";
@@ -103,7 +102,7 @@ async function installIntoToolcache({
     undefined, // otherLanguagePacksReason
     false, // useOverlayAwareDefaultCliVersion
     createFeatures([]),
-    new RecordingLogger(),
+    getRunnerLogger(true),
     false,
   );
 }
@@ -141,7 +140,7 @@ function mockApiDetails(apiDetails: Required<GitHubApiDetails>) {
 }
 
 async function stubCodeql(): Promise<codeql.CodeQL> {
-  const codeqlObject = await getCodeQLForTesting();
+  const codeqlObject = await codeql.getCodeQLForTesting();
   sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.17.6"));
   sinon
     .stub(codeqlObject, "isTracedLanguage")
@@ -177,7 +176,7 @@ test.serial(
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
           false,
         );
 
@@ -213,7 +212,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
 
@@ -253,7 +252,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
       t.assert(toolcache.find("CodeQL", "0.0.0-20200610"));
@@ -363,7 +362,7 @@ for (const {
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
           false,
         );
         t.assert(toolcache.find("CodeQL", expectedToolcacheVersion));
@@ -406,7 +405,7 @@ for (const toolcacheVersion of [
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
           false,
         );
         t.is(
@@ -451,7 +450,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
       t.deepEqual(result.toolsVersion, "0.0.0-20200601");
@@ -498,7 +497,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
       t.deepEqual(result.toolsVersion, defaults.cliVersion);
@@ -538,7 +537,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
       t.deepEqual(result.toolsVersion, defaults.cliVersion);
@@ -580,7 +579,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
         false,
       );
 
@@ -665,7 +664,7 @@ const injectedConfigMacro = makeMacro({
         tempDir,
       };
       thisStubConfig.computedConfig = generateCodeScanningConfig(
-        new RecordingLogger(),
+        getRunnerLogger(true),
         thisStubConfig.originalUserInput,
         augmentationProperties,
       );
@@ -1040,7 +1039,7 @@ test.serial("runTool summarizes autobuilder errors", async (t) => {
     [2019-09-18 12:00:01] [autobuild] A non-error message
   `;
   stubToolRunnerConstructor(1, stderr);
-  const codeqlObject = await getCodeQLForTesting();
+  const codeqlObject = await codeql.getCodeQLForTesting();
   sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.17.6"));
   sinon.stub(codeqlObject, "resolveExtractor").resolves("/path/to/extractor");
   // io throws because of the test CodeQL object.
@@ -1096,7 +1095,7 @@ test.serial("runTool recognizes fatal internal errors", async (t) => {
     com.semmle.util.exception.CatastrophicError: An error occurred while evaluating ControlFlowGraph::ControlFlow::Root.isRootOf/1#dispred#f610e6ed/2@86282cc8
     Severe disk cache trouble (corruption or out of space) at /home/runner/work/_temp/codeql_databases/go/db-go/default/cache/pages/28/33.pack: Failed to write item to disk`;
   stubToolRunnerConstructor(1, stderr);
-  const codeqlObject = await getCodeQLForTesting();
+  const codeqlObject = await codeql.getCodeQLForTesting();
   sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.17.6"));
   sinon.stub(codeqlObject, "resolveExtractor").resolves("/path/to/extractor");
   // io throws because of the test CodeQL object.

src/codeql.ts

@@ -26,7 +26,7 @@ import {
 } from "./feature-flags";
 import { isAnalyzingDefaultBranch } from "./git-utils";
 import { Language } from "./languages";
-import type { Logger } from "./logging";
+import { getRunnerLogger, Logger } from "./logging";
 import { writeBaseDatabaseOidsFile, writeOverlayChangesFile } from "./overlay";
 import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
 import * as setupCodeql from "./setup-codeql";
@@ -486,6 +486,18 @@ export function createStubCodeQL(partialCodeql: Partial<CodeQL>): CodeQL {
   };
 }
 
+/**
+ * Get a real, newly created CodeQL instance for testing. The instance refers to
+ * a non-existent placeholder codeql command, so tests that use this function
+ * should also stub the toolrunner.ToolRunner constructor.
+ */
+export async function getCodeQLForTesting(
+  cmd = "codeql-for-testing",
+  logger: Logger = getRunnerLogger(true),
+): Promise<CodeQL> {
+  return getCodeQLForCmd(logger, cmd, false);
+}
+
 /**
  * Return a CodeQL object for CodeQL CLI access.
  *
@@ -494,7 +506,7 @@ export function createStubCodeQL(partialCodeql: Partial<CodeQL>): CodeQL {
  *        version requirement. Must be set to true outside tests.
  * @returns A new CodeQL object
  */
-export async function getCodeQLForCmd(
+async function getCodeQLForCmd(
   logger: Logger,
   cmd: string,
   checkVersion: boolean,

src/config-utils.test.ts

@@ -21,6 +21,7 @@ import { RepositoryProperties } from "./feature-flags/properties";
 import * as gitUtils from "./git-utils";
 import { GitVersionInfo } from "./git-utils";
 import { BuiltInLanguage, Language } from "./languages";
+import { getRunnerLogger } from "./logging";
 import { CODEQL_OVERLAY_MINIMUM_VERSION } from "./overlay";
 import * as overlayDiagnostics from "./overlay/diagnostics";
 import { OverlayDisabledReason } from "./overlay/diagnostics";
@@ -41,7 +42,6 @@ import {
   callee,
   SAMPLE_DOTCOM_API_DETAILS,
   AssertableTarget,
-  RecordingLogger,
 } from "./testing-utils";
 import {
   GitHubVariant,
@@ -102,7 +102,7 @@ function createTestInitConfigInputs(
       features: createFeatures([]),
       repositoryProperties: {},
       enableFileCoverageInformation: true,
-      logger: new RecordingLogger(),
+      logger: getRunnerLogger(true),
     } satisfies configUtils.InitConfigInputs,
     overrides,
   );
@@ -150,7 +150,7 @@ function mockListLanguages(languages: string[]) {
 
 test.serial("load empty config", async (t) => {
   return await withTmpDir(async (tempDir) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const languages = "javascript,python";
 
     setupActionsVars(tempDir, tempDir);
@@ -196,7 +196,7 @@ test.serial("load empty config", async (t) => {
 
 test.serial("load code quality config", async (t) => {
   return await withTmpDir(async (tempDir) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const languages = "actions";
 
     setupActionsVars(tempDir, tempDir);
@@ -252,7 +252,7 @@ test.serial(
   "initActionState doesn't throw if there are queries configured in the repository properties",
   async (t) => {
     return await withTmpDir(async (tempDir) => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const languages = "javascript";
 
       setupActionsVars(tempDir, tempDir);
@@ -315,7 +315,7 @@ test.serial(
 
 test.serial("loading a saved config produces the same config", async (t) => {
   return await withTmpDir(async (tempDir) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     const codeql = createStubCodeQL({
       async resolveLanguages() {
@@ -363,7 +363,7 @@ test.serial("loading a saved config produces the same config", async (t) => {
 
 test.serial("loading config with version mismatch throws", async (t) => {
   return await withTmpDir(async (tempDir) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     const codeql = createStubCodeQL({
       async resolveLanguages() {
@@ -788,12 +788,12 @@ test.serial("Unknown languages", async (t) => {
   });
 });
 
-const mockLogger = new RecordingLogger();
+const mockLogger = getRunnerLogger(true);
 
 test.serial("no generateRegistries when registries is undefined", async (t) => {
   return await withTmpDir(async (tmpDir) => {
     const registriesInput = undefined;
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const { registriesAuthTokens, qlconfigFile } =
       await configUtils.generateRegistries(registriesInput, tmpDir, logger);
 
@@ -814,7 +814,7 @@ test.serial(
           token: "not-a-token",
         },
       ]);
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const { registriesAuthTokens, qlconfigFile } =
         await configUtils.generateRegistries(registriesInput, tmpDir, logger);
 
@@ -2247,7 +2247,7 @@ test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff
   const config = createTestConfig({});
   config.overlayDatabaseMode = OverlayDatabaseMode.None;
   const codeql = createStubCodeQL({});
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
 
   await configUtils.applyIncrementalAnalysisSettings(
     config,
@@ -2265,7 +2265,7 @@ test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions w
     overlayDatabaseMode: OverlayDatabaseMode.Overlay,
   });
   const codeql = createStubCodeQL({});
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
 
   await configUtils.applyIncrementalAnalysisSettings(
     config,
@@ -2288,7 +2288,7 @@ test.serial(
     });
     config.useOverlayDatabaseCaching = true;
     const codeql = createStubCodeQL({});
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const addDiagnosticsStub = sinon
       .stub(overlayDiagnostics, "addOverlayDisablementDiagnostics")
       .resolves();
@@ -2320,7 +2320,7 @@ test.serial(
     config.useOverlayDatabaseCaching = false;
     config.overlayModeSetExplicitly = true;
     const codeql = createStubCodeQL({});
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const addDiagnosticsStub = sinon
       .stub(overlayDiagnostics, "addOverlayDisablementDiagnostics")
       .resolves();
@@ -2343,7 +2343,7 @@ test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only r
   const config = createTestConfig({});
   config.overlayDatabaseMode = OverlayDatabaseMode.None;
   const codeql = createStubCodeQL({});
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
 
   await configUtils.applyIncrementalAnalysisSettings(
     config,
@@ -2557,7 +2557,7 @@ test("determineUserConfig - merges configs if FF is enabled in Default Setup", a
     // also check whether loading the configuration from disk that was written
     // by `determineUserConfig` matches our expectations.
     const loadedFromDisk = configUtils.getLocalConfig(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       expectedConfigPath,
       false,
     );

src/config/db-config.test.ts

@@ -2,6 +2,7 @@ import test, { ExecutionContext } from "ava";
 
 import { RepositoryProperties } from "../feature-flags/properties";
 import { BuiltInLanguage, Language } from "../languages";
+import { getRunnerLogger } from "../logging";
 import {
   checkExpectedLogMessages,
   getRecordingLogger,
@@ -383,7 +384,7 @@ calculateAugmentationErrorMacro(
 
 test("parseUserConfig - successfully parses valid YAML", (t) => {
   const result = dbConfig.parseUserConfig(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     "test",
     `
     paths-ignore:
@@ -409,7 +410,7 @@ test("parseUserConfig - throws a ConfigurationError if the file is not valid YAM
   t.throws(
     () =>
       dbConfig.parseUserConfig(
-        new RecordingLogger(),
+        getRunnerLogger(true),
         "test",
         `
         paths-ignore:
@@ -628,7 +629,7 @@ test("mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Se
 /** Parses `contents` as a configuration without validating it. */
 function parseUnvalidatedConfig(contents: string): dbConfig.UserConfig {
   return dbConfig.parseUserConfig(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     "test",
     contents,
     false,

src/diff-informed-analysis-utils.test.ts

@@ -10,6 +10,7 @@ import {
   exportedForTesting,
 } from "./diff-informed-analysis-utils";
 import { Feature, FeatureEnablement, initFeatures } from "./feature-flags";
+import { getRunnerLogger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
 import {
   setupTests,
@@ -18,7 +19,6 @@ import {
   mockFeatureFlagApiEndpoint,
   setupActionsVars,
   makeMacro,
-  RecordingLogger,
 } from "./testing-utils";
 import { GitHubVariant, withTmpDir } from "./util";
 import type { GitHubVersion } from "./util";
@@ -55,7 +55,7 @@ const testShouldPerformDiffInformedAnalysis = makeMacro({
       setupActionsVars(tmpDir, tmpDir);
 
       const testCase = { ...defaultTestCase, ...partialTestCase };
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const codeql = mockCodeQLVersion(testCase.codeQLVersion);
 
       if (testCase.diffInformedQueriesEnvVar !== undefined) {
@@ -182,7 +182,7 @@ test.serial(
   async (t) => {
     await withTmpDir(async (tmpDir) => {
       setupActionsVars(tmpDir, tmpDir);
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const codeql = mockCodeQLVersion("2.21.0");
       const features = createFeatures([Feature.DiffInformedQueries]);
 
@@ -207,7 +207,7 @@ test.serial(
   async (t) => {
     await withTmpDir(async (tmpDir) => {
       setupActionsVars(tmpDir, tmpDir);
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const codeql = mockCodeQLVersion("2.21.0");
       // A features implementation whose getValue rejects, simulating an
       // unexpected failure when determining whether diff-informed analysis
@@ -237,7 +237,7 @@ test.serial(
   async (t) => {
     await withTmpDir(async (tmpDir) => {
       setupActionsVars(tmpDir, tmpDir);
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const codeql = mockCodeQLVersion("2.21.0");
       const features = createFeatures([Feature.DiffInformedQueries]);
 
@@ -274,7 +274,7 @@ test.serial(
   async (t) => {
     await withTmpDir(async (tmpDir) => {
       setupActionsVars(tmpDir, tmpDir);
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const codeql = mockCodeQLVersion("2.21.0");
       const features = createFeatures([Feature.DiffInformedQueries]);
 
@@ -313,7 +313,7 @@ function runGetDiffRanges(changes: number, patch: string[] | undefined): any {
       changes,
       patch: patch?.join("\n"),
     },
-    new RecordingLogger(),
+    getRunnerLogger(true),
   );
 }
 

src/feature-flags/properties.test.ts

@@ -2,6 +2,7 @@ import test from "ava";
 import * as sinon from "sinon";
 
 import * as api from "../api-client";
+import { getRunnerLogger } from "../logging";
 import { parseRepositoryNwo } from "../repository";
 import { RecordingLogger, setupTests } from "../testing-utils";
 
@@ -18,7 +19,7 @@ test.serial(
       url: "",
       data: {},
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     await t.throwsAsync(
       properties.loadPropertiesFromApi(logger, mockRepositoryNwo),
@@ -38,7 +39,7 @@ test.serial(
       url: "",
       data: [{}],
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     await t.throwsAsync(
       properties.loadPropertiesFromApi(logger, mockRepositoryNwo),
@@ -62,7 +63,7 @@ test.serial(
         { property_name: "also-not-used-by-us", value: ["A", "B", "C"] },
       ],
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     await t.notThrowsAsync(
       properties.loadPropertiesFromApi(logger, mockRepositoryNwo),
@@ -85,7 +86,7 @@ test.serial("loadPropertiesFromApi loads known properties", async (t) => {
       { property_name: "unknown-property", value: "something" },
     ] satisfies properties.GitHubPropertiesResponse,
   });
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
   const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
   const response = await properties.loadPropertiesFromApi(
     logger,
@@ -112,7 +113,7 @@ test.serial("loadPropertiesFromApi parses true boolean property", async (t) => {
       { property_name: "github-codeql-extra-queries", value: "+queries" },
     ] satisfies properties.GitHubPropertiesResponse,
   });
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
   const warningSpy = sinon.spy(logger, "warning");
   const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
   const response = await properties.loadPropertiesFromApi(
@@ -140,7 +141,7 @@ test.serial(
         },
       ] satisfies properties.GitHubPropertiesResponse,
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const warningSpy = sinon.spy(logger, "warning");
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     const response = await properties.loadPropertiesFromApi(
@@ -163,7 +164,7 @@ test.serial(
       url: "",
       data: [{ property_name: "github-codeql-extra-queries", value: 123 }],
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     await t.throwsAsync(
       properties.loadPropertiesFromApi(logger, mockRepositoryNwo),
@@ -189,7 +190,7 @@ test.serial(
         },
       ] satisfies properties.GitHubPropertiesResponse,
     });
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     const warningSpy = sinon.spy(logger, "warning");
     const mockRepositoryNwo = parseRepositoryNwo("owner/repo");
     const response = await properties.loadPropertiesFromApi(

src/feature-flags/testing-util.ts

@@ -8,12 +8,11 @@ import {
   FeatureWithoutCLI,
   initFeatures,
 } from "../feature-flags";
-import type { Logger } from "../logging";
+import { getRunnerLogger } from "../logging";
 import { parseRepositoryNwo } from "../repository";
 import {
   LoggedMessage,
   mockCodeQLVersion,
-  RecordingLogger,
   setupActionsVars,
 } from "../testing-utils";
 import { ToolsFeature } from "../tools-features";
@@ -52,7 +51,7 @@ export function assertAllFeaturesUndefinedInApi(
 
 export function setUpFeatureFlagTests(
   tmpDir: string,
-  logger: Logger = new RecordingLogger(),
+  logger = getRunnerLogger(true),
   gitHubVersion = { type: GitHubVariant.DOTCOM } as util.GitHubVersion,
 ): FeatureEnablement {
   setupActionsVars(tmpDir, tmpDir);

src/fingerprints.test.ts

@@ -5,8 +5,9 @@ import * as ava from "ava";
 import test from "ava";
 
 import * as fingerprints from "./fingerprints";
+import { getRunnerLogger } from "./logging";
 import * as sarif from "./sarif";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import { setupTests } from "./testing-utils";
 import * as util from "./util";
 
 setupTests(test);
@@ -130,7 +131,7 @@ function testResolveUriToFile(uri: any, index: any, artifactsURIs: any[]) {
     location,
     artifacts,
     process.cwd(),
-    new RecordingLogger(),
+    getRunnerLogger(true),
   );
 }
 
@@ -217,7 +218,7 @@ test("addFingerprints", async (t) => {
     await fingerprints.addFingerprints(
       input,
       sourceRoot,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     ),
     expected,
   );
@@ -245,7 +246,7 @@ test("missingRegions", async (t) => {
     await fingerprints.addFingerprints(
       input,
       sourceRoot,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     ),
     expected,
   );

src/init-action-post-helper.test.ts

@@ -11,6 +11,7 @@ import * as debugArtifacts from "./debug-artifacts";
 import { EnvVar } from "./environment";
 import { Feature } from "./feature-flags";
 import * as initActionPostHelper from "./init-action-post-helper";
+import { getRunnerLogger } from "./logging";
 import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
 import * as overlayStatus from "./overlay/status";
 import { parseRepositoryNwo } from "./repository";
@@ -19,7 +20,6 @@ import {
   createFeatures,
   createTestConfig,
   DEFAULT_ACTIONS_VARS,
-  getCodeQLForTesting,
   getTestEnv,
   makeMacro,
   makeVersionInfo,
@@ -62,7 +62,7 @@ test.serial("init-post action with debug mode off", async (t) => {
       createFeatures([]),
       "success",
       getTestEnv(),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     t.assert(uploadAllAvailableDebugArtifactsSpy.notCalled);
@@ -86,7 +86,7 @@ test.serial("init-post action with debug mode on", async (t) => {
       createFeatures([]),
       "success",
       getTestEnv(),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     t.assert(uploadAllAvailableDebugArtifactsSpy.called);
@@ -383,7 +383,7 @@ test.serial(
         createFeatures([Feature.OverlayAnalysisStatusSave]),
         "success",
         getTestEnv(),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.true(
@@ -453,7 +453,7 @@ test.serial(
         createFeatures([]),
         "success",
         getTestEnv(),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.true(
@@ -497,7 +497,7 @@ test.serial("does not save overlay status when build successful", async (t) => {
       createFeatures([Feature.OverlayAnalysisStatusSave]),
       "success",
       env,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     t.true(
@@ -536,7 +536,7 @@ test.serial(
         createFeatures([]),
         "success",
         getTestEnv(),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.true(
@@ -589,7 +589,7 @@ async function runOverlayPostStep({
       createFeatures([Feature.OverlayAnalysisStatusSave]),
       jobStatus,
       env,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     return { saveOverlayStatusStub };
@@ -737,7 +737,7 @@ async function testFailedSarifUpload(
     .withArgs("matrix")
     .returns(JSON.stringify(matrix));
 
-  const codeqlObject = await getCodeQLForTesting();
+  const codeqlObject = await codeql.getCodeQLForTesting();
   sinon.stub(codeql, "getCodeQL").resolves(codeqlObject);
   sinon.stub(codeqlObject, "getVersion").resolves(makeVersionInfo("2.17.6"));
   const databaseExportDiagnosticsStub = sinon.stub(
@@ -764,7 +764,7 @@ async function testFailedSarifUpload(
     config,
     parseRepositoryNwo("github/codeql-action"),
     createFeatures(features),
-    new RecordingLogger(),
+    getRunnerLogger(true),
   );
   if (expectUpload) {
     t.deepEqual(result, {
@@ -832,7 +832,7 @@ async function mockRiskAssessmentEnv(matrix: string) {
     .stub(apiClient, "getGitHubVersion")
     .resolves({ type: util.GitHubVariant.GHES, version: "3.0.0" });
 
-  const codeqlObject = await getCodeQLForTesting();
+  const codeqlObject = await codeql.getCodeQLForTesting();
   const databaseExportDiagnostics = sinon
     .stub(codeqlObject, "databaseExportDiagnostics")
     .resolves();

src/logging.test.ts

@@ -1,17 +0,0 @@
-import test from "ava";
-
-import { joinMessageStrings } from "./logging";
-import { setupTests } from "./testing-utils";
-
-setupTests(test);
-
-test("joinMessageStrings", async (t) => {
-  // For strings and errors, it is the identity function.
-  t.deepEqual(joinMessageStrings("Hello"), "Hello");
-
-  const error = new Error("Some error");
-  t.deepEqual(joinMessageStrings(error), error);
-
-  // For arrays of strings, we get a join-ed string.
-  t.deepEqual(joinMessageStrings(["foo", "bar"]), "foo bar");
-});

src/logging.ts

@@ -1,92 +1,45 @@
 import * as core from "@actions/core";
 
-/** Types that all of our logging functions accept. */
-export type Loggable = string | string[];
-
-/** Some logging functions also accept errors directly. */
-export type LoggableError = Loggable | Error;
-
 export interface Logger {
-  /**
-   * Logs {@link message} at debug-level.
-   * In Actions, this will only be output if workflow debugging is enabled.
-   *
-   * @param message
-   *    A string or an array of strings. If an array of strings is provided, they are joined using
-   *    space as the delimiter and printed as a single line.
-   */
-  debug: (message: Loggable) => void;
-  /**
-   * Logs {@link message} at info-level.
-   * In Actions, this will always be output.
-   *
-   * @param message
-   *    A string or an array of strings. If an array of strings is provided, they are joined using
-   *    space as the delimiter and printed as a single line.
-   */
-  info: (message: Loggable) => void;
-  /**
-   * Logs {@link message} at warning-level.
-   * In Actions, this will always be output and highlighted as a warning. It will additionally
-   * result in a warning-level workflow annotation.
-   *
-   * @param message
-   *    A string or an array of strings. If an array of strings is provided, they are joined using
-   *    space as the delimiter and printed as a single line.
-   */
-  warning: (message: LoggableError) => void;
-  /**
-   * Logs {@link message} at error-level.
-   * In Actions, this will always be output and highlighted as an error. It will additionally
-   * result in an error-level workflow annotation.
-   *
-   * @param message
-   *    A string or an array of strings. If an array of strings is provided, they are joined using
-   *    space as the delimiter and printed as a single line.
-   */
-  error: (message: LoggableError) => void;
-  /**
-   * Gets a value indicating whether debugging is enabled.
-   */
+  debug: (message: string) => void;
+  info: (message: string) => void;
+  warning: (message: string | Error) => void;
+  error: (message: string | Error) => void;
+
   isDebug: () => boolean;
 
   startGroup: (name: string) => void;
   endGroup: () => void;
 }
 
-/** If `message` is an array of strings, the array is `join`-ed into one string separated by spaces. */
-export const joinMessageStrings = <T>(
-  message: string[] | string | T,
-): string | T => {
-  if (Array.isArray(message)) {
-    return message.join(" ");
-  } else {
-    return message;
-  }
-};
-
-/**
- * Wraps a logging function so that `joinMessageStrings` is applied to the input before
- * calling the wrapped logging function.
- */
-const autoJoinMessageStrings = <T>(fn: (msg: string | T) => void) => {
-  return (message: string[] | string | T) => {
-    return fn(joinMessageStrings(message));
-  };
-};
-
 export function getActionsLogger(): Logger {
   return {
-    debug: autoJoinMessageStrings(core.debug),
-    info: autoJoinMessageStrings(core.info),
-    warning: autoJoinMessageStrings(core.warning),
-    error: autoJoinMessageStrings(core.error),
+    debug: core.debug,
+    info: core.info,
+    warning: core.warning,
+    error: core.error,
     isDebug: core.isDebug,
     startGroup: core.startGroup,
     endGroup: core.endGroup,
   };
 }
 
+export function getRunnerLogger(debugMode: boolean): Logger {
+  return {
+    // eslint-disable-next-line no-console
+    debug: debugMode ? console.debug : () => undefined,
+    // eslint-disable-next-line no-console
+    info: console.info,
+    // eslint-disable-next-line no-console
+    warning: console.warn,
+    // eslint-disable-next-line no-console
+    error: console.error,
+    isDebug: () => debugMode,
+    startGroup: () => undefined,
+    endGroup: () => undefined,
+  };
+}
+
 export function withGroup<T>(groupName: string, f: () => T): T {
   core.startGroup(groupName);
   try {

src/overlay/caching.test.ts

@@ -11,11 +11,11 @@ import type { ResolveDatabaseOutput } from "../codeql";
 import * as environment from "../environment";
 import * as gitUtils from "../git-utils";
 import { BuiltInLanguage } from "../languages";
+import { getRunnerLogger } from "../logging";
 import {
   createTestConfig,
   makeMacro,
   mockCodeQLVersion,
-  RecordingLogger,
   setupTests,
 } from "../testing-utils";
 import * as utils from "../util";
@@ -63,7 +63,7 @@ const testDownloadOverlayBaseDatabaseFromCache = makeMacro({
       const dbLocation = path.join(tmpDir, "db");
       await fs.promises.mkdir(dbLocation, { recursive: true });
 
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const testCase = { ...defaultDownloadTestCase, ...partialTestCase };
       const config = createTestConfig({
         dbLocation,
@@ -214,7 +214,7 @@ testDownloadOverlayBaseDatabaseFromCache.serial(
 );
 
 test.serial("overlay-base database cache keys remain stable", async (t) => {
-  const logger = new RecordingLogger();
+  const logger = getRunnerLogger(true);
   const config = createTestConfig({ languages: ["python", "javascript"] });
   const codeQlVersion = "2.23.0";
   const commitOid = "abc123def456";
@@ -261,7 +261,7 @@ test.serial("overlay-base database cache keys remain stable", async (t) => {
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases returns unique versions sorted latest first",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     sinon.stub(apiClient, "listActionsCaches").resolves([
@@ -287,7 +287,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases returns empty list when no caches exist",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     sinon.stub(apiClient, "listActionsCaches").resolves([]);
@@ -303,7 +303,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases returns empty list when cache keys are unparseable",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     sinon.stub(apiClient, "listActionsCaches").resolves([
@@ -324,7 +324,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases returns the single version when only one cache exists",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     sinon.stub(apiClient, "listActionsCaches").resolves([
@@ -344,7 +344,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases resolves language aliases",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
     // The alias `c++` should be resolved to "cpp" and match cache entries keyed with "cpp"
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
@@ -365,7 +365,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases de-duplicates resolved language aliases",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     const listActionsCachesStub = sinon
@@ -391,7 +391,7 @@ test.serial(
 test.serial(
   "getCodeQlVersionsForOverlayBaseDatabases ignores nightly versions with build metadata",
   async (t) => {
-    const logger = new RecordingLogger();
+    const logger = getRunnerLogger(true);
 
     sinon.stub(apiClient, "getAutomationID").resolves("test-automation-id/");
     sinon.stub(apiClient, "listActionsCaches").resolves([

src/overlay/index.test.ts

@@ -6,11 +6,8 @@ import * as sinon from "sinon";
 
 import * as actionsUtil from "../actions-util";
 import * as gitUtils from "../git-utils";
-import {
-  createTestConfig,
-  RecordingLogger,
-  setupTests,
-} from "../testing-utils";
+import { getRunnerLogger } from "../logging";
+import { createTestConfig, setupTests } from "../testing-utils";
 import { withTmpDir } from "../util";
 
 import { writeBaseDatabaseOidsFile, writeOverlayChangesFile } from ".";
@@ -30,7 +27,7 @@ test.serial(
         ),
       );
 
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const config = createTestConfig({ dbLocation });
 
       // Mock the getFileOidsUnderPath function to return base OIDs
@@ -94,7 +91,7 @@ test.serial(
         ),
       );
 
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const config = createTestConfig({ dbLocation });
 
       // Mock the getFileOidsUnderPath function to return base OIDs
@@ -172,7 +169,7 @@ test.serial(
         ),
       );
 
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const config = createTestConfig({ dbLocation });
 
       // Mock the getFileOidsUnderPath function to return base OIDs
@@ -235,7 +232,7 @@ test.serial(
         ),
       );
 
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const config = createTestConfig({ dbLocation });
 
       // Base OIDs (sourceRoot-relative paths)

src/setup-codeql.test.ts

@@ -14,6 +14,7 @@ import * as diagnostics from "./diagnostics";
 import { ActionsEnvVars, Env, EnvVar, ReadOnlyEnv } from "./environment";
 import { Feature } from "./feature-flags";
 import { BuiltInLanguage } from "./languages";
+import { getRunnerLogger } from "./logging";
 import { getCacheRestoreKeyPrefix } from "./overlay/caching";
 import { MIN_PER_LANGUAGE_BUNDLE_CLI_VERSION } from "./per-language-bundles";
 import * as setupCodeql from "./setup-codeql";
@@ -21,7 +22,6 @@ import * as tar from "./tar";
 import {
   LINKED_CLI_VERSION,
   LoggedMessage,
-  RecordingLogger,
   SAMPLE_DEFAULT_CLI_VERSION,
   SAMPLE_DOTCOM_API_DETAILS,
   checkExpectedLogMessages,
@@ -117,7 +117,7 @@ test.serial("convert to semver", (t) => {
     try {
       const parsedVersion = setupCodeql.convertToSemVer(
         version,
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
       t.deepEqual(parsedVersion, expectedVersion);
     } catch (e) {
@@ -145,7 +145,7 @@ test.serial(
         GitHubVariant.DOTCOM,
         false,
         features,
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.deepEqual(source, {
@@ -231,7 +231,7 @@ for (const {
           GitHubVariant.DOTCOM,
           tarSupportsZstd,
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.is(source.toolsVersion, LINKED_CLI_VERSION.cliVersion);
@@ -597,7 +597,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         createFeatures([]),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.is(result.toolsVersion, "local");
@@ -634,7 +634,7 @@ for (const toolsInput of ["nightly", "nightly-latest"]) {
           GitHubVariant.DOTCOM,
           true, // tarSupportsZstd
           createFeatures([Feature.PerLanguageBundles]),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.deepEqual(source, {
@@ -694,7 +694,7 @@ test.serial(
           GitHubVariant.DOTCOM,
           true, // tarSupportsZstd
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.is(source.sourceType, "download");
@@ -733,7 +733,7 @@ for (const perLanguageBundles of [false, true]) {
               ? [Feature.ForceNightly, Feature.PerLanguageBundles]
               : [Feature.ForceNightly],
           ),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.is(source.sourceType, "download");
@@ -848,7 +848,7 @@ test.serial(
           GitHubVariant.DOTCOM,
           true,
           createFeatures([]),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         ),
         {
           message:
@@ -1020,7 +1020,7 @@ test.serial(
     t.is(
       setupCodeql.tryGetTagNameFromUrl(
         "https://github.com/org/codeql-bundle-testing/releases/download/codeql-bundle-v2.19.0/codeql-bundle-linux64.tar.zst",
-        new RecordingLogger(),
+        getRunnerLogger(true),
       ),
       "codeql-bundle-v2.19.0",
     );
@@ -1032,7 +1032,7 @@ test.serial(
   (t) => {
     sinon.stub(toolcache, "findAllVersions").returns([]);
     t.is(
-      setupCodeql.getLatestToolcacheVersion(new RecordingLogger()),
+      setupCodeql.getLatestToolcacheVersion(getRunnerLogger(true)),
       undefined,
     );
   },
@@ -1044,7 +1044,7 @@ test.serial(
     const testVersions = ["2.3.1", "3.2.1", "1.2.3"];
     sinon.stub(toolcache, "findAllVersions").returns(testVersions);
 
-    t.is(setupCodeql.getLatestToolcacheVersion(new RecordingLogger()), "3.2.1");
+    t.is(setupCodeql.getLatestToolcacheVersion(getRunnerLogger(true)), "3.2.1");
   },
 );
 
@@ -1098,7 +1098,7 @@ test.serial(
         GitHubVariant.DOTCOM,
         false,
         createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(listStub.calledOnce);
@@ -1137,7 +1137,7 @@ test.serial(
         GitHubVariant.DOTCOM,
         false,
         createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(listStub.notCalled);
@@ -1175,7 +1175,7 @@ test.serial(
         GitHubVariant.DOTCOM,
         true, // tarSupportsZstd
         createFeatures([Feature.PerLanguageBundles]),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.is(source.sourceType, "download");
@@ -1226,7 +1226,7 @@ test.serial(
           GitHubVariant.DOTCOM,
           true, // tarSupportsZstd
           features,
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.is(source.sourceType, "download");
@@ -1296,7 +1296,7 @@ for (const fallback of [false, true]) {
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           createFeatures([Feature.PerLanguageBundles]),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         );
 
         t.true(fetchRelease.calledTwice);
@@ -1561,7 +1561,7 @@ for (const error of [
             undefined, // otherLanguagePacksReason
             false, // useOverlayAwareDefaultCliVersion
             createFeatures([Feature.PerLanguageBundles]),
-            new RecordingLogger(),
+            getRunnerLogger(true),
           ),
           { is: error },
         );
@@ -1597,7 +1597,7 @@ test.serial(
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           createFeatures([]),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         ),
         { is: error },
       );
@@ -1630,7 +1630,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, [
       { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
@@ -1653,7 +1653,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, []);
   },
@@ -1670,7 +1670,7 @@ const noLanguagesMacro = makeMacro({
       overlayMatchEnabledVersions,
       rawLanguages,
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, []);
     t.assert(
@@ -1695,7 +1695,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, []);
   },
@@ -1715,7 +1715,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, [
       { cliVersion: "2.20.2", tagName: "codeql-bundle-v2.20.2" },
@@ -1732,7 +1732,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, []);
     t.assert(
@@ -1756,7 +1756,7 @@ test.serial(
       overlayMatchEnabledVersions,
       ["javascript"],
       createFeatures([Feature.OverlayAnalysisMatchCodeqlVersionDryRun]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(
       result,
@@ -1787,7 +1787,7 @@ test.serial(
         Feature.OverlayAnalysisMatchCodeqlVersion,
         Feature.OverlayAnalysisMatchCodeqlVersionDryRun,
       ]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
     t.deepEqual(result, [
       { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
@@ -2093,7 +2093,7 @@ test.serial(
         env: new ReadOnlyEnv({
           [ActionsEnvVars.RUNNER_TOOL_CACHE]: injectedRoot,
         }),
-        logger: new RecordingLogger(),
+        logger: getRunnerLogger(true),
       });
 
       t.deepEqual(result, {
@@ -2248,7 +2248,7 @@ test.serial(
         undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         createFeatures([]),
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.is(
@@ -2288,7 +2288,7 @@ test.serial(
           undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           createFeatures([]),
-          new RecordingLogger(),
+          getRunnerLogger(true),
         ),
         { is: error },
       );
@@ -2403,7 +2403,7 @@ test.serial(
   "isToolcacheOnWorkspaceFilesystem assumes a different filesystem when it cannot tell",
   async (t) => {
     await withTmpDir(async (tmpDir) => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
 
       setupActionsVars(tmpDir, tmpDir);
       t.true(toolsDownload.isToolcacheOnWorkspaceFilesystem(logger));

src/start-proxy.test.ts

@@ -13,7 +13,7 @@ import { setUpFeatureFlagTests } from "./feature-flags/testing-util";
 import { UnvalidatedObject, validateSchema } from "./json";
 import { makeFromSchema } from "./json/testing-util";
 import { BuiltInLanguage } from "./languages";
-import { Logger } from "./logging";
+import { getRunnerLogger, Logger } from "./logging";
 import { BundlePlatform, getBundlePlatform } from "./platform";
 import * as repository from "./setup/repository";
 import * as startProxyExports from "./start-proxy";
@@ -67,7 +67,7 @@ test("getCredentials prefers registriesCredentials over registrySecrets", async
   ]);
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     registrySecrets,
     registryCredentials,
     undefined,
@@ -84,7 +84,7 @@ test("getCredentials throws an error when configurations are not an array", asyn
   t.throws(
     () =>
       startProxyExports.getCredentials(
-        new RecordingLogger(),
+        getRunnerLogger(true),
         undefined,
         registryCredentials,
         undefined,
@@ -103,7 +103,7 @@ test("getCredentials throws error when credential is not an object", async (t) =
     t.throws(
       () =>
         startProxyExports.getCredentials(
-          new RecordingLogger(),
+          getRunnerLogger(true),
           undefined,
           testCredential,
           undefined,
@@ -124,7 +124,7 @@ test("getCredentials throws error when credential is missing type", async (t) =>
     t.throws(
       () =>
         startProxyExports.getCredentials(
-          new RecordingLogger(),
+          getRunnerLogger(true),
           undefined,
           testCredential,
           undefined,
@@ -147,7 +147,7 @@ test("getCredentials throws error when credential missing host and url", async (
     t.throws(
       () =>
         startProxyExports.getCredentials(
-          new RecordingLogger(),
+          getRunnerLogger(true),
           undefined,
           testCredential,
           undefined,
@@ -161,7 +161,7 @@ test("getCredentials throws error when credential missing host and url", async (
 
 test("getCredentials filters by language when specified", async (t) => {
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     toEncodedJSON(mixedCredentials),
     BuiltInLanguage.java,
@@ -172,7 +172,7 @@ test("getCredentials filters by language when specified", async (t) => {
 
 test("getCredentials returns all for a language when specified", async (t) => {
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     toEncodedJSON([...mixedCredentials, gitSourceCredential]),
     BuiltInLanguage.go,
@@ -192,7 +192,7 @@ test("getCredentials returns all goproxy_servers for Go when specified", async (
   ];
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     toEncodedJSON(multipleGoproxyServers),
     BuiltInLanguage.go,
@@ -221,7 +221,7 @@ test("getCredentials returns all maven_repositories for Java when specified", as
   ];
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     toEncodedJSON(multipleMavenRepositories),
     BuiltInLanguage.java,
@@ -239,7 +239,7 @@ test("getCredentials returns all credentials when no language specified", async
   const credentialsInput = toEncodedJSON(mixedCredentials);
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     credentialsInput,
     undefined,
@@ -269,7 +269,7 @@ test("getCredentials throws an error when non-printable characters are used", as
     t.throws(
       () =>
         startProxyExports.getCredentials(
-          new RecordingLogger(),
+          getRunnerLogger(true),
           undefined,
           credentialsInput,
           undefined,
@@ -300,7 +300,7 @@ for (const oidcSchemaInfo of startProxyExports.oidcSchemas) {
       t.throws(
         () =>
           startProxyExports.getCredentials(
-            new RecordingLogger(),
+            getRunnerLogger(true),
             undefined,
             credentialsInput,
             undefined,
@@ -324,7 +324,7 @@ test("getCredentials accepts OIDC configurations", (t) => {
   );
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     toEncodedJSON(oidcConfigurations),
     BuiltInLanguage.csharp,
@@ -514,7 +514,7 @@ test("getCredentials validates 'replaces-base' correctly", async (t) => {
   ]);
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     credentialsInput,
     BuiltInLanguage.java,
@@ -533,7 +533,7 @@ test("getCredentials validates 'replaces-base' correctly", async (t) => {
   };
   t.throws(() =>
     startProxyExports.getCredentials(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       undefined,
       toEncodedJSON([{ ...baseInvalid, "replaces-base": null }]),
       BuiltInLanguage.java,
@@ -541,7 +541,7 @@ test("getCredentials validates 'replaces-base' correctly", async (t) => {
   );
   t.throws(() =>
     startProxyExports.getCredentials(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       undefined,
       toEncodedJSON([{ ...baseInvalid, "replaces-base": 123 }]),
       BuiltInLanguage.java,
@@ -549,7 +549,7 @@ test("getCredentials validates 'replaces-base' correctly", async (t) => {
   );
   t.throws(() =>
     startProxyExports.getCredentials(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       undefined,
       toEncodedJSON([{ ...baseInvalid, "replaces-base": "true" }]),
       BuiltInLanguage.java,
@@ -565,7 +565,7 @@ test("getCredentials returns only ALWAYS_ENABLED_REGISTRY_TYPE credentials for A
   ]);
 
   const credentials = startProxyExports.getCredentials(
-    new RecordingLogger(),
+    getRunnerLogger(true),
     undefined,
     credentialsInput,
     BuiltInLanguage.actions,
@@ -597,7 +597,7 @@ test("getCredentials always returns ALWAYS_ENABLED_REGISTRY_TYPE credentials for
   // Test all languages.
   for (const language of Object.values(BuiltInLanguage)) {
     const credentials = startProxyExports.getCredentials(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       undefined,
       credentialsInput,
       language,

src/status-report.test.ts

@@ -6,6 +6,7 @@ import * as actionsUtil from "./actions-util";
 import { Config } from "./config-utils";
 import { EnvVar, RegistryProxyVars } from "./environment";
 import { BuiltInLanguage } from "./languages";
+import { getRunnerLogger } from "./logging";
 import { ToolsSource } from "./setup-codeql";
 import type { Registry } from "./start-proxy";
 import {
@@ -153,7 +154,7 @@ test.serial("createStatusReportBase", async (t) => {
         languages: [BuiltInLanguage.java, BuiltInLanguage.swift],
       }),
       { numAvailableBytes: 100, numTotalBytes: 500 },
-      new RecordingLogger(),
+      getRunnerLogger(false),
       "failure cause",
       "exception stack trace",
     );
@@ -202,7 +203,7 @@ test.serial("createStatusReportBase - empty configuration", async (t) => {
       new Date("May 19, 2023 05:19:00"),
       {},
       { numAvailableBytes: 100, numTotalBytes: 500 },
-      new RecordingLogger(),
+      getRunnerLogger(false),
     );
 
     if (t.truthy(statusReport)) {
@@ -224,7 +225,7 @@ test.serial("createStatusReportBase - partial configuration", async (t) => {
         languages: ["go"],
       },
       { numAvailableBytes: 100, numTotalBytes: 500 },
-      new RecordingLogger(),
+      getRunnerLogger(false),
     );
 
     if (t.truthy(statusReport)) {
@@ -247,7 +248,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -263,7 +264,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -280,7 +281,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -296,7 +297,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -313,7 +314,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -329,7 +330,7 @@ test.serial("createStatusReportBase_firstParty", async (t) => {
           new Date("May 19, 2023 05:19:00"),
           createTestConfig({}),
           { numAvailableBytes: 100, numTotalBytes: 500 },
-          new RecordingLogger(),
+          getRunnerLogger(false),
           "failure cause",
           "exception stack trace",
         )
@@ -410,7 +411,7 @@ const testCreateInitWithConfigStatusReport = makeMacro({
         new Date("May 19, 2023 05:19:00"),
         config,
         { numAvailableBytes: 100, numTotalBytes: 500 },
-        new RecordingLogger(),
+        getRunnerLogger(false),
         "failure cause",
         "exception stack trace",
       );

src/tar.test.ts

@@ -3,8 +3,9 @@ import * as stream from "stream";
 
 import test from "ava";
 
+import { getRunnerLogger } from "./logging";
 import { extractTarZst } from "./tar";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import { setupTests } from "./testing-utils";
 import { withTmpDir } from "./util";
 
 setupTests(test);
@@ -16,7 +17,7 @@ test("extractTarZst rejects if the input stream errors", async (t) => {
       archive,
       path.join(tmpDir, "dest"),
       { type: "gnu", version: "1.34" },
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     archive.destroy(

src/testing-utils.ts

@@ -30,7 +30,7 @@ import {
   featureConfig,
   FeatureEnablement,
 } from "./feature-flags";
-import { joinMessageStrings, Loggable, LoggableError, Logger } from "./logging";
+import { Logger } from "./logging";
 import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
 import { getBundlePlatform } from "./platform";
 import { ActionName } from "./status-report";
@@ -658,13 +658,12 @@ export class RecordingLogger implements Logger {
 
   constructor(private readonly logToConsole: boolean = true) {}
 
-  private addMessage(level: LogLevel, message: LoggableError): void {
-    const joinedMessage = joinMessageStrings(message);
-    this.messages.push({ type: level, message: joinedMessage });
+  private addMessage(level: LogLevel, message: string | Error): void {
+    this.messages.push({ type: level, message });
 
     if (this.logToConsole) {
       // eslint-disable-next-line no-console
-      console.debug(joinedMessage);
+      console.debug(message);
     }
   }
 
@@ -685,19 +684,19 @@ export class RecordingLogger implements Logger {
     return true;
   }
 
-  debug(message: Loggable) {
+  debug(message: string) {
     this.addMessage("debug", message);
   }
 
-  info(message: Loggable) {
+  info(message: string) {
     this.addMessage("info", message);
   }
 
-  warning(message: LoggableError) {
+  warning(message: string | Error) {
     this.addMessage("warning", message);
   }
 
-  error(message: LoggableError) {
+  error(message: string | Error) {
     this.addMessage("error", message);
   }
 
@@ -1042,15 +1041,3 @@ export function makeTestToken(length: number = 36) {
     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
   return chars.repeat(Math.ceil(length / chars.length)).slice(0, length);
 }
-
-/**
- * Get a real, newly created CodeQL instance for testing. The instance refers to
- * a non-existent placeholder codeql command, so tests that use this function
- * should also stub the toolrunner.ToolRunner constructor.
- */
-export async function getCodeQLForTesting(
-  cmd = "codeql-for-testing",
-  logger: Logger = new RecordingLogger(),
-): Promise<codeql.CodeQL> {
-  return codeql.getCodeQLForCmd(logger, cmd, false);
-}

src/tools-download.test.ts

@@ -10,8 +10,9 @@ import { https } from "follow-redirects";
 import nock from "nock";
 import * as sinon from "sinon";
 
+import { getRunnerLogger } from "./logging";
 import * as tar from "./tar";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import { setupTests } from "./testing-utils";
 import { downloadAndExtract } from "./tools-download";
 import * as util from "./util";
 
@@ -33,7 +34,7 @@ test.serial(
         undefined,
         {},
         undefined,
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(Number.isInteger(statusReport.downloadDurationMs));
@@ -71,7 +72,7 @@ test.serial(
         undefined,
         {},
         { type: "gnu", version: "1.34" },
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(Number.isInteger(statusReport.downloadDurationMs));
@@ -104,7 +105,7 @@ test.serial(
           undefined,
           {},
           { type: "gnu", version: "1.34" },
-          new RecordingLogger(),
+          getRunnerLogger(true),
         ),
         {
           instanceOf: util.HTTPError,
@@ -148,7 +149,7 @@ test.serial(
         undefined,
         {},
         { type: "gnu", version: "1.34" },
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(Number.isInteger(statusReport.downloadDurationMs));
@@ -188,7 +189,7 @@ test.serial(
         undefined,
         {},
         { type: "gnu", version: "1.34" },
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.is(
@@ -236,7 +237,7 @@ test.serial(
         undefined,
         {},
         { type: "gnu", version: "1.34" },
-        new RecordingLogger(),
+        getRunnerLogger(true),
       );
 
       t.assert(Number.isInteger(statusReport.totalDurationMs));

src/tracer-config.test.ts

@@ -4,12 +4,11 @@ import * as path from "path";
 import test from "ava";
 import * as sinon from "sinon";
 
-import type { CodeQL } from "./codeql";
+import { CodeQL, getCodeQLForTesting } from "./codeql";
 import * as configUtils from "./config-utils";
 import { BuiltInLanguage } from "./languages";
 import {
   createTestConfig,
-  getCodeQLForTesting,
   makeVersionInfo,
   RecordingLogger,
   setupTests,

src/trap-caching.test.ts

@@ -16,12 +16,12 @@ import * as configUtils from "./config-utils";
 import { Feature } from "./feature-flags";
 import * as gitUtils from "./git-utils";
 import { BuiltInLanguage } from "./languages";
+import { getRunnerLogger } from "./logging";
 import {
   createFeatures,
   createTestConfig,
   getRecordingLogger,
   makeVersionInfo,
-  RecordingLogger,
   setupTests,
 } from "./testing-utils";
 import {
@@ -265,7 +265,7 @@ test.serial("cleanup removes only old CodeQL TRAP caches", async (t) => {
     const statusReport = await cleanupTrapCaches(
       config,
       createFeatures([Feature.CleanupTrapCaches]),
-      new RecordingLogger(),
+      getRunnerLogger(true),
     );
 
     t.is(listStub.callCount, 1);

src/upload-lib.test.ts

@@ -10,9 +10,9 @@ import * as analyses from "./analyses";
 import { AnalysisKind, CodeQuality, CodeScanning } from "./analyses";
 import * as api from "./api-client";
 import * as diffUtils from "./diff-informed-analysis-utils";
-import { Logger } from "./logging";
+import { getRunnerLogger, Logger } from "./logging";
 import * as sarif from "./sarif";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import { setupTests } from "./testing-utils";
 import * as uploadLib from "./upload-lib";
 import { UploadPayload } from "./upload-lib/types";
 import { GitHubVariant, initializeEnvironment, withTmpDir } from "./util";
@@ -29,7 +29,7 @@ test.serial("validateSarifFileSchema - valid", (t) => {
     uploadLib.validateSarifFileSchema(
       uploadLib.readSarifFileOrThrow(inputFile),
       inputFile,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     ),
   );
 });
@@ -40,7 +40,7 @@ test.serial("validateSarifFileSchema - invalid", (t) => {
     uploadLib.validateSarifFileSchema(
       uploadLib.readSarifFileOrThrow(inputFile),
       inputFile,
-      new RecordingLogger(),
+      getRunnerLogger(true),
     ),
   );
 });
@@ -179,7 +179,7 @@ test.serial("finding SARIF files", async (t) => {
     }
 
     const groupedSarifFiles = await uploadLib.getGroupedSarifFilePaths(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       tmpDir,
     );
 
@@ -200,7 +200,7 @@ test.serial("getGroupedSarifFilePaths - Risk Assessment files", async (t) => {
     fs.writeFileSync(sarifPath, "");
 
     const groupedSarifFiles = await uploadLib.getGroupedSarifFilePaths(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       sarifPath,
     );
 
@@ -218,7 +218,7 @@ test.serial("getGroupedSarifFilePaths - Code Quality file", async (t) => {
     fs.writeFileSync(sarifPath, "");
 
     const groupedSarifFiles = await uploadLib.getGroupedSarifFilePaths(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       sarifPath,
     );
 
@@ -236,7 +236,7 @@ test.serial("getGroupedSarifFilePaths - Code Scanning file", async (t) => {
     fs.writeFileSync(sarifPath, "");
 
     const groupedSarifFiles = await uploadLib.getGroupedSarifFilePaths(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       sarifPath,
     );
 
@@ -254,7 +254,7 @@ test.serial("getGroupedSarifFilePaths - Other file", async (t) => {
     fs.writeFileSync(sarifPath, "");
 
     const groupedSarifFiles = await uploadLib.getGroupedSarifFilePaths(
-      new RecordingLogger(),
+      getRunnerLogger(true),
       sarifPath,
     );
 
@@ -932,7 +932,7 @@ function uploadPayloadFixtures(analysis: analyses.AnalysisConfig) {
         owner: mockData.owner,
         repo: mockData.repo,
       },
-      new RecordingLogger(),
+      getRunnerLogger(true),
       analysis,
     );
 
@@ -1019,7 +1019,7 @@ function runFilterAlertsByDiffRange(
   diffRanges: diffUtils.DiffThunkRange[],
 ): Partial<sarif.Log> {
   sinon.stub(diffUtils, "readDiffRangesJsonFile").returns(diffRanges);
-  return uploadLib.filterAlertsByDiffRange(new RecordingLogger(), input);
+  return uploadLib.filterAlertsByDiffRange(getRunnerLogger(true), input);
 }
 
 test.serial(

src/upload-sarif.test.ts

@@ -5,12 +5,8 @@ import test, { ExecutionContext } from "ava";
 import * as sinon from "sinon";
 
 import { AnalysisKind, getAnalysisConfig } from "./analyses";
-import {
-  createFeatures,
-  makeMacro,
-  RecordingLogger,
-  setupTests,
-} from "./testing-utils";
+import { getRunnerLogger } from "./logging";
+import { createFeatures, makeMacro, setupTests } from "./testing-utils";
 import { UploadResult } from "./upload-lib";
 import * as uploadLib from "./upload-lib";
 import { postProcessAndUploadSarif } from "./upload-sarif";
@@ -55,7 +51,7 @@ const postProcessAndUploadSarifMacro = makeMacro({
     expectedResult: Partial<Record<AnalysisKind, UploadSarifExpectedResult>>,
   ) => {
     await util.withTmpDir(async (tempDir) => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const testPath = sarifPath(tempDir);
       const features = createFeatures([]);
 
@@ -210,7 +206,7 @@ test.serial(
   "postProcessAndUploadSarif doesn't upload if upload is disabled",
   async (t) => {
     await util.withTmpDir(async (tempDir) => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const features = createFeatures([]);
 
       const toFullPath = (filename: string) => path.join(tempDir, filename);
@@ -243,7 +239,7 @@ test.serial(
   "postProcessAndUploadSarif writes post-processed SARIF files if output directory is provided",
   async (t) => {
     await util.withTmpDir(async (tempDir) => {
-      const logger = new RecordingLogger();
+      const logger = getRunnerLogger(true);
       const features = createFeatures([]);
 
       const toFullPath = (filename: string) => path.join(tempDir, filename);

src/util.test.ts

@@ -10,7 +10,8 @@ import * as sinon from "sinon";
 
 import * as api from "./api-client";
 import { EnvVar } from "./environment";
-import { RecordingLogger, setupTests } from "./testing-utils";
+import { getRunnerLogger } from "./logging";
+import { setupTests } from "./testing-utils";
 import * as util from "./util";
 
 setupTests(test);
@@ -91,7 +92,7 @@ for (const {
 
 test("getMemoryFlag() throws if the ram input is < 0 or NaN", async (t) => {
   for (const input of ["-1", "hello!"]) {
-    t.throws(() => util.getMemoryFlag(input, new RecordingLogger()));
+    t.throws(() => util.getMemoryFlag(input, getRunnerLogger(true)));
   }
 });
 
@@ -108,13 +109,13 @@ test("getThreadsFlag() should return the correct --threads flag", (t) => {
   ];
 
   for (const [input, expectedFlag] of tests) {
-    const flag = util.getThreadsFlag(input, new RecordingLogger());
+    const flag = util.getThreadsFlag(input, getRunnerLogger(true));
     t.deepEqual(flag, expectedFlag);
   }
 });
 
 test("getThreadsFlag() throws if the threads input is not an integer", (t) => {
-  t.throws(() => util.getThreadsFlag("hello!", new RecordingLogger()));
+  t.throws(() => util.getThreadsFlag("hello!", getRunnerLogger(true)));
 });
 
 test.serial(
@@ -384,7 +385,7 @@ test("getCgroupCpuCountFromCpus calculates the number of CPUs correctly", async
     const testCpuFile = `${tmpDir}/cpus-file`;
     fs.writeFileSync(testCpuFile, "1, 9-10\n", "utf-8");
     t.deepEqual(
-      util.getCgroupCpuCountFromCpus(testCpuFile, new RecordingLogger()),
+      util.getCgroupCpuCountFromCpus(testCpuFile, getRunnerLogger(true)),
       3,
     );
   });
@@ -395,7 +396,7 @@ test("getCgroupCpuCountFromCpus returns undefined if the CPU file doesn't exist"
     const testCpuFile = `${tmpDir}/cpus-file`;
     t.false(fs.existsSync(testCpuFile));
     t.deepEqual(
-      util.getCgroupCpuCountFromCpus(testCpuFile, new RecordingLogger()),
+      util.getCgroupCpuCountFromCpus(testCpuFile, getRunnerLogger(true)),
       undefined,
     );
   });
@@ -406,7 +407,7 @@ test("getCgroupCpuCountFromCpus returns undefined if the CPU file exists but is
     const testCpuFile = `${tmpDir}/cpus-file`;
     fs.writeFileSync(testCpuFile, "\n", "utf-8");
     t.deepEqual(
-      util.getCgroupCpuCountFromCpus(testCpuFile, new RecordingLogger()),
+      util.getCgroupCpuCountFromCpus(testCpuFile, getRunnerLogger(true)),
       undefined,
     );
   });
@@ -416,7 +417,7 @@ test.serial(
   "checkDiskUsage succeeds and produces positive numbers",
   async (t) => {
     process.env["GITHUB_WORKSPACE"] = os.tmpdir();
-    const diskUsage = await util.checkDiskUsage(new RecordingLogger());
+    const diskUsage = await util.checkDiskUsage(getRunnerLogger(true));
     if (t.truthy(diskUsage)) {
       t.true(diskUsage.numAvailableBytes > 0);
       t.true(diskUsage.numTotalBytes > 0);

src/workflow.test.ts

@@ -3,11 +3,10 @@ import * as yaml from "js-yaml";
 import * as sinon from "sinon";
 
 import * as actionsUtil from "./actions-util";
-import { createStubCodeQL } from "./codeql";
+import { createStubCodeQL, getCodeQLForTesting } from "./codeql";
 import { ActionsEnvVars, EnvVar } from "./environment";
 import {
   checkExpectedLogMessages,
-  getCodeQLForTesting,
   createTestConfig,
   getRecordingLogger,
   getTestEnv,

unreleased-change-notes/README.md

@@ -2,6 +2,16 @@
 
 Change-notes are Markdown files used to document user-facing changes. When making a change that affects users, create a Markdown file here that describes the change. During the next release, the change-note files in `unreleased-change-notes/` will automatically be added to `CHANGELOG.md`.
 
+### What do document
+
+Some guidelines to help you decide whether a change needs a change note:
+
+- New functionality that is gated behind a feature flag should be documented when it is added, highlighting that the feature will only be rolled out later. Ideally, this should also include an estimate for when the feature is expected to become available.
+- When a feature flag is removed and the gated functionality becomes the default behaviour, this does not need to be documented.
+- Changes that may require users to make changes to their workflows must be documented.
+
+If in doubt, see `CHANGELOG.md` for what we have included previously.
+
 ### Change-note file format
 
 Change-note files must follow a certain format so that they can be automatically validated and processed. Failure to follow the format will result in a failed PR check.