Skip to content

Fix more test flakes - #2490

Merged
yuandrew merged 7 commits into
mainfrom
test-flakes-fix-07-23
Jul 28, 2026
Merged

Fix more test flakes#2490
yuandrew merged 7 commits into
mainfrom
test-flakes-fix-07-23

Conversation

@yuandrew

@yuandrew yuandrew commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

What was changed

  1. TestWorkerHeartbeatSuite/TestWorkerHeartbeatBasic exposed a metrics-publication ordering race introduced in Narrow scope of used slots lock for tracking slot supplier #1617. The race became observable after 💥Add worker heartbeat support #2186 began capturing slot metrics in worker heartbeats: an older update could overwrite a newer slot count and leave the heartbeat reporting stale state.

Fixed this by versioning slot-state changes and retrying metrics publication when the state changes during an update, while keeping metrics callbacks outside the slot mutex. Added TestScalableTaskPollerSuite/TestTrackingSlotSupplierPublishesLatestConcurrentState as deterministic regression coverage.

  1. TestTaskHandlersTestSuite/TestLocalActivityRetry_Workflow had a polling goroutine whose exit condition could never be reached: it waited for four local-activity failures, but the activity succeeded after three. When the stoppable local-activity tunnel added in Allow local activities to finish running when stopping a worker #1875 closed during cleanup, PollTask correctly returned nil, and the test passed that nil task to ProcessTask, causing a scheduling-dependent background panic. The runaway-goroutine form also became more visible after CI: Tighten unit test timeout to 5 minutes #2430 reduced the unit-test timeout from 15 minutes to 5 minutes.

Fixed the poll loop by treating nil from the closed tunnel as end-of-stream and returning before calling ProcessTask.

  1. TestIntegrationSuite/TestLocalActivityCompleteWithinGracefulShutdown relied on a 100 ms sleep to assume the first local activity had started before worker shutdown. After Fix GetWorkerStopChannel() for Local Activities #1965 made the activity wait for the worker-stop signal, stopping the worker before the activity began could cause both local activities to fail during shutdown and produce zero successful local-activity markers. This surfaced in Docker Compose CI when worker startup exceeded the assumed delay.

Fixed the test by waiting for an explicit local-activity-started signal before stopping the worker, then waiting for workflow completion before inspecting history.

  1. TestWorkerDeploymentTestSuite/TestDeploymentDrainage, introduced in Support Worker Deployments 3.1 #1832, only waited for the workflow execution to report Running before changing the current deployment version from 1.0 to 2.0. Because Running does not guarantee that the first workflow task has executed, slower CI scheduling could allow that task to reach the 2.0 AutoUpgrade worker before the 1.0 worker recorded its pinned behavior. Fix Worker Deployment test flakes #2333 added routing-configuration propagation checks but did not close this workflow-task race.

Fixed the test by waiting until the workflow is confirmed running on build 1.0 before changing the current version.

  1. TestIntegrationSuite/TestResourceBasedSlotSupplierManyActs, introduced in WorkerTuner & Resource based autotuning #1546, created a new workflow-context layer for each of its 200 local and 200 regular activities. Repeated option and interceptor lookups through these increasingly deep context chains produced quadratic work and could cross the workflow deadlock detector’s one-second threshold on slower race-enabled CI.

Fixed the test workflow by creating one local-activity options context and one regular-activity options context and reusing them for every execution. The test continues to schedule and await all 400 activities.

Why?

green means go

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

Note

Low Risk
Changes are limited to worker metrics publication, local-activity poller shutdown behavior, and test timing/setup; no security or API contract changes beyond documented fixes.

Overview
Addresses several CI flakes and two worker correctness issues around local activity polling and task slot metrics.

trackingSlotSupplier now bumps a usedSlotsVersion on slot use/release and publishMetrics re-reads slot state and retries when the version changes during a gauge update, so concurrent MarkSlotUsed calls cannot leave stale WorkerTaskSlotsUsed (and heartbeats that embed those metrics). A regression test blocks the first gauge callback to prove the second update is not serialized behind a stale publish.

localActivityTaskPoller.PollTask treats a nil task from a closed local-activity tunnel as end-of-stream (nil, nil) instead of passing nil into ProcessTask, avoiding shutdown panics; unit coverage and the local-activity retry test poll loop were updated accordingly.

Integration/unit tests are hardened: graceful shutdown waits for the local activity to start before stopping the worker; deployment drainage waits until the workflow is running on build 1.0 before switching current version; the many-activities workflow reuses single local/regular activity option contexts to avoid quadratic context depth. CHANGELOG entries were reorganized; TestNexusUpdateWorkflowOperation is temporarily skipped pending issue #2479.

Reviewed by Cursor Bugbot for commit 4caaa72. Bugbot is set up for automated code reviews on this repo. Configure here.

@yuandrew
yuandrew marked this pull request as ready for review July 24, 2026 18:25
@yuandrew
yuandrew requested a review from a team as a code owner July 24, 2026 18:25

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 6532631. Configure here.

Comment thread internal/internal_task_handlers_test.go
@yuandrew
yuandrew merged commit ad65494 into main Jul 28, 2026
69 of 72 checks passed
@yuandrew
yuandrew deleted the test-flakes-fix-07-23 branch July 28, 2026 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants