GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Remove `getRunnerLogger` by mbg · Pull Request #4235 · github/codeql-action

Conversation

@mbg

@mbg mbg commented Oct 14, 2026

Copy link
Copy Markdown
Member

Follow-up to #4198 (comment), which cherry-picks the commit with the Logger improvement and addresses the getRunnerLogger point.

The getRunnerLogger function is only used in tests where it is largely superseded by getRecordingLogger / the RecordingLogger class. The Logger returned by getRunnerLogger outputs log messages to stdout. In setupTests, we install test hooks which ensure that all stdout/stderr output is captured and printed if a test fails.

The recording logger is able to both record log messages and output them to stdout. The former is useful for assertions while the latter mirrors the behaviour of getRecordingLogger. While RecordingLogger always uses console.debug to output to stdout, this doesn't matter because we capture the output with the hooks installed by setupTests. The debug-level filter of getRunnerLogger was only used by one file and not practically useful -- the tests still pass and the increased output isn't an issue if tests fail. This PR replaces getRunnerLogger with new RecordingLogger.

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

  • Dotcom - Impacts CodeQL workflows on github.com and/or GitHub Enterprise Cloud with Data Residency.
  • GHES - Impacts CodeQL workflows on GitHub Enterprise Server.

How did/will you validate this change?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

N/A

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
mbg added 3 commits October 14, 2026 14:13
@mbg
Allow logging functions to accept string[] directly
4a2ca5c
@mbg
Move getCodeQLForTesting to testing-utils.ts
924990a
@mbg
Remove getRunnerLogger
23d418c
Copilot AI balanced review requested due to automatic review settings October 14, 2026 14:05
@mbg
mbg requested a review from a team as a code owner October 14, 2026 14:05
Copilot started reviewing on behalf of mbg October 14, 2026 14:06
View session
@mbg mbg mentioned this pull request Oct 14, 2026
Use setup-codeql logic for finding release assets in start-proxy #4198
Merged
@github-actions github-actions Bot added the size/XL May be very hard to review label Oct 14, 2026
Copilot AI reviewed Oct 14, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

🟡 Changes recommended

RecordingLogger formats array messages differently in its recorded and console outputs.

1 open finding
  • Medium severity Normalize string arrays before console.debug logging · New
What changed in this PR

Removes the test-only getRunnerLogger, standardizing tests on RecordingLogger.

Changes:

  • Adds array-message normalization to the logging interface.
  • Moves getCodeQLForTesting into testing utilities.
  • Updates affected tests and helpers to use RecordingLogger.
File Description
lib/​entry-points.js Excluded generated file; contents unavailable for review.
src/​logging.ts Removes getRunnerLogger and supports array messages.
src/​logging.test.ts Tests message normalization.
src/​testing-utils.ts Extends RecordingLogger and adds the CodeQL test helper.
src/​codeql.ts Exposes the underlying CodeQL factory and removes its test helper.
src/​codeql.test.ts Uses relocated test utilities.
src/​workflow.test.ts Uses relocated CodeQL test helper.
src/​tracer-config.test.ts Uses relocated CodeQL test helper.
src/​init-action-post-helper.test.ts Uses new test logger and CodeQL helper.
src/​feature-flags/​testing-util.ts Defaults feature tests to RecordingLogger.
src/​actions-util.test.ts Replaces runner logger.
src/​analyses.test.ts Replaces runner logger.
src/​analyze.test.ts Replaces runner logger.
src/​artifact-scanner.test.ts Replaces runner logger.
src/​cli/​output-cache.test.ts Replaces runner logger.
src/​config-utils.test.ts Replaces runner logger.
src/​config/​db-config.test.ts Replaces runner logger.
src/​diff-informed-analysis-utils.test.ts Replaces runner logger.
src/​feature-flags/​properties.test.ts Replaces runner logger.
src/​fingerprints.test.ts Replaces runner logger.
src/​overlay/​caching.test.ts Replaces runner logger.
src/​overlay/​index.test.ts Replaces runner logger.
src/​setup-codeql.test.ts Replaces runner logger.
src/​start-proxy.test.ts Replaces runner logger.
src/​status-report.test.ts Replaces runner logger.
src/​tar.test.ts Replaces runner logger.
src/​tools-download.test.ts Replaces runner logger.
src/​trap-caching.test.ts Replaces runner logger.
src/​upload-lib.test.ts Replaces runner logger.
src/​upload-sarif.test.ts Replaces runner logger.
src/​util.test.ts Replaces runner logger.
Files excluded by content exclusion policy (1)
  • lib/entry-points.js

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/testing-utils.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

github-advanced-security[bot]
github-advanced-security Bot found potential problems Oct 14, 2026
View reviewed changes
Comment thread src/testing-utils.ts Dismissed

Uh oh!

There was an error while loading. Please reload this page.

mbg added 2 commits October 14, 2026 15:11
@mbg
Ensure consistent output formatting in addMessage
7d7a550
@mbg
Merge branch 'main' into mbg/deprecate-getRunnerLogger
2b128fe
@mbg mbg mentioned this pull request Oct 16, 2026
Remove TRAP cache cleanup #4243
Merged
@mbg
Merge branch 'main' into mbg/deprecate-getRunnerLogger
6cf1931
henrymercer
henrymercer previously approved these changes Oct 16, 2026
View reviewed changes
Comment thread src/logging.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.

@mbg
Add JSDocs for Logger logging functions
8bb7f8f
@mbg
mbg dismissed henrymercer’s stale review via 8bb7f8f October 16, 2026 15:28
@mbg
mbg requested a review from henrymercer October 16, 2026 15:28
@mbg
Merge branch 'main' into mbg/deprecate-getRunnerLogger
16aecb7
henrymercer
henrymercer previously approved these changes Oct 21, 2026
View reviewed changes
@mbg
mbg enabled auto-merge October 21, 2026 16:02
@mbg
mbg added this pull request to the merge queue Oct 21, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 21, 2026
@mbg
Merge branch 'main' into mbg/deprecate-getRunnerLogger
487ba2e
@mbg
mbg dismissed henrymercer’s stale review via 487ba2e October 21, 2026 18:30
@mbg
mbg requested a review from henrymercer October 21, 2026 18:31
@mbg
mbg enabled auto-merge October 22, 2026 09:22
henrymercer
henrymercer approved these changes Oct 22, 2026
View reviewed changes
@mbg
mbg added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit a50b39a Oct 22, 2026
226 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mbg
mbg deleted the mbg/deprecate-getRunnerLogger branch October 22, 2026 10:34
This was referenced Nov 3, 2026
Merge main into releases/v4 #4286
Merged
Merge releases/v4 into releases/v3 #4291
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@henrymercer henrymercer henrymercer approved these changes
+1 more reviewer
@github-advanced-security github-advanced-security[bot] github-advanced-security[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

size/XL May be very hard to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@mbg @henrymercer @github-advanced-security
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.