Skip to content

Commit f756351

Browse files
authored
fix: Fix flakes in Worker Tuner and Workflow Update tests (#1463)
1 parent 913e67e commit f756351

File tree

5 files changed

+161
-155
lines changed

5 files changed

+161
-155
lines changed

.github/workflows/ci.yml

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ env:
1919
|| startsWith(github.ref, 'refs/heads/releases'))
2020
&& github.event_name != 'pull_request'
2121
}}
22-
TESTS_CLI_VERSION: 'v0.12.0'
22+
# Use these variables to force specific version of CLI/Time Skipping Server for SDK tests
23+
# TESTS_CLI_VERSION: 'v0.13.2'
24+
# TESTS_TIME_SKIPPING_SERVER_VERSION: 'v1.24.1'
2325

2426
jobs:
2527
# Compile native bridge code for each target platform.
@@ -281,21 +283,28 @@ jobs:
281283
if: matrix.server == 'cli'
282284
shell: bash
283285
run: |
284-
temporal server start-dev --headless &
286+
temporal server start-dev --headless &> /tmp/devserver.log &
285287
286288
- name: Run Tests
287289
run: npm test
288290
env:
289291
RUN_INTEGRATION_TESTS: true
290292
REUSE_V8_CONTEXT: ${{ matrix.reuse-v8-context }}
291293

292-
- name: Upload logs
294+
- name: Upload NPM logs
293295
uses: actions/upload-artifact@v4
294-
if: failure()
296+
if: failure() || cancelled()
295297
with:
296298
name: integration-tests-${{ matrix.platform }}-node${{ matrix.node }}-${{ matrix.server }}-${{ matrix.reuse-v8-context && 'reuse' || 'noreuse' }}-logs
297299
path: ${{ startsWith(matrix.platform, 'windows') && 'C:\\npm\\_logs\\' || '~/.npm/_logs/' }}
298300

301+
- name: Upload Dev Server logs
302+
uses: actions/upload-artifact@v4
303+
if: failure() || cancelled()
304+
with:
305+
name: integration-tests-${{ matrix.platform }}-node${{ matrix.node }}-${{ matrix.server }}-${{ matrix.reuse-v8-context && 'reuse' || 'noreuse' }}-devserver-logs
306+
path: /tmp/devserver.log
307+
299308
# Tests that npm init @temporalio results in a working worker and client
300309
test-npm-init:
301310
needs: build-packages

