From 741aeb25b0b7ea88fc1a5cf1336a6b6afadac529 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Tue, 18 Nov 2025 16:23:36 -0500 Subject: [PATCH 1/9] ci: do not make HTTP request to third party server in the 'fetch-esm' sample test --- .github/workflows/ci.yml | 12 ++++++------ scripts/test-example.js | 12 ++++++++---- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9659c0aae..31bf2f7f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,12 +60,12 @@ jobs: run: shell: bash steps: - - name: 'Checkout code' + - name: "Checkout code" uses: actions/checkout@v4 with: submodules: recursive - - name: 'Cache index.node' + - name: "Cache index.node" id: cached-artifact uses: actions/cache@v4 with: @@ -77,7 +77,7 @@ jobs: uses: arduino/setup-protoc@v3 with: # TODO: Upgrade proto once https://github.com/arduino/setup-protoc/issues/99 is fixed - version: '23.x' + version: "23.x" repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Upgrade Rust to latest stable @@ -90,7 +90,7 @@ jobs: workspaces: packages/core-bridge -> target prefix-key: corebridge-buildcache-debug shared-key: ${{ matrix.platform }} - env-vars: '' + env-vars: "" save-if: ${{ env.IS_MAIN_OR_RELEASE == 'true' }} - name: Compile rust code @@ -148,7 +148,7 @@ jobs: - name: Set git config run: git config --global core.autocrlf false - - name: 'Checkout code' + - name: "Checkout code" uses: actions/checkout@v4 with: submodules: recursive @@ -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 diff --git a/scripts/test-example.js b/scripts/test-example.js index 0152b00c0..cd4a080c8 100644 --- a/scripts/test-example.js +++ b/scripts/test-example.js @@ -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,7 +33,7 @@ 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]}"`); } } @@ -41,13 +41,17 @@ async function test(workdir) { 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() From 3c1439e989ed8402ce7e0562f1d9f562867b5c02 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Tue, 18 Nov 2025 17:56:01 -0500 Subject: [PATCH 2/9] Fix typo --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3deabe783..634cb47a1 100644 --- a/.github/workflows/ci.yml +++ b/.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 --script-name workflow-local --expected-output 'Hello World And Hello Wonderful Temporal!'" + 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 From a2268faede81d372f03a6e97aea1e546375efa7c Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Tue, 18 Nov 2025 18:09:45 -0500 Subject: [PATCH 3/9] format --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 634cb47a1..5f679c371 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,12 +60,12 @@ jobs: run: shell: bash steps: - - name: "Checkout code" + - name: 'Checkout code' uses: actions/checkout@v4 with: submodules: recursive - - name: "Cache index.node" + - name: 'Cache index.node' id: cached-artifact uses: actions/cache@v4 with: @@ -77,7 +77,7 @@ jobs: uses: arduino/setup-protoc@v3 with: # TODO: Upgrade proto once https://github.com/arduino/setup-protoc/issues/99 is fixed - version: "23.x" + version: '23.x' repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Upgrade Rust to latest stable @@ -90,7 +90,7 @@ jobs: workspaces: packages/core-bridge -> target prefix-key: corebridge-buildcache-debug shared-key: ${{ matrix.platform }} - env-vars: "" + env-vars: '' save-if: ${{ env.IS_MAIN_OR_RELEASE == 'true' }} - name: Compile rust code @@ -148,7 +148,7 @@ jobs: - name: Set git config run: git config --global core.autocrlf false - - name: "Checkout code" + - name: 'Checkout code' uses: actions/checkout@v4 with: submodules: recursive From e84b424205d3c2276a47e9a7696a58cac5ffbd78 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Wed, 19 Nov 2025 00:16:51 -0500 Subject: [PATCH 4/9] Address more flakes --- packages/test/src/test-integration-workflows.ts | 2 +- packages/test/src/test-prometheus.ts | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/test/src/test-integration-workflows.ts b/packages/test/src/test-integration-workflows.ts index 32620bee8..32a43781c 100644 --- a/packages/test/src/test-integration-workflows.ts +++ b/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(); diff --git a/packages/test/src/test-prometheus.ts b/packages/test/src/test-prometheus.ts index 264547402..18c5d7964 100644 --- a/packages/test/src/test-prometheus.ts +++ b/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(); From 83e0bd4593b28286c7f6b23c2fd3a82256971487 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Wed, 19 Nov 2025 00:39:35 -0500 Subject: [PATCH 5/9] test-example: Fix race on worker exit --- scripts/test-example.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/test-example.js b/scripts/test-example.js index cd4a080c8..d124ce8bd 100644 --- a/scripts/test-example.js +++ b/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 } = require('./utils'); const npm = /^win/.test(process.platform) ? 'npm.cmd' : 'npm'; @@ -19,7 +19,11 @@ async function withWorker(workdir, fn) { try { return await fn(); } finally { - await kill(worker); + await Promise.race([ + // Give 1s for the worker to terminate by itself, then send a SIGINT + waitOnChild(worker), + sleep(1000).then(() => kill(worker)), + ]); } } From 0bdbb9416fbf7b8100605aeaf89c7130b2df0c69 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Wed, 19 Nov 2025 01:13:31 -0500 Subject: [PATCH 6/9] Add missing import --- scripts/test-example.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test-example.js b/scripts/test-example.js index d124ce8bd..fd8b0af30 100644 --- a/scripts/test-example.js +++ b/scripts/test-example.js @@ -1,6 +1,6 @@ const { spawn: spawnChild, spawnSync } = require('child_process'); const arg = require('arg'); -const { shell, kill, sleep } = require('./utils'); +const { shell, kill, sleep, waitOnChild } = require('./utils'); const npm = /^win/.test(process.platform) ? 'npm.cmd' : 'npm'; From d2284252b056e8027fdc20e542d151191daacbd6 Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Thu, 20 Nov 2025 11:09:26 -0500 Subject: [PATCH 7/9] Fix another flake --- scripts/utils.ts | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 scripts/utils.ts diff --git a/scripts/utils.ts b/scripts/utils.ts new file mode 100644 index 000000000..261976815 --- /dev/null +++ b/scripts/utils.ts @@ -0,0 +1,77 @@ +import { ChildProcess, spawn, SpawnOptions } from 'node:child_process'; + +export class ChildProcessError extends Error { + constructor( + message: string, + public readonly code: number | null, + public readonly signal: NodeJS.Signals | null + ) { + super(message); + this.name = 'ChildProcessError'; + } +} + +export const shell = /^win/.test(process.platform); + +export async function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +export async function waitOnChild(child: ChildProcess): Promise { + return new Promise((resolve, reject) => { + child.on('exit', (code: number, signal: NodeJS.Signals) => { + if (code === 0) { + resolve(); + } else { + reject(new ChildProcessError('Process failed', code, signal as NodeJS.Signals)); + } + }); + child.on('error', reject); + }); +} + +export async function kill(child: ChildProcess, signal: NodeJS.Signals = 'SIGINT'): Promise { + if (typeof child.pid !== 'number') { + throw new TypeError('Expected child with pid'); + } + + // -PID not supported on Windows + if (process.platform === 'win32') { + process.kill(child.pid, signal); + } else { + process.kill(-child.pid, signal); + } + + try { + await waitOnChild(child); + } catch (err: unknown) { + 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: NodeJS.Signals): number { + // 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, + } as const satisfies Partial>; + + if (signal in SIGNAL_NUMBERS) return SIGNAL_NUMBERS[signal]; + throw new TypeError(`Unknown signal in getSignalNumber: '${signal}'. Please add a case for that signal.`); +} + +export async function spawnNpx(args: string[], opts: SpawnOptions): Promise { + const npx = /^win/.test(process.platform) ? 'npx.cmd' : 'npx'; + const npxArgs = ['--prefer-offline', '--timing=true', '--yes', '--', ...args]; + await waitOnChild(spawn(npx, npxArgs, { ...opts, shell })); +} From 8643867847c759f4e09fb9548b67aebe8e840c0c Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Thu, 20 Nov 2025 12:05:27 -0500 Subject: [PATCH 8/9] Fighting with exit code on signals --- scripts/test-example.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/scripts/test-example.js b/scripts/test-example.js index fd8b0af30..914d88a57 100644 --- a/scripts/test-example.js +++ b/scripts/test-example.js @@ -19,11 +19,7 @@ async function withWorker(workdir, fn) { try { return await fn(); } finally { - await Promise.race([ - // Give 1s for the worker to terminate by itself, then send a SIGINT - waitOnChild(worker), - sleep(1000).then(() => kill(worker)), - ]); + await kill(worker); } } From 1d82a616d39294af9184209781d4423b51250f5d Mon Sep 17 00:00:00 2001 From: James Watkins-Harvey Date: Thu, 20 Nov 2025 13:04:03 -0500 Subject: [PATCH 9/9] Had changed the worng file --- scripts/utils.js | 29 ++++++++++++++---- scripts/utils.ts | 77 ------------------------------------------------ 2 files changed, 23 insertions(+), 83 deletions(-) delete mode 100644 scripts/utils.ts diff --git a/scripts/utils.js b/scripts/utils.js index 4f82aeef4..ce5ed0d65 100644 --- a/scripts/utils.js +++ b/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]; diff --git a/scripts/utils.ts b/scripts/utils.ts deleted file mode 100644 index 261976815..000000000 --- a/scripts/utils.ts +++ /dev/null @@ -1,77 +0,0 @@ -import { ChildProcess, spawn, SpawnOptions } from 'node:child_process'; - -export class ChildProcessError extends Error { - constructor( - message: string, - public readonly code: number | null, - public readonly signal: NodeJS.Signals | null - ) { - super(message); - this.name = 'ChildProcessError'; - } -} - -export const shell = /^win/.test(process.platform); - -export async function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - -export async function waitOnChild(child: ChildProcess): Promise { - return new Promise((resolve, reject) => { - child.on('exit', (code: number, signal: NodeJS.Signals) => { - if (code === 0) { - resolve(); - } else { - reject(new ChildProcessError('Process failed', code, signal as NodeJS.Signals)); - } - }); - child.on('error', reject); - }); -} - -export async function kill(child: ChildProcess, signal: NodeJS.Signals = 'SIGINT'): Promise { - if (typeof child.pid !== 'number') { - throw new TypeError('Expected child with pid'); - } - - // -PID not supported on Windows - if (process.platform === 'win32') { - process.kill(child.pid, signal); - } else { - process.kill(-child.pid, signal); - } - - try { - await waitOnChild(child); - } catch (err: unknown) { - 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: NodeJS.Signals): number { - // 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, - } as const satisfies Partial>; - - if (signal in SIGNAL_NUMBERS) return SIGNAL_NUMBERS[signal]; - throw new TypeError(`Unknown signal in getSignalNumber: '${signal}'. Please add a case for that signal.`); -} - -export async function spawnNpx(args: string[], opts: SpawnOptions): Promise { - const npx = /^win/.test(process.platform) ? 'npx.cmd' : 'npx'; - const npxArgs = ['--prefer-offline', '--timing=true', '--yes', '--', ...args]; - await waitOnChild(spawn(npx, npxArgs, { ...opts, shell })); -}