diff --git a/src/config/file.test.ts b/src/config/file.test.ts index 3362d26c63..4b7b88c9ca 100644 --- a/src/config/file.test.ts +++ b/src/config/file.test.ts @@ -13,7 +13,7 @@ test("getConfigFileInput returns undefined by default", async (t) => { await callee(getConfigFileInput) .withArgs({}) .withFeatures([Feature.ConfigFileRepositoryProperty]) - .passes(async (fn) => t.is(await fn(), undefined)); + .passes(t.is, undefined); }); const repositoryProperties = { @@ -22,47 +22,29 @@ const repositoryProperties = { test("getConfigFileInput returns input value", async (t) => { const testInput = "/some/path"; - const target = callee(getConfigFileInput).withFeatures([ - Feature.ConfigFileRepositoryProperty, - ]); - - const actionsEnv = target.getState().actions; - sinon - .stub(actionsEnv, "getOptionalInput") - .withArgs("config-file") - .returns(testInput); // Even though both an input and repository property are configured, // we prefer the direct input to the Action. - const targetWithArgs = target - .withActions(actionsEnv) - .withArgs(repositoryProperties); - await targetWithArgs.passes(async (fn) => t.is(await fn(), testInput)); - - // Check for the expected log message. - t.true( - targetWithArgs - .getLogger() - .hasMessage("Using configuration file input from workflow"), - ); + await callee(getConfigFileInput) + .withFeatures([Feature.ConfigFileRepositoryProperty]) + .withActions((actionsEnv) => { + sinon + .stub(actionsEnv, "getOptionalInput") + .withArgs("config-file") + .returns(testInput); + }) + .withArgs(repositoryProperties) + .logs(t, "Using configuration file input from workflow") + .passes(t.is, testInput); }); test("getConfigFileInput returns repository property value", async (t) => { // Since there is no direct input, we should use the repository property. - const target = callee(getConfigFileInput) + await callee(getConfigFileInput) .withFeatures([Feature.ConfigFileRepositoryProperty]) - .withArgs(repositoryProperties); - - await target.passes(async (fn) => - t.is(await fn(), repositoryProperties[RepositoryPropertyName.CONFIG_FILE]), - ); - - // Check for the expected log message. - t.true( - target - .getLogger() - .hasMessage("Using configuration file input from repository property"), - ); + .withArgs(repositoryProperties) + .logs(t, "Using configuration file input from repository property") + .passes(t.is, repositoryProperties[RepositoryPropertyName.CONFIG_FILE]); }); test("getConfigFileInput ignores empty repository property value", async (t) => { @@ -70,27 +52,18 @@ test("getConfigFileInput ignores empty repository property value", async (t) => await callee(getConfigFileInput) .withFeatures([Feature.ConfigFileRepositoryProperty]) .withArgs({ [RepositoryPropertyName.CONFIG_FILE]: " " }) - .passes(async (fn) => t.is(await fn(), undefined)); + .passes(t.is, undefined); }); test("getConfigFileInput ignores repository property value when FF is off", async (t) => { // Since the FF is off, we should ignore the repository property value. - const target = callee(getConfigFileInput) + await callee(getConfigFileInput) .withFeatures([]) - .withArgs(repositoryProperties); - - await target.passes(async (fn) => t.is(await fn(), undefined)); - - t.false( - target - .getLogger() - .hasMessage("Using configuration file input from repository property"), - ); - t.true( - target - .getLogger() - .hasMessage( - "Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.", - ), - ); + .withArgs(repositoryProperties) + .notLogs(t, "Using configuration file input from repository property") + .logs( + t, + "Ignoring configuration file input from repository property, because the corresponding feature flag is disabled.", + ) + .passes(t.is, undefined); }); diff --git a/src/config/remote-file.test.ts b/src/config/remote-file.test.ts index ac1492d906..fd1fcf3c17 100644 --- a/src/config/remote-file.test.ts +++ b/src/config/remote-file.test.ts @@ -4,7 +4,7 @@ import sinon from "sinon"; import { ActionsEnvVars } from "../environment"; import * as errors from "../error-messages"; import { Feature } from "../feature-flags"; -import { callee, getTestEnv } from "../testing-utils"; +import { callee } from "../testing-utils"; import { ConfigurationError } from "../util"; import { @@ -50,7 +50,7 @@ test("parseRemoteFileAddress accepts full remote addresses", async (t) => { for (const oldFormatInput of oldFormatInputs) { await target .withArgs(oldFormatInput.input) - .passes(async (fn) => t.deepEqual(await fn(), oldFormatInput.expected)); + .passes(t.deepEqual, oldFormatInput.expected); } // New format. @@ -78,28 +78,23 @@ test("parseRemoteFileAddress accepts full remote addresses", async (t) => { // Should fail when the FF is not enabled. await targetWithArgs .withFeatures([]) - .passes(async (fn) => - t.throwsAsync(fn, { instanceOf: ConfigurationError }), - ); + .throws(t, { instanceOf: ConfigurationError }); // And pass when the FF is enabled. await targetWithArgs .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => t.deepEqual(await fn(), newFormatInput.expected)); + .passes(t.deepEqual, newFormatInput.expected); } }); test("parseRemoteFileAddress accepts remote address without an owner", async (t) => { - const target = callee(parseRemoteFileAddress); - - const env = target.getState().env; const owner = "test-owner"; - const getRequired = sinon.stub(env, "getRequired"); - getRequired - .withArgs(ActionsEnvVars.GITHUB_REPOSITORY) - .returns(`${owner}/current-repo`); - - const targetWithEnv = target.withEnv(env); + const target = callee(parseRemoteFileAddress).withEnv((env) => { + const getRequired = sinon.stub(env, "getRequired"); + getRequired + .withArgs(ActionsEnvVars.GITHUB_REPOSITORY) + .returns(`${owner}/current-repo`); + }); const testCases: ParseRemoteFileAddressTest[] = [ { @@ -141,33 +136,33 @@ test("parseRemoteFileAddress accepts remote address without an owner", async (t) ]; for (const testCase of testCases) { - const targetWithArgs = targetWithEnv.withArgs(testCase.input); + const targetWithArgs = target.withArgs(testCase.input); // Should fail when the FF is not enabled. await targetWithArgs .withFeatures([]) - .passes(async (fn) => - t.throwsAsync(fn, { instanceOf: ConfigurationError }), - ); + .throws(t, { instanceOf: ConfigurationError }); // And pass when the FF is enabled. await targetWithArgs .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => t.deepEqual(await fn(), testCase.expected)); + .passes(t.deepEqual, testCase.expected); } }); test("parseRemoteFileAddress throws for invalid `GITHUB_REPOSITORY`", async (t) => { - const target = callee(parseRemoteFileAddress).withArgs("repo@ref"); - - const env = target.getState().env; - const getRequired = sinon.stub(env, "getRequired"); + const getRequired: sinon.SinonStub = sinon.stub(); getRequired.withArgs(ActionsEnvVars.GITHUB_REPOSITORY).returns(`not-valid`); + const target = callee(parseRemoteFileAddress) + .withArgs("repo@ref") + .withEnv((env) => { + sinon.define(env, "getRequired", getRequired); + }); + await target - .withEnv(env) .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => t.throwsAsync(fn, { instanceOf: Error })); + .throws(t, { instanceOf: Error }); t.assert(getRequired.calledOnceWith(ActionsEnvVars.GITHUB_REPOSITORY)); }); @@ -202,14 +197,12 @@ test("parseRemoteFileAddress accepts remote address without a path", async (t) = // Should fail when the FF is not enabled. await targetWithArgs .withFeatures([]) - .passes(async (fn) => - t.throwsAsync(fn, { instanceOf: ConfigurationError }), - ); + .throws(t, { instanceOf: ConfigurationError }); // And pass when the FF is enabled. await targetWithArgs .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => t.deepEqual(await fn(), testCase.expected)); + .passes(t.deepEqual, testCase.expected); } }); @@ -217,28 +210,25 @@ test("parseRemoteFileAddress accepts remote address without a ref", async (t) => const target = callee(parseRemoteFileAddress).withArgs("owner/repo:path"); // Should only accept the input if the FF is enabled. - await target.withFeatures([]).passes(t.throwsAsync); + await target.withFeatures([]).throws(t); await target .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => - t.deepEqual(await fn(), { - owner: "owner", - repo: "repo", - path: "path", - ref: DEFAULT_CONFIG_FILE_REF, - } satisfies RemoteFileAddress), - ); + .passes(t.deepEqual, { + owner: "owner", + repo: "repo", + path: "path", + ref: DEFAULT_CONFIG_FILE_REF, + } satisfies RemoteFileAddress); }); test("parseRemoteFileAddress rejects invalid values", async (t) => { - const env = getTestEnv(); const owner = "owner"; - const getRequired = sinon.stub(env, "getRequired"); - getRequired - .withArgs(ActionsEnvVars.GITHUB_REPOSITORY) - .returns(`${owner}/current-repo`); - - const target = callee(parseRemoteFileAddress).withEnv(env); + const target = callee(parseRemoteFileAddress).withEnv((env) => { + const getRequired = sinon.stub(env, "getRequired"); + getRequired + .withArgs(ActionsEnvVars.GITHUB_REPOSITORY) + .returns(`${owner}/current-repo`); + }); const testInputs = [ " ", @@ -262,21 +252,17 @@ test("parseRemoteFileAddress rejects invalid values", async (t) => { const targetWithArgs = target.withArgs(testInput); // Should throw both when the new format is and isn't accepted. - await targetWithArgs.withFeatures([]).passes(async (fn) => - t.throwsAsync(fn, { - instanceOf: ConfigurationError, - message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput), - }), - ); + await targetWithArgs.withFeatures([]).throws(t, { + instanceOf: ConfigurationError, + message: errors.getConfigFileRepoOldFormatInvalidMessage(testInput), + }); await targetWithArgs .withFeatures([Feature.NewRemoteFileAddresses]) - .passes(async (fn) => - t.throwsAsync(fn, { - // When the new format is accepted, there are some more specific - // errors in some cases. It is sufficient for us to check that - // an exception is thrown. - instanceOf: ConfigurationError, - }), - ); + .throws(t, { + // When the new format is accepted, there are some more specific + // errors in some cases. It is sufficient for us to check that + // an exception is thrown. + instanceOf: ConfigurationError, + }); } }); diff --git a/src/environment.ts b/src/environment.ts index 6ea0e87e4f..80759d6d61 100644 --- a/src/environment.ts +++ b/src/environment.ts @@ -193,10 +193,12 @@ export enum ActionsEnvVars { GITHUB_SERVER_URL = "GITHUB_SERVER_URL", GITHUB_SHA = "GITHUB_SHA", GITHUB_WORKFLOW = "GITHUB_WORKFLOW", + GITHUB_WORKSPACE = "GITHUB_WORKSPACE", RUNNER_ENVIRONMENT = "RUNNER_ENVIRONMENT", RUNNER_NAME = "RUNNER_NAME", RUNNER_OS = "RUNNER_OS", RUNNER_TEMP = "RUNNER_TEMP", + RUNNER_TOOL_CACHE = "RUNNER_TOOL_CACHE", } /** A type representing all known environment variables. */ diff --git a/src/testing-utils.ts b/src/testing-utils.ts index ec518853f8..60e8fb5540 100644 --- a/src/testing-utils.ts +++ b/src/testing-utils.ts @@ -3,6 +3,8 @@ import path from "path"; import * as github from "@actions/github"; import test, { + type ThrownError, + type ThrowsExpectation, type ExecutionContext, type MacroDeclarationOptions, type TestFn, @@ -208,35 +210,50 @@ export function initAllState( }; } +type DelayedCheck< + Args extends readonly any[], + R, + Fs extends ReadonlyArray, +> = (env: Readonly>) => Promise; + +export type ValueOrMutation = T | ((val: T) => void); + /** * Wraps a function that accepts an `ActionState` for testing in different environments. */ -export class TestEnv< +abstract class BaseEnvBuilder< Args extends readonly any[], R, Fs extends ReadonlyArray, > { - private readonly fn: (state: ActionState, ...args: Args) => R; - private args?: Args; + protected readonly fn: (state: ActionState, ...args: Args) => R; private logger: RecordingLogger; - private state: ActionState; + protected state: ActionState; + protected checks: Array>; constructor( fn: (state: ActionState, ...args: Args) => R, - cloneFrom?: TestEnv, + cloneFrom?: BaseEnvBuilder, ) { this.fn = fn; - this.args = cloneFrom?.args; this.logger = new RecordingLogger(); this.state = cloneFrom !== undefined - ? { ...cloneFrom.state, logger: this.logger } + ? ({ + ...cloneFrom.state, + env: Object.create(cloneFrom.state.env), + actions: Object.create(cloneFrom.state.actions), + logger: this.logger, + } satisfies ActionState) : initAllState({ logger: this.logger }); + this.checks = [...(cloneFrom?.checks ?? [])]; } - private clone(): TestEnv { - return new TestEnv(this.fn, this); - } + /** + * Creates a clone of this object. Used internally. + * Must be overridden by subclasses. + */ + protected abstract clone(): this; public getLogger(): RecordingLogger { return this.logger; @@ -246,48 +263,181 @@ export class TestEnv< return this.state; } - public getArgs(): Args | undefined { - return this.args; + public withArgs(...args: Args): CallableEnvBuilder { + const result = new CallableEnvBuilder(this.fn, args, this.clone()); + return result; } - public withArgs(...args: Args) { + public withFeatures(enabled: Feature[]): this { const result = this.clone(); - result.args = args; + result.state.features = createFeatures(enabled); return result; } - public withFeatures(enabled: Feature[]): TestEnv { + /** + * Sets environment variables that are always available to GitHub Actions, + * excluding some that are expected to be set to paths. + * + * @param overrides Overrides for the defaults. + */ + public withDefaultActionsEnv(overrides?: ActionVarOverrides): this { const result = this.clone(); - result.state.features = createFeatures(enabled); + setupBaseActionsVars(overrides, result.state.env); return result; } - public withEnv(env: Env): TestEnv { + /** + * Sets environment variables that are always available to GitHub Actions. + * @param tempDir A value for `RUNNER_TEMP` and `GITHUB_WORKSPACE`. + * @param toolsDir A value for `RUNNER_TOOL_CACHE`. + * @param overrides Overrides for the defaults. + */ + public withActionsEnv( + tempDir: string, + toolsDir: string, + overrides?: ActionVarOverrides, + ): this { const result = this.clone(); - result.state.env = env; + setupActionsVars(tempDir, toolsDir, overrides, result.state.env); return result; } - public withActions(actions: ActionsEnv): TestEnv { + public withEnv(arg: ValueOrMutation): this { const result = this.clone(); - result.state.actions = actions; + if (typeof arg === "function") { + arg(result.state.env); + } else { + result.state.env = arg; + } return result; } - call(): R { - if (!this.args) { - throw new Error("Trying to call function in TestEnv without arguments."); + public withActions(arg: ValueOrMutation): this { + const result = this.clone(); + if (typeof arg === "function") { + arg(result.state.actions); + } else { + result.state.actions = arg; } - return this.fn(this.state as unknown as ActionState, ...this.args); + return result; } - public passes( - assertion: (makeCall: () => R) => T | Promise, - ): T | Promise { - return assertion(() => { - const result = this.call(); - return result; + /** + * Adds a delayed check that `messages` are logged. The check will be + * performed after the main assertion passes. + */ + public logs(t: ExecutionContext, ...messages: string[]): this { + const result = this.clone(); + result.checks.push(async (env) => { + checkExpectedLogMessages(t, env.getLogger().messages, messages); + }); + return result; + } + + /** + * Adds a delayed check that `messages` are not logged. The check will be + * performed after the main assertion passes. + */ + public notLogs(t: ExecutionContext, ...messages: string[]): this { + const result = this.clone(); + result.checks.push(async (env) => { + checkUnexpectedLogMessages(t, env.getLogger().messages, messages); }); + return result; + } +} + +class EnvBuilder< + Args extends readonly any[], + R, + Fs extends ReadonlyArray, +> extends BaseEnvBuilder { + protected clone(): this { + return new EnvBuilder(this.fn, this) as this; + } +} + +class CallableEnvBuilder< + Args extends readonly any[], + R, + Fs extends ReadonlyArray, +> extends BaseEnvBuilder { + private args: Args; + + constructor( + fn: (state: ActionState, ...args: Args) => R, + args: Args, + cloneFrom?: BaseEnvBuilder, + ) { + super(fn, cloneFrom); + this.args = args; + } + + protected clone(): this { + return new CallableEnvBuilder(this.fn, this.args, this) as this; + } + + public getArgs(): Args { + return this.args; + } + + call(): R { + return this.fn(this.state as unknown as ActionState, ...this.args); + } + + /** + * Calls the underlying function in the configured environment and passes + * the result to `assertion` along with extra `assertionArgs`. + * + * @param assertion The assertion to apply to the result. + * @param assertionArgs Extra arguments for the assertion. + * @returns The result of the assertion. + */ + public async passes( + assertion: (val: Awaited, ...assertionArgs: AArgs) => AResult, + ...assertionArgs: AArgs + ): Promise { + // this.call() may or may not return a promise, + // `Promise.resolve` turns the result into one if it isn't already, + // and we then await it. That ensures that `result` is an `Awaited`. + const result = await Promise.resolve(this.call()); + + // Run the main assertion on the `result`. + const assertionResult = await assertion(result, ...assertionArgs); + + // Run other delayed checks. + for (const delayedCheck of this.checks) { + await delayedCheck(this); + } + + // Return the result of the main assertion. + return assertionResult; + } + + /** + * Asserts that calling the underlying function should throw an exception. + * + * @param t The execution context for the assertion. + * @param expectations Expectations for the error. + * @returns The error that was thrown. + */ + public async throws( + t: ExecutionContext, + expectations?: ThrowsExpectation, + ): Promise> { + // Run the main assertion. + const error = await t.throwsAsync( + async () => Promise.resolve(this.call()), + expectations, + ); + + // Run other delayed checks. + for (const delayedCheck of this.checks) { + await delayedCheck(this); + } + + // Return the error. + return error; } } @@ -296,8 +446,8 @@ export function callee< Args extends readonly any[], R, Fs extends readonly StateFeature[], ->(fn: (state: ActionState, ...args: Args) => R): TestEnv { - return new TestEnv(fn); +>(fn: (state: ActionState, ...args: Args) => R): EnvBuilder { + return new EnvBuilder(fn); } /** @@ -349,16 +499,18 @@ export function setupBaseActionsVars( * @param tempDir A value for `RUNNER_TEMP` and `GITHUB_WORKSPACE`. * @param toolsDir A value for `RUNNER_TOOL_CACHE`. * @param overrides Overrides for the defaults. + * @param env The environment to set the variables for. */ export function setupActionsVars( tempDir: string, toolsDir: string, overrides?: ActionVarOverrides, + env: Env = getEnv(), ) { - setupBaseActionsVars(overrides); - process.env["RUNNER_TEMP"] = tempDir; - process.env["RUNNER_TOOL_CACHE"] = toolsDir; - process.env["GITHUB_WORKSPACE"] = tempDir; + setupBaseActionsVars(overrides, env); + env.set(ActionsEnvVars.RUNNER_TEMP, tempDir); + env.set(ActionsEnvVars.RUNNER_TOOL_CACHE, toolsDir); + env.set(ActionsEnvVars.GITHUB_WORKSPACE, tempDir); } type LogLevel = "debug" | "info" | "warning" | "error"; @@ -492,6 +644,34 @@ export function checkExpectedLogMessages( } } +/** + * Checks that `messages` contains none of `unexpectedMessages`. + */ +export function checkUnexpectedLogMessages( + t: ExecutionContext, + 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(); + } +} + /** * Asserts that `message` should not have been logged to `logger`. */