From 4ec2100521467fac8fd7c96dd454d3c982ab4e73 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 17 Dec 2019 11:49:39 -0500 Subject: [PATCH 1/2] remove trailing comma from commands --- packages/core/__tests__/command.ts | 34 +++++++++++++++++++ .../__tests__/{lib.test.ts => core.test.ts} | 0 packages/core/src/command.ts | 9 ++++- 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 packages/core/__tests__/command.ts rename packages/core/__tests__/{lib.test.ts => core.test.ts} (100%) diff --git a/packages/core/__tests__/command.ts b/packages/core/__tests__/command.ts new file mode 100644 index 0000000000..5d35109488 --- /dev/null +++ b/packages/core/__tests__/command.ts @@ -0,0 +1,34 @@ +import * as command from '../src/command' +import * as os from 'os' +import * as path from 'path' + +let originalWriteFunction + +describe('@actions/core/src/command', () => { + beforeAll(() => { + originalWriteFunction = process.stdout.write + }) + + beforeEach(() => { + process.stdout.write = jest.fn() + }) + + afterEach(() => { + }) + + afterAll(() => { + process.stdout.write = originalWriteFunction + }) + + it('command asdf', () => { + }) +}) + +// Assert that process.stdout.write calls called only with the given arguments. +function assertWriteCalls(calls: string[]): void { + expect(process.stdout.write).toHaveBeenCalledTimes(calls.length) + + for (let i = 0; i < calls.length; i++) { + expect(process.stdout.write).toHaveBeenNthCalledWith(i + 1, calls[i]) + } +} diff --git a/packages/core/__tests__/lib.test.ts b/packages/core/__tests__/core.test.ts similarity index 100% rename from packages/core/__tests__/lib.test.ts rename to packages/core/__tests__/core.test.ts diff --git a/packages/core/src/command.ts b/packages/core/src/command.ts index 0773a72415..29185dfc11 100644 --- a/packages/core/src/command.ts +++ b/packages/core/src/command.ts @@ -51,13 +51,20 @@ class Command { if (this.properties && Object.keys(this.properties).length > 0) { cmdStr += ' ' + let first = true for (const key in this.properties) { if (this.properties.hasOwnProperty(key)) { const val = this.properties[key] if (val) { + if (first) { + first = false + } else { + cmdStr += ',' + } + // safely append the val - avoid blowing up when attempting to // call .replace() if message is not a string for some reason - cmdStr += `${key}=${escape(`${val || ''}`)},` + cmdStr += `${key}=${escape(`${val || ''}`)}` } } } From d32364fe6328f3e357d53effe38a521a6a63cc02 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Tue, 17 Dec 2019 13:12:12 -0500 Subject: [PATCH 2/2] . --- packages/core/__tests__/command.test.ts | 79 +++++++++++++++++++++++++ packages/core/__tests__/command.ts | 34 ----------- packages/core/__tests__/core.test.ts | 10 ++-- 3 files changed, 84 insertions(+), 39 deletions(-) create mode 100644 packages/core/__tests__/command.test.ts delete mode 100644 packages/core/__tests__/command.ts diff --git a/packages/core/__tests__/command.test.ts b/packages/core/__tests__/command.test.ts new file mode 100644 index 0000000000..0fa99e97df --- /dev/null +++ b/packages/core/__tests__/command.test.ts @@ -0,0 +1,79 @@ +import * as command from '../src/command' +import * as os from 'os' + +/* eslint-disable @typescript-eslint/unbound-method */ + +let originalWriteFunction: (str: string) => boolean + +describe('@actions/core/src/command', () => { + beforeAll(() => { + originalWriteFunction = process.stdout.write + }) + + beforeEach(() => { + process.stdout.write = jest.fn() + }) + + afterEach(() => {}) + + afterAll(() => { + process.stdout.write = (originalWriteFunction as unknown) as ( + str: string + ) => boolean + }) + + it('command only', () => { + command.issueCommand('some-command', {}, '') + assertWriteCalls([`::some-command::${os.EOL}`]) + }) + + it('command with message', () => { + command.issueCommand('some-command', {}, 'some message') + assertWriteCalls([`::some-command::some message${os.EOL}`]) + }) + + it('command with message and properties', () => { + command.issueCommand( + 'some-command', + {prop1: 'value 1', prop2: 'value 2'}, + 'some message' + ) + assertWriteCalls([ + `::some-command prop1=value 1,prop2=value 2::some message${os.EOL}` + ]) + }) + + it('command with one property', () => { + command.issueCommand('some-command', {prop1: 'value 1'}, '') + assertWriteCalls([`::some-command prop1=value 1::${os.EOL}`]) + }) + + it('command with two properties', () => { + command.issueCommand( + 'some-command', + {prop1: 'value 1', prop2: 'value 2'}, + '' + ) + assertWriteCalls([`::some-command prop1=value 1,prop2=value 2::${os.EOL}`]) + }) + + it('command with three properties', () => { + command.issueCommand( + 'some-command', + {prop1: 'value 1', prop2: 'value 2', prop3: 'value 3'}, + '' + ) + assertWriteCalls([ + `::some-command prop1=value 1,prop2=value 2,prop3=value 3::${os.EOL}` + ]) + }) +}) + +// Assert that process.stdout.write calls called only with the given arguments. +function assertWriteCalls(calls: string[]): void { + expect(process.stdout.write).toHaveBeenCalledTimes(calls.length) + + for (let i = 0; i < calls.length; i++) { + expect(process.stdout.write).toHaveBeenNthCalledWith(i + 1, calls[i]) + } +} diff --git a/packages/core/__tests__/command.ts b/packages/core/__tests__/command.ts deleted file mode 100644 index 5d35109488..0000000000 --- a/packages/core/__tests__/command.ts +++ /dev/null @@ -1,34 +0,0 @@ -import * as command from '../src/command' -import * as os from 'os' -import * as path from 'path' - -let originalWriteFunction - -describe('@actions/core/src/command', () => { - beforeAll(() => { - originalWriteFunction = process.stdout.write - }) - - beforeEach(() => { - process.stdout.write = jest.fn() - }) - - afterEach(() => { - }) - - afterAll(() => { - process.stdout.write = originalWriteFunction - }) - - it('command asdf', () => { - }) -}) - -// Assert that process.stdout.write calls called only with the given arguments. -function assertWriteCalls(calls: string[]): void { - expect(process.stdout.write).toHaveBeenCalledTimes(calls.length) - - for (let i = 0; i < calls.length; i++) { - expect(process.stdout.write).toHaveBeenNthCalledWith(i + 1, calls[i]) - } -} diff --git a/packages/core/__tests__/core.test.ts b/packages/core/__tests__/core.test.ts index 8c069a31c6..2c7e0b5424 100644 --- a/packages/core/__tests__/core.test.ts +++ b/packages/core/__tests__/core.test.ts @@ -37,21 +37,21 @@ describe('@actions/core', () => { it('exportVariable produces the correct command and sets the env', () => { core.exportVariable('my var', 'var val') - assertWriteCalls([`::set-env name=my var,::var val${os.EOL}`]) + assertWriteCalls([`::set-env name=my var::var val${os.EOL}`]) }) it('exportVariable escapes variable names', () => { core.exportVariable('special char var \r\n];', 'special val') expect(process.env['special char var \r\n];']).toBe('special val') assertWriteCalls([ - `::set-env name=special char var %0D%0A%5D%3B,::special val${os.EOL}` + `::set-env name=special char var %0D%0A%5D%3B::special val${os.EOL}` ]) }) it('exportVariable escapes variable values', () => { core.exportVariable('my var2', 'var val\r\n') expect(process.env['my var2']).toBe('var val\r\n') - assertWriteCalls([`::set-env name=my var2,::var val%0D%0A${os.EOL}`]) + assertWriteCalls([`::set-env name=my var2::var val%0D%0A${os.EOL}`]) }) it('setSecret produces the correct command', () => { @@ -101,7 +101,7 @@ describe('@actions/core', () => { it('setOutput produces the correct command', () => { core.setOutput('some output', 'some value') - assertWriteCalls([`::set-output name=some output,::some value${os.EOL}`]) + assertWriteCalls([`::set-output name=some output::some value${os.EOL}`]) }) it('setFailure sets the correct exit code and failure message', () => { @@ -171,7 +171,7 @@ describe('@actions/core', () => { it('saveState produces the correct command', () => { core.saveState('state_1', 'some value') - assertWriteCalls([`::save-state name=state_1,::some value${os.EOL}`]) + assertWriteCalls([`::save-state name=state_1::some value${os.EOL}`]) }) it('getState gets wrapper action state', () => {