mirror of
https://github.com/hanzoai/tasks.git
synced 2026-10-04 02:04:03 +00:00
Fix two flaky/broken functional tests (#9660)
## What changed? 1. tests/task_queue_stats_test.go — Remove the `t *testing.T` field from `taskQueueStatsSuite` and replace all `s.t` usages with `s.T()`. Critically, fix `validateDescribeWorkerDeploymentVersion` to use `a.NoError(err)` instead of `require.NoError(s.T(), err)` inside the callback. 2. tests/xdc/history_replication_signals_and_updates_test.go — Fix `pollWorkflowResult` to handle errors from `GetWorkflowExecutionHistory` gracefully: return the error from the inner function, retry with a nil page token on transient errors (e.g. CurrentBranchChanged after conflict resolution), and abort cleanly when the context expires. ## Why? TestTaskQueueStats panic: Calling require.NoError(s.T(), err) inside an EventuallyWithT callback is unsafe. EventuallyWithT runs its callback in a separate goroutine on a ticker. If the outer test context expires and the test completes while this callback is still running, the require.NoError call invokes t.Fail() on a completed test, which panics the entire test binary with panic: Fail in goroutine after TestXxx has completed. The fix uses the CollectT-scoped a.NoError(err) so assertions are buffered and safe. TestConflictResolutionGetResult hang/crash: pollWorkflowResult was calling responseInner.History.Events without checking if responseInner was nil. When GetWorkflowExecutionHistory returns an error (e.g. CurrentBranchChanged after conflict resolution resets the branch token), the response is nil, causing a nil pointer dereference that panics the goroutine. Because the goroutine crashed before sending to workflowResultCh, the main test goroutine blocked on <-workflowResultCh for the entire 30-minute CI timeout. The fix returns the error from the inner function and retries with a nil page token on transient errors, which allows the poll to succeed on the updated branch. ## How did you test it? - [ ] built - [X] run locally and tested manually - [X] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks The XDC fix changes `pollWorkflowResult` to retry indefinitely on any non-context error. If a non-transient error were to occur repeatedly, this could loop until the context expires rather than failing immediately. This is acceptable since the context timeout provides a hard bound, and the test failure message via `c.t.s.NoError(ctx.Err(), ...)` will make the root cause clear.
This commit is contained in:
@@ -1573,7 +1573,7 @@ func (s *taskQueueStatsSuite) validateDescribeWorkerDeploymentVersion(
|
||||
|
||||
req.ReportTaskQueueStats = true
|
||||
resp, err := s.FrontendClient().DescribeWorkerDeploymentVersion(ctx, req)
|
||||
require.NoError(s.T(), err)
|
||||
a.NoError(err)
|
||||
a.Len(resp.VersionTaskQueues, 2, "should be 1 task queue for Workflows and 1 for Activities")
|
||||
|
||||
for _, info := range resp.VersionTaskQueues {
|
||||
|
||||
@@ -1046,7 +1046,7 @@ func (c *hrsuTestCluster) getHistoryForRunId(ctx context.Context, runId string)
|
||||
}
|
||||
|
||||
func (c *hrsuTestCluster) pollWorkflowResult(ctx context.Context, runId string) *historypb.HistoryEvent {
|
||||
getHistoryWithLongPoll := func(token []byte) ([]*historypb.HistoryEvent, []byte) {
|
||||
getHistoryWithLongPoll := func(token []byte) ([]*historypb.HistoryEvent, []byte, error) {
|
||||
responseInner, err := c.testCluster.FrontendClient().GetWorkflowExecutionHistory(ctx, &workflowservice.GetWorkflowExecutionHistoryRequest{
|
||||
Namespace: c.t.tv.NamespaceName().String(),
|
||||
Execution: &commonpb.WorkflowExecution{
|
||||
@@ -1058,25 +1058,33 @@ func (c *hrsuTestCluster) pollWorkflowResult(ctx context.Context, runId string)
|
||||
NextPageToken: token,
|
||||
HistoryEventFilterType: enumspb.HISTORY_EVENT_FILTER_TYPE_CLOSE_EVENT,
|
||||
})
|
||||
c.t.s.NoError(err)
|
||||
return responseInner.History.Events, responseInner.NextPageToken
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
}
|
||||
return responseInner.History.Events, responseInner.NextPageToken, nil
|
||||
}
|
||||
|
||||
var token []byte
|
||||
var allEvents []*historypb.HistoryEvent
|
||||
multiPoll := false
|
||||
for {
|
||||
events, nextPageToken := getHistoryWithLongPoll(token)
|
||||
if ctx.Err() != nil {
|
||||
c.t.s.NoError(ctx.Err(), "context expired while waiting for workflow result")
|
||||
return nil
|
||||
}
|
||||
events, nextPageToken, err := getHistoryWithLongPoll(token)
|
||||
if err != nil {
|
||||
// Transient error (e.g. CurrentBranchChanged after conflict resolution): retry from scratch.
|
||||
token = nil
|
||||
continue
|
||||
}
|
||||
allEvents = append(allEvents, events...)
|
||||
if nextPageToken == nil {
|
||||
break
|
||||
}
|
||||
token = nextPageToken
|
||||
multiPoll = true
|
||||
}
|
||||
|
||||
c.t.s.Len(allEvents, 1)
|
||||
c.t.s.True(multiPoll, "Expected to have multiple polls of history events")
|
||||
return allEvents[0]
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user