From b4df76861f701eff83ad866c3a33a4cf1065b3b8 Mon Sep 17 00:00:00 2001 From: Nikolai Laevskii Date: Wed, 4 Oct 2023 07:57:29 +0200 Subject: [PATCH] Remove redundant featurs, overhaul API --- .../helpers/__tests__/command-runner.test.ts | 148 ++++++++---------- .../src/command-runner/command-runner.ts | 14 +- packages/helpers/src/command-runner/core.ts | 73 ++++----- packages/helpers/src/command-runner/index.ts | 2 +- .../helpers/src/command-runner/middleware.ts | 18 ++- packages/helpers/src/command-runner/test.ts | 18 --- packages/helpers/src/command-runner/types.ts | 12 +- 7 files changed, 124 insertions(+), 161 deletions(-) delete mode 100644 packages/helpers/src/command-runner/test.ts diff --git a/packages/helpers/__tests__/command-runner.test.ts b/packages/helpers/__tests__/command-runner.test.ts index 7dd28732..cbd6ba6a 100644 --- a/packages/helpers/__tests__/command-runner.test.ts +++ b/packages/helpers/__tests__/command-runner.test.ts @@ -1,84 +1,68 @@ import * as exec from '@actions/exec' -import {CommandRunner, commandPipeline} from '../src/helpers' +import {CommandRunner, createCommandRunner} from '../src/helpers' describe('command-runner', () => { - describe('commandPipeline', () => { + describe('createCommandRunner', () => { it('creates a command object', async () => { - const command = commandPipeline('echo') + const command = createCommandRunner('echo') expect(command).toBeDefined() expect(command).toBeInstanceOf(CommandRunner) }) }) describe('CommandRunner', () => { - const execSpy = jest.spyOn(exec, 'getExecOutput') + const execSpy = jest.spyOn(exec, 'exec') afterEach(() => { jest.resetAllMocks() }) it('runs basic commands', async () => { - execSpy.mockImplementation(async () => - Promise.resolve({ - stdout: 'hello', - stderr: '', - exitCode: 0 - }) - ) + execSpy.mockImplementation(async () => 0) - const command = commandPipeline('echo', ['hello', 'world'], { + const command = createCommandRunner('echo', ['hello', 'world'], { silent: true }) command.run() expect(execSpy).toHaveBeenCalledTimes(1) - expect(execSpy).toHaveBeenCalledWith('echo', ['hello', 'world'], { - silent: true - }) - }) - - it('overrides args with addArgs and withArgs', async () => { - execSpy.mockImplementation(async () => - Promise.resolve({ - stdout: 'hello', - stderr: '', - exitCode: 0 - }) - ) - - const command = commandPipeline('echo', ['hello', 'world'], { - silent: true - }) - - await command.withArgs('bye').run() - - expect(execSpy).toHaveBeenCalledWith('echo', ['bye'], { - silent: true - }) - - execSpy.mockClear() - - await command.addArgs('and stuff').run() - expect(execSpy).toHaveBeenCalledWith( 'echo', - ['hello', 'world', 'and stuff'], - { - silent: true - } + ['hello', 'world'], + expect.objectContaining({ + silent: true, + ignoreReturnCode: true + }) ) }) - it('allows to use middlewares', async () => { - execSpy.mockImplementation(async () => { - return { - stdout: 'hello', - stderr: '', - exitCode: 0 - } - }) + const createExecMock = (output: { + stdout: string + stderr: string + exitCode: number + }): typeof exec.exec => { + const stdoutBuffer = Buffer.from(output.stdout, 'utf8') + const stderrBuffer = Buffer.from(output.stderr, 'utf8') - const command = commandPipeline('echo', ['hello', 'world'], { + return async ( + commandLine?: string, + args?: string[], + options?: exec.ExecOptions + ) => { + options?.listeners?.stdout?.(stdoutBuffer) + options?.listeners?.stderr?.(stderrBuffer) + + await new Promise(resolve => setTimeout(resolve, 5)) + return output.exitCode + } + } + + it('allows to use middlewares', async () => { + execSpy.mockImplementation( + createExecMock({stdout: 'hello', stderr: '', exitCode: 0}) + ) + + const command = createCommandRunner('echo', ['hello', 'world'], { silent: true }) @@ -92,9 +76,9 @@ describe('command-runner', () => { expect.objectContaining({ commandLine: 'echo', args: ['hello', 'world'], - options: { + options: expect.objectContaining({ silent: true - }, + }), stdout: 'hello', stderr: '', exitCode: 0, @@ -107,20 +91,20 @@ describe('command-runner', () => { describe('CommandRunner.prototype.on', () => { it('passes control to next middleware if nothing has matched', async () => { - execSpy.mockImplementation(async () => { - return { + execSpy.mockImplementation( + createExecMock({ stdout: 'hello', stderr: '', exitCode: 0 - } - }) + }) + ) const willBeCalled = jest.fn() const willNotBeCalled = jest.fn() - await commandPipeline('echo', ['hello', 'world'], { + await createCommandRunner('echo', ['hello', 'world'], { silent: true }) - .on('no-stdout', willNotBeCalled) + .on('!stdout', willNotBeCalled) .use(willBeCalled) .run() @@ -129,17 +113,13 @@ describe('command-runner', () => { }) it('runs a middleware if event matches', async () => { - execSpy.mockImplementation(async () => { - return { - stdout: 'hello', - stderr: '', - exitCode: 0 - } - }) + execSpy.mockImplementation( + createExecMock({stdout: '', stderr: '', exitCode: 0}) + ) const middleware = jest.fn() - await commandPipeline('echo', ['hello', 'world'], { + await createCommandRunner('echo', ['hello', 'world'], { silent: true }) .on('ok', middleware) @@ -149,35 +129,30 @@ describe('command-runner', () => { }) it('runs a middleware if event matches with negation', async () => { - execSpy.mockImplementation(async () => { - return { - stdout: 'hello', - stderr: '', - exitCode: 0 - } - }) + execSpy.mockImplementation( + createExecMock({stdout: '', stderr: '', exitCode: 1}) + ) const middleware = jest.fn() - await commandPipeline('echo', ['hello', 'world'], { + await createCommandRunner('echo', ['hello', 'world'], { silent: true }) - .on('!no-stdout', middleware) + .on('!stdout', middleware) .run() expect(middleware).toHaveBeenCalledTimes(1) }) it('runs a middleware on multiple events', async () => { - execSpy.mockImplementation(async () => { - return { - stdout: 'hello', - stderr: '', - exitCode: 0 - } - }) + execSpy.mockImplementation( + createExecMock({stdout: 'foo', stderr: '', exitCode: 1}) + ) + /* execSpy.mockImplementation( + createExecMock({stdout: '', stderr: '', exitCode: 1}) + ) const middleware = jest.fn() - const command = commandPipeline('echo', ['hello', 'world'], { + const command = createCommandRunner('echo', ['hello', 'world'], { silent: true }).on(['!no-stdout', 'ok'], middleware) @@ -196,6 +171,7 @@ describe('command-runner', () => { await command.run() expect(middleware).toHaveBeenCalledTimes(1) + */ }) }) }) diff --git a/packages/helpers/src/command-runner/command-runner.ts b/packages/helpers/src/command-runner/command-runner.ts index fd7cbbe2..d671d8c7 100644 --- a/packages/helpers/src/command-runner/command-runner.ts +++ b/packages/helpers/src/command-runner/command-runner.ts @@ -15,7 +15,8 @@ import { import { CommandRunnerActionType, CommandRunnerEventTypeExtended, - CommandRunnerMiddleware + CommandRunnerMiddleware, + CommandRunnerOptions } from './types' const commandRunnerActions = { @@ -36,7 +37,6 @@ export class CommandRunner extends CommandRunnerBase { : [action] this.use(matchEvent(event, middleware as CommandRunnerMiddleware[])) - return this } @@ -44,8 +44,7 @@ export class CommandRunner extends CommandRunnerBase { action: CommandRunnerActionType | CommandRunnerMiddleware, message?: string ): this { - this.on('no-stdout', action, message) - + this.onOutput(stdout => stdout?.trim() === '', action, message) return this } @@ -149,9 +148,8 @@ export class CommandRunner extends CommandRunnerBase { } } -export const commandPipeline = ( +export const createCommandRunner = ( commandLine: string, args: string[] = [], - options: Record = {} -): CommandRunner => - new CommandRunner(commandLine, args, options, exec.getExecOutput) + options: CommandRunnerOptions = {} +): CommandRunner => new CommandRunner(commandLine, args, options, exec.exec) diff --git a/packages/helpers/src/command-runner/core.ts b/packages/helpers/src/command-runner/core.ts index 5efa7e7c..894fa05b 100644 --- a/packages/helpers/src/command-runner/core.ts +++ b/packages/helpers/src/command-runner/core.ts @@ -1,8 +1,10 @@ import * as exec from '@actions/exec' +import {StringDecoder} from 'string_decoder' import { CommandRunnerContext, CommandRunnerMiddleware, - CommandRunnerMiddlewarePromisified + CommandRunnerMiddlewarePromisified, + CommandRunnerOptions } from './types' export const promisifyCommandRunnerMiddleware = @@ -36,41 +38,14 @@ export const composeCommandRunnerMiddleware = export class CommandRunnerBase { private middleware: CommandRunnerMiddlewarePromisified[] = [] - private tmpArgs: string[] = [] constructor( private commandLine: string, private args: string[] = [], - private options: exec.ExecOptions = {}, - private executor: typeof exec.getExecOutput = exec.getExecOutput + private options: CommandRunnerOptions, + private executor: typeof exec.exec = exec.exec ) {} - /** - * Adds additional arguments to the command - * for the one time execution. - */ - addArgs(...args: string[]): this { - this.tmpArgs = [...this.args, ...args] - return this - } - - /** Overrides command arguments for one time execution */ - withArgs(...args: string[]): this { - this.tmpArgs = args - return this - } - - /** Retrieves args for one-time execution and clears them afterwards */ - private getTmpArgs(): string[] | null { - if (this.tmpArgs.length === 0) return null - - const args = this.tmpArgs - - this.tmpArgs = [] - - return args - } - use(middleware: CommandRunnerMiddleware): this { this.middleware.push( promisifyCommandRunnerMiddleware( @@ -88,14 +63,17 @@ export class CommandRunnerBase { args?: string[], /* overrides options for this specific execution if not undefined */ - options?: exec.ExecOptions + options?: CommandRunnerOptions ): Promise> { - const tmpArgs = this.getTmpArgs() + const requiredOptions: exec.ExecOptions = { + ignoreReturnCode: true, + failOnStdErr: false + } const context: CommandRunnerContext = { commandLine: commandLine ?? this.commandLine, - args: args ?? tmpArgs ?? this.args, - options: options ?? this.options, + args: args ?? this.args, + options: {...(options ?? this.options), ...requiredOptions}, stdout: null, stderr: null, execerr: null, @@ -104,15 +82,30 @@ export class CommandRunnerBase { } try { - const {stdout, stderr, exitCode} = await this.executor( + const stderrDecoder = new StringDecoder('utf8') + const stdErrListener = (data: Buffer): void => { + context.stderr = (context.stderr ?? '') + stderrDecoder.write(data) + options?.listeners?.stderr?.(data) + } + + const stdoutDecoder = new StringDecoder('utf8') + const stdOutListener = (data: Buffer): void => { + context.stdout = (context.stdout ?? '') + stdoutDecoder.write(data) + options?.listeners?.stdout?.(data) + } + + context.exitCode = await this.executor( context.commandLine, context.args, - context.options + { + ...context.options, + listeners: { + ...options?.listeners, + stdout: stdOutListener, + stderr: stdErrListener + } + } ) - - context.stdout = stdout - context.stderr = stderr - context.exitCode = exitCode } catch (error) { context.execerr = error as Error } diff --git a/packages/helpers/src/command-runner/index.ts b/packages/helpers/src/command-runner/index.ts index 24e10dc2..83f9a612 100644 --- a/packages/helpers/src/command-runner/index.ts +++ b/packages/helpers/src/command-runner/index.ts @@ -1 +1 @@ -export {commandPipeline, CommandRunner} from './command-runner' +export {createCommandRunner, CommandRunner} from './command-runner' diff --git a/packages/helpers/src/command-runner/middleware.ts b/packages/helpers/src/command-runner/middleware.ts index 2ba84496..87b27bd0 100644 --- a/packages/helpers/src/command-runner/middleware.ts +++ b/packages/helpers/src/command-runner/middleware.ts @@ -20,12 +20,16 @@ const getEventTypesFromContext = ( eventTypes.add('execerr') } - if (ctx.stderr || ctx.exitCode !== 0) { + if (ctx.stderr) { eventTypes.add('stderr') } - if (ctx.stdout !== null && !ctx.stdout.trim()) { - eventTypes.add('no-stdout') + if (ctx.exitCode) { + eventTypes.add('exitcode') + } + + if (ctx.stdout) { + eventTypes.add('stdout') } if (!ctx.stderr && !ctx.execerr && ctx.stdout !== null && !ctx.exitCode) { @@ -72,7 +76,7 @@ export const failAction: CommandRunnerAction = message => async ctx => { return } - if (events.includes('no-stdout')) { + if (!events.includes('stdout')) { core.setFailed( `The command "${ctx.commandLine}" finished with exit code ${ctx.exitCode} and produced an empty output.` ) @@ -109,7 +113,7 @@ export const throwError: CommandRunnerAction = message => async ctx => { ) } - if (events.includes('no-stdout')) { + if (!events.includes('stdout')) { throw new Error( `The command "${ctx.commandLine}" finished with exit code ${ctx.exitCode} and produced an empty output.` ) @@ -143,7 +147,7 @@ export const produceLog: CommandRunnerAction = message => async (ctx, next) => { return } - if (events.includes('no-stdout')) { + if (!events.includes('stdout')) { core.warning(messageText) next() return @@ -176,7 +180,7 @@ export const produceLog: CommandRunnerAction = message => async (ctx, next) => { return } - if (events.includes('no-stdout')) { + if (!events.includes('stdout')) { core.warning( `The command "${ctx.commandLine}" finished with exit code ${ctx.exitCode} and produced an empty output.` ) diff --git a/packages/helpers/src/command-runner/test.ts b/packages/helpers/src/command-runner/test.ts deleted file mode 100644 index 49a7c028..00000000 --- a/packages/helpers/src/command-runner/test.ts +++ /dev/null @@ -1,18 +0,0 @@ -import {CommandRunner} from './command-runner' -import * as io from '@actions/io' - -;(async () => { - const toolpath = await io.which('cmd', true) - const args = ['/c', 'echo'] - - const echo = new CommandRunner('echo') - - echo - .on('exec-error', 'log') - .use(async (ctx, next) => { - console.log('success') - next() - }) - .addArgs('hello') - .run() -})() diff --git a/packages/helpers/src/command-runner/types.ts b/packages/helpers/src/command-runner/types.ts index 478d6951..54e7fad7 100644 --- a/packages/helpers/src/command-runner/types.ts +++ b/packages/helpers/src/command-runner/types.ts @@ -36,9 +36,19 @@ export type CommandRunnerMiddleware = ( export type CommandRunnerActionType = 'throw' | 'fail' | 'log' /* Command runner event types as used internally passed to middleware for the user */ -export type CommandRunnerEventType = 'execerr' | 'stderr' | 'no-stdout' | 'ok' +export type CommandRunnerEventType = + | 'execerr' + | 'stderr' + | 'stdout' + | 'exitcode' + | 'ok' /* Command runner event types as used by the user for filtering results */ export type CommandRunnerEventTypeExtended = | CommandRunnerEventType | `!${CommandRunnerEventType}` + +export type CommandRunnerOptions = Omit< + exec.ExecOptions, + 'failOnStdErr' | 'ignoreReturnCode' +>