packages/client/src/workflow-client.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -806,13 +806,15 @@ export class WorkflowClient extends BaseClient {
806806
waitForStage: WorkflowUpdateStage,
807807
input: WorkflowStartUpdateInput
808808
): Promise<WorkflowStartUpdateOutput> {
809+
waitForStage = waitForStage >= WorkflowUpdateStage.ACCEPTED ? waitForStage : WorkflowUpdateStage.ACCEPTED;
810+
const waitForStageProto = workflowUpdateStage.toProtoEnum(waitForStage);
809811
const updateId = input.options?.updateId ?? uuid4();
810812
const req: temporal.api.workflowservice.v1.IUpdateWorkflowExecutionRequest = {
811813
namespace: this.options.namespace,
812814
workflowExecution: input.workflowExecution,
813815
firstExecutionRunId: input.firstExecutionRunId,
814816
waitPolicy: {
815-
lifecycleStage: workflowUpdateStage.toProtoEnum(waitForStage),
817+
lifecycleStage: waitForStageProto,
816818
},
817819
request: {
818820
meta: {
@@ -834,7 +836,7 @@ export class WorkflowClient extends BaseClient {
834836
try {
835837
do {
836838
response = await this.workflowService.updateWorkflowExecution(req);
837-
} while (response.stage < waitForStage && response.stage < WorkflowUpdateStage.ACCEPTED);
839+
} while (response.stage < waitForStageProto);
838840
} catch (err) {
839841
this.rethrowUpdateGrpcError(err, 'Workflow Update failed', input.workflowExecution);
840842
}

packages/test/src/helpers-integration.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
WorkerOptions,
1818
WorkflowBundle,
1919
bundleWorkflowCode,
20+
makeTelemetryFilterString,
2021
} from '@temporalio/worker';
2122
import * as workflow from '@temporalio/workflow';
2223
import { ConnectionInjectorInterceptor } from './activities/interceptors';
@@ -52,8 +53,22 @@ export function makeTestFunction(opts: {
5253
}): TestFn<Context> {
5354
const test = anyTest as TestFn<Context>;
5455
test.before(async (t) => {
56+
const workflowBundle = await bundleWorkflowCode({
57+
...bundlerOptions,
58+
workflowInterceptorModules: [...defaultWorkflowInterceptorModules, ...(opts.workflowInterceptorModules ?? [])],
59+
workflowsPath: opts.workflowsPath,
60+
});
5561
// Ignore invalid log levels
56-
Runtime.install({ logger: new DefaultLogger((process.env.TEST_LOG_LEVEL || 'DEBUG').toUpperCase() as LogLevel) });
62+
Runtime.install({
63+
logger: new DefaultLogger((process.env.TEST_LOG_LEVEL || 'DEBUG').toUpperCase() as LogLevel),
64+
telemetryOptions: {
65+
logging: {
66+
filter: makeTelemetryFilterString({
67+
core: (process.env.TEST_LOG_LEVEL || 'INFO').toUpperCase() as LogLevel,
68+
}),
69+
},
70+
},
71+
});
5772
const env = await TestWorkflowEnvironment.createLocal({
5873
...opts.workflowEnvironmentOpts,
5974
server: {
@@ -65,11 +80,6 @@ export function makeTestFunction(opts: {
6580
},
6681
});
6782
await registerDefaultCustomSearchAttributes(env.connection);
68-
const workflowBundle = await bundleWorkflowCode({
69-
...bundlerOptions,
70-
workflowInterceptorModules: [...defaultWorkflowInterceptorModules, ...(opts.workflowInterceptorModules ?? [])],
71-
workflowsPath: opts.workflowsPath,
72-
});
7383
t.context = {
7484
env,
7585
workflowBundle,

packages/test/src/test-worker-lifecycle.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
/* eslint-disable no-duplicate-imports */
2-
// ^ needed for lint passing in CI
31
/**
42
* Test the various states of a Worker.
53
* Most tests use a mocked core, some tests run serially because they emit signals to the process
@@ -16,7 +14,7 @@ if (RUN_INTEGRATION_TESTS) {
1614
test.serial('Worker shuts down gracefully', async (t) => {
1715
const worker = await Worker.create({
1816
...defaultOptions,
19-
taskQueue: 'shutdown-test',
17+
taskQueue: t.title.replace(/ /g, '_'),
2018
});
2119
t.is(worker.getState(), 'INITIALIZED');
2220
const p = worker.run();
@@ -33,7 +31,7 @@ if (RUN_INTEGRATION_TESTS) {
3331
test.serial('Worker shuts down gracefully if interrupted before running', async (t) => {
3432
const worker = await Worker.create({
3533
...defaultOptions,
36-
taskQueue: 'shutdown-test',
34+
taskQueue: t.title.replace(/ /g, '_'),
3735
});
3836
t.is(worker.getState(), 'INITIALIZED');
3937
process.emit('SIGINT', 'SIGINT');
@@ -47,7 +45,7 @@ if (RUN_INTEGRATION_TESTS) {
4745
await t.throwsAsync(
4846
Worker.create({
4947
...defaultOptions,
50-
taskQueue: 'shutdown-test',
48+
taskQueue: t.title.replace(/ /g, '_'),
5149
namespace: 'oogabooga',
5250
}),
5351
{
@@ -61,7 +59,7 @@ if (RUN_INTEGRATION_TESTS) {
6159
test.serial('Mocked run shuts down gracefully', async (t) => {
6260
try {
6361
const worker = isolateFreeWorker({
64-
taskQueue: 'shutdown-test',
62+
taskQueue: t.title.replace(/ /g, '_'),
6563
});
6664
t.is(worker.getState(), 'INITIALIZED');
6765
const p = worker.run();
@@ -78,7 +76,7 @@ test.serial('Mocked run shuts down gracefully', async (t) => {
7876
test.serial('Mocked run shuts down gracefully if interrupted before running', async (t) => {
7977
try {
8078
const worker = isolateFreeWorker({
81-
taskQueue: 'shutdown-test',
79+
taskQueue: t.title.replace(/ /g, '_'),
8280
});
8381
// worker.native.initiateShutdown = () => new Promise(() => undefined);
8482
t.is(worker.getState(), 'INITIALIZED');
@@ -95,7 +93,7 @@ test.serial('Mocked run shuts down gracefully if interrupted before running', as
9593
test.serial('Mocked run throws if not shut down gracefully', async (t) => {
9694
const worker = isolateFreeWorker({
9795
shutdownForceTime: '5ms',
98-
taskQueue: 'shutdown-test',
96+
taskQueue: t.title.replace(/ /g, '_'),
9997
});
10098
t.is(worker.getState(), 'INITIALIZED');
10199
const p = worker.run();
@@ -113,7 +111,7 @@ test.serial('Mocked run throws if not shut down gracefully', async (t) => {
113111
test.serial('Mocked throws combined error in runUntil', async (t) => {
114112
const worker = isolateFreeWorker({
115113
shutdownForceTime: '5ms',
116-
taskQueue: 'shutdown-test',
114+
taskQueue: t.title.replace(/ /g, '_'),
117115
});
118116
worker.native.initiateShutdown = () => new Promise(() => undefined);
119117
const err = await t.throwsAsync(

0 commit comments

Comments
 (0)