fix(tests): use shebang line for test scripts #2666

Problem:
Since nodejs16 our childprocess tests fail.

Solution:
The exact failure was from the `spawn` syscall, exit code 8. Which is for
ENOEXEC. Couldn't find the _exact_ cause but it probably has something to do
with `libuv`. Perhaps they're moving towards more of a POXIS 'spawn' rather than
`execvp` semantics.

Author: JadenSimon

src/shared/utilities/childProcess.ts

@@ -171,7 +171,7 @@ export class ChildProcess {
                 this.childProcess = crossSpawn.spawn(this.command, args, options.spawnOptions)
                 this.registerLifecycleListeners(this.childProcess, errorHandler, timeout)
             } catch (err) {
-                return errorHandler(err as Error)
+                return reject(err)
             }
 
             // Emitted whenever:

src/test/shared/utilities/childProcess.test.ts

@@ -8,7 +8,7 @@ import * as fs from 'fs-extra'
 import * as os from 'os'
 import * as path from 'path'
 import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../../shared/filesystemUtilities'
-import { ChildProcess, ChildProcessResult } from '../../../shared/utilities/childProcess'
+import { ChildProcess } from '../../../shared/utilities/childProcess'
 import { sleep } from '../../../shared/utilities/timeoutUtils'
 import { Timeout, waitUntil } from '../../../shared/utilities/timeoutUtils'
 
@@ -25,163 +25,11 @@ describe('ChildProcess', async function () {
         await tryRemoveFolder(tempFolder)
     })
 
