Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be more lenient in accepting command inputs #405

Merged
merged 7 commits into from Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions packages/core/__tests__/command.test.ts
Expand Up @@ -112,6 +112,21 @@ describe('@actions/core/src/command', () => {
`::some-command prop1=value 1,prop2=value 2,prop3=value 3::${os.EOL}`
])
})

it('should handle issueing commands for non-string objects', () => {
command.issueCommand(
'some-command',
{
prop1: ({test: 'object'} as unknown) as string,
prop2: (123 as unknown) as string,
prop3: (true as unknown) as string
},
({test: 'object'} as unknown) as string
)
assertWriteCalls([
`::some-command prop1={"test"%3A"object"},prop2=123,prop3=true::{"test":"object"}${os.EOL}`
])
})
})

// Assert that process.stdout.write calls called only with the given arguments.
Expand Down
53 changes: 51 additions & 2 deletions packages/core/__tests__/core.test.ts
Expand Up @@ -54,6 +54,16 @@ describe('@actions/core', () => {
assertWriteCalls([`::set-env name=my var2::var val%0D%0A${os.EOL}`])
})

it('exportVariable handles boolean inputs', () => {
core.exportVariable('my var', true)
assertWriteCalls([`::set-env name=my var::true${os.EOL}`])
})

it('exportVariable handles number inputs', () => {
core.exportVariable('my var', 5)
assertWriteCalls([`::set-env name=my var::5${os.EOL}`])
})

