From 616fa81820c9b34fd44f4a0cfeda436b4b8338d3 Mon Sep 17 00:00:00 2001 From: Nick Fields <46869826+nick-fields@users.noreply.github.com> Date: Wed, 3 Aug 2022 23:02:05 -0400 Subject: [PATCH] Use spawn not exec to run commands (#88) * minor: use spawn to stream larger output rather than exec which buffers it * test: verify distinct error code is returned from large output test * test: breakout additional integration tests to run in parallel * test: dont pass/fail PRs for coverage yet --- .github/codecov.yml | 5 + .github/workflows/ci_cd.yml | 224 +++++++++++++++++++--------- dist/index.js | 4 +- sample.env | 12 +- src/index.ts | 6 +- test-data/large-output/Makefile | 13 ++ test-data/large-output/kibibyte.txt | 13 ++ 7 files changed, 196 insertions(+), 81 deletions(-) create mode 100644 test-data/large-output/Makefile create mode 100644 test-data/large-output/kibibyte.txt diff --git a/.github/codecov.yml b/.github/codecov.yml index af03a54..bb33a82 100644 --- a/.github/codecov.yml +++ b/.github/codecov.yml @@ -5,3 +5,8 @@ comment: layout: 'diff, flags' behavior: default require_changes: true +coverage: + # don't pass/fail PRs for coverage yet + status: + project: off + patch: off diff --git a/.github/workflows/ci_cd.yml b/.github/workflows/ci_cd.yml index d86c854..ce5dd6f 100644 --- a/.github/workflows/ci_cd.yml +++ b/.github/workflows/ci_cd.yml @@ -4,7 +4,7 @@ on: jobs: # runs on branch pushes only - ci_unuit: + ci_unit: name: Run Unit Tests if: startsWith(github.ref, 'refs/heads') runs-on: ubuntu-latest @@ -24,6 +24,157 @@ jobs: directory: ./coverage/ verbose: true + ci_integration_envvar: + name: Run Integration Env Var Tests + if: startsWith(github.ref, 'refs/heads') + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: 16 + - name: Install dependencies + run: npm ci + - name: env-vars-passed-through + uses: ./ + env: + NODE_OPTIONS: '--max_old_space_size=3072' + with: + timeout_minutes: 1 + max_attempts: 2 + command: node -e 'console.log(process.env.NODE_OPTIONS)' + + ci_integration_large_output: + name: Run Integration Large Output Tests + if: startsWith(github.ref, 'refs/heads') + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: 16 + - name: Install dependencies + run: npm ci + - name: Test 100MiB of output can be processed + id: large-output + continue-on-error: true + uses: ./ + with: + max_attempts: 1 + timeout_minutes: 5 + command: 'make -C ./test-data/large-output bytes-102400' + - name: Assert test had expected result + uses: nick-invision/assert-action@v1 + with: + expected: failure + actual: ${{ steps.large-output.outcome }} + - name: Assert exit code is expected + uses: nick-invision/assert-action@v1 + with: + expected: 2 + actual: ${{ steps.large-output.outputs.exit_code }} + + ci_integration_retry_on_exit_code: + name: Run Integration retry_on_exit_code Tests + if: startsWith(github.ref, 'refs/heads') + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: 16 + - name: Install dependencies + run: npm ci + - name: retry_on_exit_code (with expected error code) + id: retry_on_exit_code_expected + uses: ./ + continue-on-error: true + with: + timeout_minutes: 1 + retry_on_exit_code: 2 + max_attempts: 3 + command: node -e "process.exit(2)" + - uses: nick-invision/assert-action@v1 + with: + expected: failure + actual: ${{ steps.retry_on_exit_code_expected.outcome }} + - uses: nick-invision/assert-action@v1 + with: + expected: 3 + actual: ${{ steps.retry_on_exit_code_expected.outputs.total_attempts }} + + - name: retry_on_exit_code (with unexpected error code) + id: retry_on_exit_code_unexpected + uses: ./ + continue-on-error: true + with: + timeout_minutes: 1 + retry_on_exit_code: 2 + max_attempts: 3 + command: node -e "process.exit(1)" + - uses: nick-invision/assert-action@v1 + with: + expected: failure + actual: ${{ steps.retry_on_exit_code_unexpected.outcome }} + - uses: nick-invision/assert-action@v1 + with: + expected: 1 + actual: ${{ steps.retry_on_exit_code_unexpected.outputs.total_attempts }} + + ci_integration_continue_on_error: + name: Run Integration continue_on_error Tests + if: startsWith(github.ref, 'refs/heads') + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v2 + - name: Setup Node.js + uses: actions/setup-node@v1 + with: + node-version: 16 + - name: Install dependencies + run: npm ci + - name: happy-path (continue_on_error) + id: happy_path_continue_on_error + uses: ./ + with: + command: node -e "process.exit(0)" + timeout_minutes: 1 + continue_on_error: true + - name: sad-path (continue_on_error) + id: sad_path_continue_on_error + uses: ./ + with: + command: node -e "process.exit(33)" + timeout_minutes: 1 + continue_on_error: true + - name: Verify continue_on_error returns correct exit code on success + uses: nick-invision/assert-action@v1 + with: + expected: 0 + actual: ${{ steps.happy_path_continue_on_error.outputs.exit_code }} + - name: Verify continue_on_error exits with correct outcome on success + uses: nick-invision/assert-action@v1 + with: + expected: success + actual: ${{ steps.happy_path_continue_on_error.outcome }} + - name: Verify continue_on_error returns correct exit code on error + uses: nick-invision/assert-action@v1 + with: + expected: 33 + actual: ${{ steps.sad_path_continue_on_error.outputs.exit_code }} + - name: Verify continue_on_error exits with successful outcome when an error occurs + uses: nick-invision/assert-action@v1 + with: + expected: success + actual: ${{ steps.sad_path_continue_on_error.outcome }} + ci_integration: name: Run Integration Tests if: startsWith(github.ref, 'refs/heads') @@ -98,42 +249,6 @@ jobs: command: node -e "process.exit(1)" on_retry_command: node -e "console.log('this is a retry command')" - - name: retry_on_exit_code (with expected error code) - id: retry_on_exit_code_expected - uses: ./ - continue-on-error: true - with: - timeout_minutes: 1 - retry_on_exit_code: 2 - max_attempts: 3 - command: node -e "process.exit(2)" - - uses: nick-invision/assert-action@v1 - with: - expected: failure - actual: ${{ steps.retry_on_exit_code_expected.outcome }} - - uses: nick-invision/assert-action@v1 - with: - expected: 3 - actual: ${{ steps.retry_on_exit_code_expected.outputs.total_attempts }} - - - name: retry_on_exit_code (with unexpected error code) - id: retry_on_exit_code_unexpected - uses: ./ - continue-on-error: true - with: - timeout_minutes: 1 - retry_on_exit_code: 2 - max_attempts: 3 - command: node -e "process.exit(1)" - - uses: nick-invision/assert-action@v1 - with: - expected: failure - actual: ${{ steps.retry_on_exit_code_unexpected.outcome }} - - uses: nick-invision/assert-action@v1 - with: - expected: 1 - actual: ${{ steps.retry_on_exit_code_unexpected.outputs.total_attempts }} - - name: on-retry-cmd (on-retry fails) id: on-retry-cmd-fails uses: ./ @@ -161,41 +276,6 @@ jobs: expected: failure actual: ${{ steps.sad_path_error.outcome }} - - name: happy-path (continue_on_error) - id: happy_path_continue_on_error - uses: ./ - with: - command: node -e "process.exit(0)" - timeout_minutes: 1 - continue_on_error: true - - name: sad-path (continue_on_error) - id: sad_path_continue_on_error - uses: ./ - with: - command: node -e "process.exit(33)" - timeout_minutes: 1 - continue_on_error: true - - name: Verify continue_on_error returns correct exit code on success - uses: nick-invision/assert-action@v1 - with: - expected: 0 - actual: ${{ steps.happy_path_continue_on_error.outputs.exit_code }} - - name: Verify continue_on_error exits with correct outcome on success - uses: nick-invision/assert-action@v1 - with: - expected: success - actual: ${{ steps.happy_path_continue_on_error.outcome }} - - name: Verify continue_on_error returns correct exit code on error - uses: nick-invision/assert-action@v1 - with: - expected: 33 - actual: ${{ steps.sad_path_continue_on_error.outputs.exit_code }} - - name: Verify continue_on_error exits with successful outcome when an error occurs - uses: nick-invision/assert-action@v1 - with: - expected: success - actual: ${{ steps.sad_path_continue_on_error.outcome }} - - name: retry_on (timeout) fails early if error encountered id: retry_on_timeout_fail uses: ./ diff --git a/dist/index.js b/dist/index.js index c75ff05..8346935 100644 --- a/dist/index.js +++ b/dist/index.js @@ -778,8 +778,8 @@ function runCmd(attempt) { done = false; (0, core_1.debug)("Running command ".concat(COMMAND, " on ").concat(OS, " using shell ").concat(executable)); child = attempt > 1 && NEW_COMMAND_ON_RETRY - ? (0, child_process_1.exec)(NEW_COMMAND_ON_RETRY, { shell: executable }) - : (0, child_process_1.exec)(COMMAND, { shell: executable }); + ? (0, child_process_1.spawn)(NEW_COMMAND_ON_RETRY, { shell: executable }) + : (0, child_process_1.spawn)(COMMAND, { shell: executable }); (_a = child.stdout) === null || _a === void 0 ? void 0 : _a.on('data', function (data) { process.stdout.write(data); }); diff --git a/sample.env b/sample.env index f69d5fd..d4de8c6 100644 --- a/sample.env +++ b/sample.env @@ -1,7 +1,11 @@ +# these are the bare minimum envvars required INPUT_TIMEOUT_MINUTES=1 INPUT_MAX_ATTEMPTS=3 INPUT_COMMAND="node -e 'process.exit(99)'" -INPUT_RETRY_WAIT_SECONDS=10 -SHELL=pwsh -INPUT_POLLING_INTERVAL_SECONDS=1 -INPUT_RETRY_ON=any +INPUT_CONTINUE_ON_ERROR=false + +# these are optional +#INPUT_RETRY_WAIT_SECONDS=10 +#SHELL=pwsh +#INPUT_POLLING_INTERVAL_SECONDS=1 +#INPUT_RETRY_ON=any diff --git a/src/index.ts b/src/index.ts index a691d19..c1b67ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,5 @@ import { getInput, error, warning, info, debug, setOutput } from '@actions/core'; -import { exec, execSync } from 'child_process'; +import { execSync, spawn } from 'child_process'; import ms from 'milliseconds'; import kill from 'tree-kill'; @@ -137,8 +137,8 @@ async function runCmd(attempt: number) { debug(`Running command ${COMMAND} on ${OS} using shell ${executable}`); const child = attempt > 1 && NEW_COMMAND_ON_RETRY - ? exec(NEW_COMMAND_ON_RETRY, { shell: executable }) - : exec(COMMAND, { shell: executable }); + ? spawn(NEW_COMMAND_ON_RETRY, { shell: executable }) + : spawn(COMMAND, { shell: executable }); child.stdout?.on('data', (data) => { process.stdout.write(data); diff --git a/test-data/large-output/Makefile b/test-data/large-output/Makefile new file mode 100644 index 0000000..2d15b4a --- /dev/null +++ b/test-data/large-output/Makefile @@ -0,0 +1,13 @@ +SHELL = bash + +# this tests fix for the following issues +# https://github.com/nick-fields/retry/issues/76 +# https://github.com/nick-fields/retry/issues/84 + +bytes-%: + for i in {1..$*}; do cat kibibyte.txt; done; exit 2 +.PHONY: bytes-% + +lines-%: + for i in {1..$*}; do echo a; done; exit 2 +.PHONY: lines-% diff --git a/test-data/large-output/kibibyte.txt b/test-data/large-output/kibibyte.txt new file mode 100644 index 0000000..54960cb --- /dev/null +++ b/test-data/large-output/kibibyte.txt @@ -0,0 +1,13 @@ +1: 0000 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +2: 0081 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +3: 0162 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +4: 243 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +5: 324 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +6: 405 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +7: 486 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +8: 567 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +9: 648 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +a: 729 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +b: 810 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +c: 891 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +d: 972 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa \ No newline at end of file