ci: Address CI flakes and Node warning messages on the 'fetch-esm' sample (#1830)

Author: mjameswh

.github/workflows/ci.yml

@@ -256,7 +256,7 @@ jobs:
       - name: Instantiate sample project using verdaccio artifacts - Fetch ESM
         run: |
           node scripts/init-from-verdaccio.js --registry-dir ${{ steps.tmp-dir.outputs.dir }}/npm-registry --sample https://github.com/temporalio/samples-typescript/tree/main/fetch-esm --target-dir ${{ steps.tmp-dir.outputs.dir }}/sample-fetch-esm
-          node scripts/test-example.js --work-dir "${{ steps.tmp-dir.outputs.dir }}/sample-fetch-esm"
+          node scripts/test-example.js --work-dir "${{ steps.tmp-dir.outputs.dir }}/sample-fetch-esm" --script-name workflow-local --expected-output "Hello World And Hello Wonderful Temporal!"
 
       # End samples
 

packages/test/src/test-integration-workflows.ts

@@ -1403,7 +1403,7 @@ test('root execution is exposed', async (t) => {
         }
       }
     };
-    await waitUntil(childStarted, 5000);
+    await waitUntil(childStarted, 8000);
     const childDesc = await childHandle.describe();
     const parentDesc = await handle.describe();
 

packages/test/src/test-prometheus.ts

@@ -109,15 +109,22 @@ test.serial('Exporting Prometheus metrics from Core works with lots of options',
           'temporal_workflow_task_replay_latency_seconds_bucket{namespace="default",' +
             'service_name="temporal-core-sdk",task_queue="test-prometheus",' +
             'workflow_type="successString",my_tag="my_value",le="0.001"}'
-        )
+        ),
+        `Actual: \n-------\n${text}\n-------`
       );
 
       // Verify histogram overrides
-      t.assert(text.match(/temporal_request_latency_seconds_bucket\{.*,le="31415"/));
-      t.assert(text.match(/workflow_task_execution_latency_seconds_bucket\{.*,le="31415"/));
+      t.assert(
+        text.match(/temporal_request_latency_seconds_bucket\{.*,le="31415"/),
+        `Actual: \n-------\n${text}\n-------`
+      );
+      t.assert(
+        text.match(/workflow_task_execution_latency_seconds_bucket\{.*,le="31415"/),
+        `Actual: \n-------\n${text}\n-------`
+      );
 
       // Verify prefix exists on client request metrics
-      t.assert(text.includes('temporal_long_request{'));
+      t.assert(text.includes('temporal_long_request{'), `Actual: \n-------\n${text}\n-------`);
     });
   } finally {
     await localEnv.teardown();

scripts/test-example.js

@@ -1,6 +1,6 @@
 const { spawn: spawnChild, spawnSync } = require('child_process');
 const arg = require('arg');
-const { shell, kill } = require('./utils');
+const { shell, kill, sleep, waitOnChild } = require('./utils');
 
 const npm = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
 
@@ -23,8 +23,8 @@ async function withWorker(workdir, fn) {
   }
 }
 
-async function test(workdir) {
-  const { status, output } = spawnSync(npm, ['run', 'workflow'], {
+async function test(workdir, scriptName, expectedOutput) {
+  const { status, output } = spawnSync(npm, ['run', scriptName], {
     cwd: workdir,
     shell,
     encoding: 'utf8',
@@ -33,21 +33,25 @@ async function test(workdir) {
   if (status !== 0) {
     throw new Error('Failed to run workflow');
   }
-  if (!output[1].includes('Hello, Temporal!\n')) {
+  if (!output[1].includes(`${expectedOutput}\n`)) {
     throw new Error(`Invalid output: "${output[1]}"`);
   }
 }
 
 async function main() {
   const opts = arg({
     '--work-dir': String,
+    '--script-name': String,
+    '--expected-output': String,
   });
   const workdir = opts['--work-dir'];
   if (!workdir) {
     throw new Error('Missing required option --work-dir');
   }
+  const scriptName = opts['--script-name'] ?? 'workflow';
+  const expectedOutput = opts['--expected-output'] ?? 'Hello, Temporal!';
 
-  await withWorker(workdir, () => test(workdir));
+  await withWorker(workdir, () => test(workdir, scriptName, expectedOutput));
 }
 
 main()

scripts/utils.js

@@ -32,18 +32,35 @@ async function kill(child, signal = 'SIGINT') {
   } else {
     process.kill(-child.pid, signal);
   }
+
   try {
     await waitOnChild(child);
   } catch (err) {
-    // Should error if the error is not a child process error or it is a child
-    // process and either the platform is Windows or the signal matches.
-    const shouldError = err.name !== 'ChildProcessError' || (process.platform !== 'win32' && err.signal !== signal);
-    if (shouldError) {
-      throw err;
-    }
+    const signalNumber = getSignalNumber(signal);
+
+    // Ignore the error if it simply indicates process termination due to the signal we just sent.
+    // On Unix, a process may complete with exit code 128 + signal number to indicate
+    // that it was terminated by a signal. But we have the signal name.
+    const shouldIgnore =
+      err instanceof ChildProcessError &&
+      (process.platform === 'win32' || err.signal === signal || err.code === 128 + signalNumber);
+
+    if (!shouldIgnore) throw err;
   }
 }
 
+function getSignalNumber(signal) {
+  // We don't need any other signals for now, and probably never will, so no need to list them all.
+  // But if any other signals ends up being needed, look at `man 3 signal` for the complete list.
+  const SIGNAL_NUMBERS = {
+    SIGINT: 2,
+    SIGTERM: 15,
+  };
+
+  if (signal in SIGNAL_NUMBERS) return SIGNAL_NUMBERS[signal];
+  throw new TypeError(`Unknown signal in getSignalNumber: '${signal}'. Please add a case for that signal.`);
+}
+
 async function spawnNpx(args, opts) {
   const npx = /^win/.test(process.platform) ? 'npx.cmd' : 'npx';
   const npxArgs = ['--prefer-offline', '--timing=true', '--yes', '--', ...args];