it('setSecret produces the correct command', () => {
core.setSecret('secret val')
assertWriteCalls([`::add-mask::secret val${os.EOL}`])
Expand Down Expand Up @@ -104,18 +114,35 @@ describe('@actions/core', () => {
assertWriteCalls([`::set-output name=some output::some value${os.EOL}`])
})

it('setFailure sets the correct exit code and failure message', () => {
it('setOutput handles bools', () => {
core.setOutput('some output', false)
assertWriteCalls([`::set-output name=some output::false${os.EOL}`])
})

it('setOutput handles numbers', () => {
core.setOutput('some output', 1.01)
assertWriteCalls([`::set-output name=some output::1.01${os.EOL}`])
})

it('setFailed sets the correct exit code and failure message', () => {
core.setFailed('Failure message')
expect(process.exitCode).toBe(core.ExitCode.Failure)
assertWriteCalls([`::error::Failure message${os.EOL}`])
})

it('setFailure escapes the failure message', () => {
it('setFailed escapes the failure message', () => {
core.setFailed('Failure \r\n\nmessage\r')
expect(process.exitCode).toBe(core.ExitCode.Failure)
assertWriteCalls([`::error::Failure %0D%0A%0Amessage%0D${os.EOL}`])
})

it('setFailed handles Error', () => {
const message = 'this is my error message'
core.setFailed(new Error(message))
expect(process.exitCode).toBe(core.ExitCode.Failure)
assertWriteCalls([`::error::Error: ${message}${os.EOL}`])
})

it('error sets the correct error message', () => {
core.error('Error message')
assertWriteCalls([`::error::Error message${os.EOL}`])
Expand All @@ -126,6 +153,12 @@ describe('@actions/core', () => {
assertWriteCalls([`::error::Error message%0D%0A%0A${os.EOL}`])
})

it('error handles an error object', () => {
const message = 'this is my error message'
core.error(new Error(message))
assertWriteCalls([`::error::Error: ${message}${os.EOL}`])
})

it('warning sets the correct message', () => {
core.warning('Warning')
assertWriteCalls([`::warning::Warning${os.EOL}`])
Expand All @@ -136,6 +169,12 @@ describe('@actions/core', () => {
assertWriteCalls([`::warning::%0D%0Awarning%0A${os.EOL}`])
})

it('warning handles an error object', () => {
const message = 'this is my error message'
core.warning(new Error(message))
assertWriteCalls([`::warning::Error: ${message}${os.EOL}`])
})

it('startGroup starts a new group', () => {
core.startGroup('my-group')
assertWriteCalls([`::group::my-group${os.EOL}`])
Expand Down Expand Up @@ -174,6 +213,16 @@ describe('@actions/core', () => {
assertWriteCalls([`::save-state name=state_1::some value${os.EOL}`])
})

it('saveState handles numbers', () => {
core.saveState('state_1', 1)
assertWriteCalls([`::save-state name=state_1::1${os.EOL}`])
})

it('saveState handles bools', () => {
core.saveState('state_1', true)
assertWriteCalls([`::save-state name=state_1::true${os.EOL}`])
})

it('getState gets wrapper action state', () => {
expect(core.getState('TEST_1')).toBe('state_val')
})
Expand Down
28 changes: 22 additions & 6 deletions packages/core/src/command.ts
Expand Up @@ -2,8 +2,11 @@ import * as os from 'os'

// For internal use, subject to change.

// We use any as a valid input type
/* eslint-disable @typescript-eslint/no-explicit-any */

interface CommandProperties {
[key: string]: string
[key: string]: any
}

/**
Expand All @@ -19,7 +22,7 @@ interface CommandProperties {
export function issueCommand(
command: string,
properties: CommandProperties,
message: string
message: any
): void {
const cmd = new Command(command, properties, message)
process.stdout.write(cmd.toString() + os.EOL)
Expand Down Expand Up @@ -73,15 +76,28 @@ class Command {
}
}

function escapeData(s: string): string {
return (s || '')
/**
* Sanatizes an input into a string so it can be passed into issueCommand safely
* @param input input to sanitize into a string
*/
export function toCommandValue(input: any): string {
if (input === null || input === undefined) {
return ''
} else if (typeof input === 'string' || input instanceof String) {
return input as string
}
return JSON.stringify(input)
}

function escapeData(s: any): string {
return toCommandValue(s)
.replace(/%/g, '%25')
.replace(/\r/g, '%0D')
.replace(/\n/g, '%0A')
}

function escapeProperty(s: string): string {
return (s || '')
function escapeProperty(s: any): string {
return toCommandValue(s)
.replace(/%/g, '%25')
.replace(/\r/g, '%0D')
.replace(/\n/g, '%0A')
Expand Down
37 changes: 21 additions & 16 deletions packages/core/src/core.ts
@@ -1,4 +1,4 @@
import {issue, issueCommand} from './command'
import {issue, issueCommand, toCommandValue} from './command'

import * as os from 'os'
import * as path from 'path'
Expand Down Expand Up @@ -33,11 +33,13 @@ export enum ExitCode {
/**
* Sets env variable for this action and future actions in the job
* @param name the name of the variable to set
* @param val the value of the variable
* @param val the value of the variable. Non-string values will be converted to a string via JSON.stringify
*/
export function exportVariable(name: string, val: string): void {
process.env[name] = val
issueCommand('set-env', {name}, val)
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function exportVariable(name: string, val: any): void {
const convertedVal = toCommandValue(val)
process.env[name] = convertedVal
issueCommand('set-env', {name}, convertedVal)
}

/**
Expand Down Expand Up @@ -78,9 +80,10 @@ export function getInput(name: string, options?: InputOptions): string {
* Sets the value of an output.
*
* @param name name of the output to set
* @param value value to store
* @param value value to store. Non-string values will be converted to a string via JSON.stringify
*/
export function setOutput(name: string, value: string): void {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function setOutput(name: string, value: any): void {
issueCommand('set-output', {name}, value)
}

Expand All @@ -93,8 +96,9 @@ export function setOutput(name: string, value: string): void {
* When the action exits it will be with an exit code of 1
* @param message add error issue message
*/
export function setFailed(message: string): void {
export function setFailed(message: string | Error): void {
process.exitCode = ExitCode.Failure

error(message)
}

Expand All @@ -119,18 +123,18 @@ export function debug(message: string): void {

/**
* Adds an error issue
* @param message error issue message
* @param message error issue message. Errors will be converted to string via toString()
*/
export function error(message: string): void {
issue('error', message)
export function error(message: string | Error): void {
thboop marked this conversation as resolved.
Show resolved Hide resolved
issue('error', message instanceof Error ? message.toString() : message)
}

/**
* Adds an warning issue
* @param message warning issue message
* @param message warning issue message. Errors will be converted to string via toString()
*/
export function warning(message: string): void {
issue('warning', message)
export function warning(message: string | Error): void {
issue('warning', message instanceof Error ? message.toString() : message)
}

/**
Expand Down Expand Up @@ -189,9 +193,10 @@ export async function group<T>(name: string, fn: () => Promise<T>): Promise<T> {
* Saves state for current action, the state can only be retrieved by this action's post job execution.
*
* @param name name of the state to store
* @param value value to store
* @param value value to store. Non-string values will be converted to a string via JSON.stringify
*/
export function saveState(name: string, value: string): void {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function saveState(name: string, value: any): void {
issueCommand('save-state', {name}, value)
}

Expand Down