remove trailing comma from commands (#263)

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])
+  }
+}

packages/core/__tests__/lib.test.ts → 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', () => {

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 || ''}`)}`
           }
         }
       }