GeezerVison

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

GeezerVison is showing github.com — Improve `TestEnv` for cleaner tests by mbg · Pull Request #4005 · github/codeql-action

Conversation

mbg commented Jul 9, 2026

Copy link

Copy Markdown

Member

Various changes to the test-only TestEnv that was introduced recently as part of #3990 to make tests nicer to write.

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:

Testing/None - This change does not impact any CodeQL workflows in production.

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?

Other - 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 self-assigned this

Jul 9, 2026

Copilot AI review requested due to automatic review settings

July 9, 2026 18:34

mbg requested a review from a team as a code owner

July 9, 2026 18:34

github-actions Bot added the size/L May be hard to review label

Jul 9, 2026

Copilot started reviewing on behalf of mbg

July 9, 2026 18:34

View session

Copilot AI reviewed

Jul 9, 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.

Pull request overview

This PR refactors the test-only TestEnv helper in src/testing-utils.ts into a builder-style API that enables cleaner test chaining and adds delayed log assertions, then updates existing config tests to use the new API.

Changes:

Replaced TestEnv with EnvBuilder/CallableEnvBuilder and updated callee(...) to return the new builder.
Added delayed log assertion helpers (logs, notLogs) and a throws(...) helper for more concise tests.
Updated src/config/*.test.ts tests to use .passes(t.is, ...), .throws(t, ...), and the new env/actions mutation helpers.

Show a summary per file

File	Description
src/testing-utils.ts	Introduces the new builder-based test harness, delayed checks, and new assertion helpers.
src/config/remote-file.test.ts	Updates tests to the new `callee(...).withEnv(...).withArgs(...).passes/throws` chaining style.
src/config/file.test.ts	Simplifies config-file input tests using delayed log assertions and the new `.passes(...)` signature.

Review details

Files reviewed: 3/3 changed files
Comments generated: 4
Review effort level: Low

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.

Comment thread

src/testing-utils.ts Resolved

Uh oh!

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

Comment thread

src/testing-utils.ts Resolved

Uh oh!

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

Comment thread

src/testing-utils.ts Resolved

Uh oh!

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

mbg force-pushed the mbg/test-env/improvements branch from 6109575 to d2e6ffe Compare

July 9, 2026 19:02

mbg added 12 commits

July 9, 2026 20:03


          Simplify return types for passes

31842a7


          Clean-up passes by splitting it into passes and throws

b571644


          Rename TestEnv to EnvBuilder

a8ac1e0


          Split EnvBuilder into it + CallableEnvBuilder

a4319e3


          Add abstract BaseEnvBuilder to enforce that clone is overriden by…

6a78290

… subclasses


          Simplify checking logs

1a57d7a


          Allow withActions and withEnv to mutate values

c77051d


          Accept Env for setupActionsVars

ab6fd52

Also extract env var strings into `ActionsEnvVars`


          Add action vars helpers to BaseEnvBuilder

da75ca5


          Fix typo

6b3ba17


          Add satisfies for state in BaseEnvBuilder constructor

a1e3f70


          Clone env and actions so that sinon stubs survive

d0a69d7

mbg force-pushed the mbg/test-env/improvements branch from d2e6ffe to d0a69d7 Compare

July 9, 2026 19:03

mbg requested a review from Copilot

July 9, 2026 19:04

Copilot started reviewing on behalf of mbg

July 9, 2026 19:04

View session

Copilot AI reviewed

Jul 9, 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.

Review details

Files reviewed: 4/4 changed files
Comments generated: 3
Review effort level: Low

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.

Comment thread

src/testing-utils.ts Resolved

Uh oh!

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

Comment thread

src/config/remote-file.test.ts Resolved

Uh oh!

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

mbg added 2 commits

July 9, 2026 22:40


          fixup! Add action vars helpers to BaseEnvBuilder

d8866c2


          Make throwsAsync arg async

9a9ab4c

mbg mentioned this pull request

Jul 9, 2026

Allow merging Default Setup config with config file #3994

Merged

mario-campos approved these changes

Jul 10, 2026

View reviewed changes

mario-campos 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.

Nice change with the Builder pattern 👍

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/testing-utils.ts

Comment on lines +647 to +674

		/**
		* Checks that `messages` contains none of `unexpectedMessages`.
		*/
		export function checkUnexpectedLogMessages(
		t: ExecutionContext<any>,
		messages: LoggedMessage[],
		unexpectedMessages: string[],
		) {
		const presentMessages: string[] = [];

		for (const unexpectedMessage of unexpectedMessages) {
		if (hasLoggedMessage(messages, unexpectedMessage)) {
		presentMessages.push(unexpectedMessage);
		}
		}

		if (presentMessages.length > 0) {
		const listify = (lines: string[]) =>
		lines.map((m) => ` - '${m}'`).join("\n");

		t.fail(
		`Did not expect\n\n${listify(presentMessages)}\n\nin the logger output, but found them in:\n\n${messages.map((m) => ` - '${m.message}'`).join("\n")}`,
		);
		} else {
		t.pass();
		}
		}

mario-campos Jul 10, 2026

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.

checkUnexpectedLogMessages seems similar enough to checkExpectedLogMessages that I would prefer that checkExpectedLogMessages be generalized to handle both use-cases. But, I don't immediately see a way to do that, without making the code less readable. So, I think it's fine.

Sorry, something went wrong.

Uh oh!

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

mbg reacted with thumbs up emoji

All reactions

👍 1 reaction

mbg added this pull request to the merge queue

Jul 11, 2026

Merged via the queue into main with commit 8096e4f

Jul 11, 2026

225 checks passed

Uh oh!

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

mbg deleted the mbg/test-env/improvements branch

July 11, 2026 05:38

This was referenced Jul 16, 2026

Merge main into releases/v4 #4020

Merged

Merge releases/v4 into releases/v3 #4022

Merged

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

mario-campos

mario-campos approved these changes

Uh oh!

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

Assignees

mbg

Labels

size/L

May be 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.

3 participants

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.