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
nick-fields · Aug 4, 2022 · 616fa81 · 616fa81
1 parent a25f198
commit 616fa81
Show file tree

Hide file tree

Showing 7 changed files with 196 additions and 81 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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