From 99befb3e6fe8907009a57d3b6ad7aa1e2c74c37d Mon Sep 17 00:00:00 2001 From: eric sciple Date: Thu, 14 Nov 2019 12:35:12 -0500 Subject: [PATCH] toolrunner should which tool before invoking --- .github/workflows/unit-tests.yml | 70 ++++--------------- package.json | 3 +- packages/core/package-lock.json | 2 +- packages/exec/README.md | 6 +- packages/exec/__tests__/exec.test.ts | 36 ++++++++++ .../exec/__tests__/scripts/print-args-cmd.cmd | 12 ++++ packages/exec/package.json | 2 +- packages/exec/src/toolrunner.ts | 3 + packages/tool-cache/package-lock.json | 2 +- 9 files changed, 72 insertions(+), 64 deletions(-) create mode 100644 packages/exec/__tests__/scripts/print-args-cmd.cmd diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index ac66f9ac56..feaa75fb6f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -6,18 +6,27 @@ on: pull_request: paths-ignore: - '**.md' + jobs: - Ubuntu: - name: Run Ubuntu - runs-on: ubuntu-latest + + build: + name: Build + + strategy: + matrix: + runs-on: [ubuntu-latest, macOS-latest, windows-latest] + fail-fast: false + + runs-on: ${{ matrix.runs-on }} + steps: - name: Checkout - uses: actions/checkout@master + uses: actions/checkout@v1 - - name: Set Node.js 10.x + - name: Set Node.js 12.x uses: actions/setup-node@master with: - node-version: 10.x + node-version: 12.x - name: npm install run: npm install @@ -36,52 +45,3 @@ jobs: - name: Format run: npm run format-check - macOS: - name: Run macOS - runs-on: macos-latest - steps: - - name: Checkout - uses: actions/checkout@master - - - name: Set Node.js 10.x - uses: actions/setup-node@master - with: - node-version: 10.x - - - name: npm install - run: npm install - - - name: Bootstrap - run: npm run bootstrap - - - name: Compile - run: npm run build - - - name: npm test - run: npm test - Windows: - name: Run Windows - runs-on: windows-latest - steps: - - name: Checkout - uses: actions/checkout@master - - - name: Set Node.js 10.x - uses: actions/setup-node@master - with: - node-version: 10.x - - - name: npm install - run: npm install - - - name: Bootstrap - run: npm run bootstrap - - - name: Compile - run: npm run build - - # TODO: This currently ignores exec due to issues with Node and spawning on Windows, which I think is exacerbated by Jest. - # It doesn't seem to affect behavior in actions themselves, just when testing with Jest. - # See other similar issues here: https://github.com/nodejs/node/issues/25484 - - name: npm test - run: npm run test-ci diff --git a/package.json b/package.json index 166174909b..563eab1e13 100644 --- a/package.json +++ b/package.json @@ -9,8 +9,7 @@ "format-check": "prettier --check packages/**/*.ts", "lint": "eslint packages/**/*.ts", "new-package": "scripts/create-package", - "test": "jest", - "test-ci": "jest --testPathIgnorePatterns=\"/packages/exec/__tests__/exec.test.ts\"" + "test": "jest" }, "devDependencies": { "@types/jest": "^24.0.11", diff --git a/packages/core/package-lock.json b/packages/core/package-lock.json index 3449e3e313..fc47b0cbdf 100644 --- a/packages/core/package-lock.json +++ b/packages/core/package-lock.json @@ -1,6 +1,6 @@ { "name": "@actions/core", - "version": "1.1.1", + "version": "1.2.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/exec/README.md b/packages/exec/README.md index e3eff742ae..2aa9f5860a 100644 --- a/packages/exec/README.md +++ b/packages/exec/README.md @@ -48,13 +48,11 @@ await exec.exec('node', ['index.js', 'foo=bar'], options); #### Exec tools not in the PATH -You can use it in conjunction with the `which` function from `@actions/io` to execute tools that are not in the PATH: +You can specify the full path for tools not in the PATH: ```js const exec = require('@actions/exec'); const io = require('@actions/io'); -const pythonPath: string = await io.which('python', true) - -await exec.exec(`"${pythonPath}"`, ['main.py']); +await exec.exec('""', ['arg1']); ``` diff --git a/packages/exec/__tests__/exec.test.ts b/packages/exec/__tests__/exec.test.ts index e0b696413f..30573fa681 100644 --- a/packages/exec/__tests__/exec.test.ts +++ b/packages/exec/__tests__/exec.test.ts @@ -572,6 +572,42 @@ describe('@actions/exec', () => { ) }) + it('execs .cmd from path (Windows)', async () => { + // this test validates whether a .cmd is resolved from the PATH when the extension is not specified + const cmd = 'print-args-cmd' // note, not print-args-cmd.cmd + const cmdPath = path.join(__dirname, 'scripts', `${cmd}.cmd`) + const args: string[] = ['my arg 1', 'my arg 2'] + const outStream = new StringStream() + let output = '' + const options = { + outStream: outStream, + listeners: { + stdout: (data: Buffer) => { + output += data.toString() + } + } + } + + const originalPath = process.env['Path'] + try { + process.env['Path'] = `${originalPath};${path.dirname(cmdPath)}` + const exitCode = await exec.exec(`${cmd}`, args, options) + expect(exitCode).toBe(0) + expect(outStream.getContents().split(os.EOL)[0]).toBe( + `[command]${ + process.env.ComSpec + } /D /S /C "${cmdPath} "my arg 1" "my arg 2""` + ) + expect(output.trim()).toBe( + 'args[0]: "my arg 1"\r\n' + + 'args[1]: "my arg 2"' + ) + } catch (err) { + process.env['Path'] = originalPath + throw err + } + }) + it('execs .cmd with arg quoting (Windows)', async () => { // this test validates .cmd quoting rules are applied, not the default libuv rules const cmdPath = path.join( diff --git a/packages/exec/__tests__/scripts/print-args-cmd.cmd b/packages/exec/__tests__/scripts/print-args-cmd.cmd new file mode 100644 index 0000000000..7f3e4e6691 --- /dev/null +++ b/packages/exec/__tests__/scripts/print-args-cmd.cmd @@ -0,0 +1,12 @@ +@echo off +setlocal +set index=0 + +:check_arg +set arg=%1 +if not defined arg goto :eof +set "arg=%arg:"=%" +echo args[%index%]: "%arg%" +set /a index=%index%+1 +shift +goto check_arg \ No newline at end of file diff --git a/packages/exec/package.json b/packages/exec/package.json index a57920eac2..ee3a773df1 100644 --- a/packages/exec/package.json +++ b/packages/exec/package.json @@ -32,7 +32,7 @@ "bugs": { "url": "https://github.com/actions/toolkit/issues" }, - "devDependencies": { + "dependencies": { "@actions/io": "^1.0.1" } } diff --git a/packages/exec/src/toolrunner.ts b/packages/exec/src/toolrunner.ts index 32dc5f948e..f669f24180 100644 --- a/packages/exec/src/toolrunner.ts +++ b/packages/exec/src/toolrunner.ts @@ -3,6 +3,7 @@ import * as events from 'events' import * as child from 'child_process' import * as stream from 'stream' import * as im from './interfaces' +import * as io from '@actions/io' /* eslint-disable @typescript-eslint/unbound-method */ @@ -392,6 +393,8 @@ export class ToolRunner extends events.EventEmitter { * @returns number */ async exec(): Promise { + this.toolPath = await io.which(this.toolPath, true) + return new Promise((resolve, reject) => { this._debug(`exec tool: ${this.toolPath}`) this._debug('arguments:') diff --git a/packages/tool-cache/package-lock.json b/packages/tool-cache/package-lock.json index ffea2aa0c0..3c3e386267 100644 --- a/packages/tool-cache/package-lock.json +++ b/packages/tool-cache/package-lock.json @@ -1,6 +1,6 @@ { "name": "@actions/tool-cache", - "version": "1.1.1", + "version": "1.1.2", "lockfileVersion": 1, "requires": true, "dependencies": {