Skip to content

test(history): respect workflow lock lifecycle in task completion suite - #12971

Open
AdamMagued wants to merge 1 commit into
temporalio:mainfrom
AdamMagued:fix-workflow-task-handler-test-lock
Open

AdamMagued wants to merge 1 commit into
temporalio:mainfrom
AdamMagued:fix-workflow-task-handler-test-lock

Conversation

@AdamMagued

Copy link
Copy Markdown

What changed?

  • Scoped workflow lock acquisition in createSentUpdate with wfContext.Lock and deferred wfContext.Unlock.
  • Protected direct workflow context accesses (LoadMutableState, AddTimerStartedEvent, AddTimerFiredEvent, Clear, TimeoutWorkflow, UpdateWorkflowExecutionAsActive, SubmitClosedWorkflowSnapshot, GetMergedTaskCompletionPages) in TestUpdateWorkflow, TestForceCreateNewWorkflowTaskOnPausedWorkflow, and TestWorkflowTaskCompletionPagination under wfContext.Lock.
  • Removed redundant LoadMutableState invocations following createStartedWorkflow release in deployment-metrics and pagination test cases.

Fixes #12784

Why?

createStartedWorkflow releases the workflow lock back to the cache before returning the workflow context. Subsequent test calls to createSentUpdate and direct mutable state access helpers interacted with mutable state and update registry outside the workflow lock, violating lock assertions in the history unit suite.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests

Executed test suite in container sandbox:
go test -tags test_dep ./service/history/api/respondworkflowtaskcompleted -run WorkflowTaskCompletedHandlerSuite -count=1
go vet ./service/history/api/respondworkflowtaskcompleted

Potential risks

None. Test-only change in service/history/api/respondworkflowtaskcompleted/api_test.go.

Hold workflow locks when accessing workflow context and mutable state
in WorkflowTaskCompletedHandlerSuite. Scope lock acquisition inside
createSentUpdate and direct-access test cases, and drop redundant
LoadMutableState calls after releasing back to cache.

Fixes temporalio#12784
@AdamMagued
AdamMagued requested review from a team as code owners November 6, 2026 12:58

This branch has not been deployed

No deployments
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.

Workflow task completion tests access context after releasing workflow lock

1 participant