-    describe('run', async function () {
-        if (process.platform === 'win32') {
-            it('runs and captures stdout - windows', async function () {
-                const batchFile = path.join(tempFolder, 'test-script.bat')
-                writeBatchFile(batchFile)
-
-                const childProcess = new ChildProcess(batchFile)
-
-                const result = await childProcess.run()
-
-                validateChildProcessResult({
-                    childProcessResult: result,
-                    expectedExitCode: 0,
-                    expectedOutput: 'hi',
-                })
-            })
-
-            it('runs cmd files containing a space in the filename and folder', async function () {
-                const subfolder: string = path.join(tempFolder, 'sub folder')
-                const command: string = path.join(subfolder, 'test script.cmd')
-
-                fs.mkdirSync(subfolder)
-
-                writeWindowsCommandFile(command)
-
-                const childProcess = new ChildProcess(command)
-
-                const result = await childProcess.run()
-
-                validateChildProcessResult({
-                    childProcessResult: result,
-                    expectedExitCode: 0,
-                    expectedOutput: 'hi',
-                })
-            })
-
-            it('errs when starting twice - windows', async function () {
-                const batchFile = path.join(tempFolder, 'test-script.bat')
-                writeBatchFile(batchFile)
-
-                const childProcess = new ChildProcess(batchFile)
-
-                // We want to verify that the error is thrown even if the first
-                // invocation is still in progress, so we don't await the promise.
-                childProcess.run()
-
-                try {
-                    await childProcess.run()
-                } catch (err) {
-                    return
-                }
-
-                assert.fail('Expected exception, but none was thrown.')
-            })
-        } else {
-            it('runs and captures stdout - unix', async function () {
-                const scriptFile = path.join(tempFolder, 'test-script.sh')
-                writeShellFile(scriptFile)
-
-                const childProcess = new ChildProcess(scriptFile)
-
-                const result = await childProcess.run()
-
-                validateChildProcessResult({
-                    childProcessResult: result,
-                    expectedExitCode: 0,
-                    expectedOutput: 'hi',
-                })
-            })
-
-            it('errs when starting twice - unix', async function () {
-                const scriptFile = path.join(tempFolder, 'test-script.sh')
-                writeShellFile(scriptFile)
-
-                const childProcess = new ChildProcess(scriptFile)
-
-                // We want to verify that the error is thrown even if the first
-                // invocation is still in progress, so we don't await the promise.
-                childProcess.run()
-
-                try {
-                    await childProcess.run()
-                } catch (err) {
-                    return
-                }
-
-                assert.fail('Expected exception, but none was thrown.')
-            })
-        } // END Linux only tests
-
-        it('runs scripts containing a space in the filename and folder', async function () {
-            const subfolder: string = path.join(tempFolder, 'sub folder')
-            let command: string
-
-            fs.mkdirSync(subfolder)
-
-            if (process.platform === 'win32') {
-                command = path.join(subfolder, 'test script.bat')
-                writeBatchFile(command)
-            } else {
-                command = path.join(subfolder, 'test script.sh')
-                writeShellFile(command)
-            }
-
-            const childProcess = new ChildProcess(command)
-
-            const result = await childProcess.run()
-
-            validateChildProcessResult({
-                childProcessResult: result,
-                expectedExitCode: 0,
-                expectedOutput: 'hi',
-            })
-        })
-
-        it('reports error for missing executable', async function () {
-            const batchFile = path.join(tempFolder, 'nonExistentScript')
-
-            const childProcess = new ChildProcess(batchFile)
-
-            const result = await childProcess.run()
-
-            assert.notStrictEqual(result.exitCode, 0)
-            assert.notStrictEqual(result.error, undefined)
-        })
-
-        function validateChildProcessResult({
-            childProcessResult,
-            expectedExitCode,
-            expectedOutput,
-        }: {
-            childProcessResult: ChildProcessResult
-            expectedExitCode: number
-            expectedOutput: string
-        }) {
-            assert.strictEqual(
-                childProcessResult.exitCode,
-                expectedExitCode,
-                `Expected exit code ${expectedExitCode}, got ${childProcessResult.exitCode}`
-            )
-
-            assert.strictEqual(
-                childProcessResult.stdout,
-                expectedOutput,
-                `Expected stdout to be ${expectedOutput} , got: ${childProcessResult.stdout}`
-            )
-        }
-    })
-
     describe('run', async function () {
         async function assertRegularRun(childProcess: ChildProcess): Promise<void> {
-            const result = await childProcess.run({
-                onStdout: text => {
-                    assert.strictEqual(text, 'hi' + os.EOL, 'Unexpected stdout')
-                },
-            })
+            const result = await childProcess.run()
             assert.strictEqual(result.exitCode, 0, 'Unexpected close code')
+            assert.strictEqual(result.stdout, 'hi', 'Unexpected stdout')
         }
 
         if (process.platform === 'win32') {
@@ -426,13 +274,12 @@ describe('ChildProcess', async function () {
                 writeShellFileWithDelays(scriptFile)
 
                 const childProcess = new ChildProcess('sh', [scriptFile])
-
-                // `await` is intentionally not used, we want to check the process while it runs.
-                childProcess.run()
+                const result = childProcess.run()
 
                 assert.strictEqual(childProcess.stopped, false)
                 childProcess.stop()
-                await waitUntil(async () => childProcess.stopped, { timeout: 1000, interval: 100, truthy: true })
+                await result
+
                 assert.strictEqual(childProcess.stopped, true)
             })
 
@@ -442,15 +289,13 @@ describe('ChildProcess', async function () {
 
                 const childProcess = new ChildProcess(scriptFile)
 
-                // `await` is intentionally not used, we want to check the process while it runs.
-                childProcess.run()
+                const result = childProcess.run()
 
                 childProcess.stop()
-                await waitUntil(async () => childProcess.stopped, { timeout: 1000, interval: 100, truthy: true })
+                await result
+
                 assert.strictEqual(childProcess.stopped, true)
-                assert.throws(() => {
-                    childProcess.stop()
-                })
+                assert.throws(() => childProcess.stop())
             })
         } // END Unix-only tests
     })
@@ -471,8 +316,8 @@ describe('ChildProcess', async function () {
         fs.writeFileSync(filename, `@echo OFF${os.EOL}echo hi`)
     }
 
-    function writeShellFile(filename: string, contents?: string): void {
-        fs.writeFileSync(filename, contents ?? 'echo hi')
+    function writeShellFile(filename: string, contents = 'echo hi'): void {
+        fs.writeFileSync(filename, `#!/bin/sh\n${contents}`)
         fs.chmodSync(filename, 0o744)
     }
 
@@ -481,7 +326,6 @@ describe('ChildProcess', async function () {
         echo hi
         sleep 20
         echo bye`
-        fs.writeFileSync(filename, file)
-        fs.chmodSync(filename, 0o744)
+        writeShellFile(filename, file)
     }
 })

src/test/testUtil.ts

@@ -111,7 +111,7 @@ export function createExecutableFile(filepath: string, contents: string): void {
     if (process.platform === 'win32') {
         fs.writeFileSync(filepath, `@echo OFF$\r\n${contents}\r\n`)
     } else {
-        fs.writeFileSync(filepath, contents)
+        fs.writeFileSync(filepath, `#!/bin/sh\n${contents}`)
         fs.chmodSync(filepath, 0o744)
     }
 }