From 24ea36f1c40004d19fd6dbeb2d882a8119d38fc5 Mon Sep 17 00:00:00 2001 From: Tim Li Date: Mon, 6 May 2024 11:21:48 -0700 Subject: [PATCH 1/4] Added unit tests for history handlers --- service/history/handler/handler_test.go | 387 ++++++++++++++++++++++++ 1 file changed, 387 insertions(+) diff --git a/service/history/handler/handler_test.go b/service/history/handler/handler_test.go index 9c2c8336ef7..f85e90e9ebc 100644 --- a/service/history/handler/handler_test.go +++ b/service/history/handler/handler_test.go @@ -764,6 +764,393 @@ func (s *handlerSuite) TestRespondActivityTaskFailed() { } } +func (s *handlerSuite) TestRespondActivityTaskCanceled() { + testInput := map[string]struct { + caseName string + input *types.HistoryRespondActivityTaskCanceledRequest + expectedError bool + }{ + "valid input": { + caseName: "valid input", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: false, + }, + "empty domainID": { + caseName: "empty domainID", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: "", + }, + expectedError: true, + }, + "ratelimit exceeded": { + caseName: "ratelimit exceeded", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: true, + }, + "token deserialization error": { + caseName: "token deserialization error", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: true, + }, + "invalid task token": { + caseName: "invalid task token", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: true, + }, + "get engine error": { + caseName: "get engine error", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: true, + }, + "engine error": { + caseName: "engine error", + input: &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: true, + }, + } + + for name, input := range testInput { + s.Run(name, func() { + switch input.caseName { + case "valid input": + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), input.input).Return(nil).Times(1) + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + case "empty domainID": + case "ratelimit exceeded": + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + case "token deserialization error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + case "invalid task token": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) + case "get engine error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) + case "engine error": + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), input.input).Return(errors.New("error")).Times(1) + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + } + err := s.handler.RespondActivityTaskCanceled(context.Background(), input.input) + if input.expectedError { + s.Error(err) + } else { + s.NoError(err) + } + }) + } +} + +func (s *handlerSuite) TestRespondDecisionTaskCompleted() { + validResp := &types.HistoryRespondDecisionTaskCompletedResponse{ + StartedResponse: &types.RecordDecisionTaskStartedResponse{ + WorkflowType: &types.WorkflowType{}, + }, + } + testInput := map[string]struct { + caseName string + input *types.HistoryRespondDecisionTaskCompletedRequest + expectedError bool + }{ + "valid input": { + caseName: "valid input", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{ + { + DecisionType: types.DecisionTypeScheduleActivityTask.Ptr(), + }, + }, + ExecutionContext: nil, + Identity: "identity", + }, + }, + expectedError: false, + }, + "empty domainID": { + caseName: "empty domainID", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: "", + }, + expectedError: true, + }, + "ratelimit exceeded": { + caseName: "ratelimit exceeded", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{ + { + DecisionType: types.DecisionTypeScheduleActivityTask.Ptr(), + }, + }, + }, + }, + expectedError: true, + }, + "token deserialization error": { + caseName: "token deserialization error", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{}, + }, + }, + expectedError: true, + }, + "invalid task token": { + caseName: "invalid task token", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{}, + }, + }, + expectedError: true, + }, + "get engine error": { + caseName: "get engine error", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{}, + }, + }, + expectedError: true, + }, + "engine error": { + caseName: "engine error", + input: &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{}, + }, + }, + expectedError: true, + }, + } + + for name, input := range testInput { + s.Run(name, func() { + switch input.caseName { + case "valid input": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), input.input).Return(validResp, nil).Times(1) + case "empty domainID": + case "ratelimit exceeded": + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + case "token deserialization error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + case "invalid task token": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) + case "get engine error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) + case "engine error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), input.input).Return(nil, errors.New("error")).Times(1) + } + resp, err := s.handler.RespondDecisionTaskCompleted(context.Background(), input.input) + if input.expectedError { + s.Nil(resp) + s.Error(err) + } else { + s.NotNil(resp) + s.NoError(err) + } + }) + + } +} + +func (s *handlerSuite) TestRespondDecisionTaskFailed() { + validInput := &types.HistoryRespondDecisionTaskFailedRequest{ + DomainUUID: testDomainID, + FailedRequest: &types.RespondDecisionTaskFailedRequest{ + TaskToken: []byte("task-token"), + Cause: types.DecisionTaskFailedCauseBadBinary.Ptr(), + Details: []byte("Details"), + Identity: "identity", + }, + } + testInput := map[string]struct { + caseName string + input *types.HistoryRespondDecisionTaskFailedRequest + expectedError bool + }{ + "valid input": { + caseName: "valid input", + input: validInput, + expectedError: false, + }, + "empty domainID": { + caseName: "empty domainID", + input: &types.HistoryRespondDecisionTaskFailedRequest{ + DomainUUID: "", + }, + expectedError: true, + }, + "ratelimit exceeded": { + caseName: "ratelimit exceeded", + input: validInput, + expectedError: true, + }, + "token deserialization error": { + caseName: "token deserialization error", + input: validInput, + expectedError: true, + }, + "invalid task token": { + caseName: "invalid task token", + input: validInput, + expectedError: true, + }, + "get engine error": { + caseName: "get engine error", + input: validInput, + expectedError: true, + }, + "engine error": { + caseName: "engine error", + input: validInput, + expectedError: true, + }, + } + + for name, input := range testInput { + s.Run(name, func() { + switch input.caseName { + case "valid input": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) + case "empty domainID": + case "ratelimit exceeded": + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + case "token deserialization error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + case "invalid task token": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) + case "get engine error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) + case "engine error": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(errors.New("error")).Times(1) + } + err := s.handler.RespondDecisionTaskFailed(context.Background(), input.input) + if input.expectedError { + s.Error(err) + } else { + s.NoError(err) + } + }) + } +} + func (s *handlerSuite) TestGetCrossClusterTasks() { numShards := 10 targetCluster := cluster.TestAlternativeClusterName From 581712d31498305096706f8ed354a9e98895a8eb Mon Sep 17 00:00:00 2001 From: Tim Li Date: Mon, 6 May 2024 14:04:07 -0700 Subject: [PATCH 2/4] Added more unit tests for history handler --- service/history/handler/handler_test.go | 71 +++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/service/history/handler/handler_test.go b/service/history/handler/handler_test.go index f85e90e9ebc..21396939036 100644 --- a/service/history/handler/handler_test.go +++ b/service/history/handler/handler_test.go @@ -1100,6 +1100,32 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { input: validInput, expectedError: true, }, + "special failure": { + caseName: "special failure", + input: &types.HistoryRespondDecisionTaskFailedRequest{ + DomainUUID: testDomainID, + FailedRequest: &types.RespondDecisionTaskFailedRequest{ + TaskToken: []byte("task-token"), + Cause: types.DecisionTaskFailedCauseUnhandledDecision.Ptr(), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: false, + }, + "special failure2": { + caseName: "special failure", + input: &types.HistoryRespondDecisionTaskFailedRequest{ + DomainUUID: testDomainID, + FailedRequest: &types.RespondDecisionTaskFailedRequest{ + TaskToken: []byte("task-token"), + Cause: types.DecisionTaskFailedCauseUnhandledDecision.Ptr(), + Details: []byte("Details"), + Identity: "identity", + }, + }, + expectedError: false, + }, } for name, input := range testInput { @@ -1140,6 +1166,25 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { }, nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(errors.New("error")).Times(1) + case "special failure": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return(testDomainID, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) + case "special failure2": + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return("", errors.New("error")).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) + } err := s.handler.RespondDecisionTaskFailed(context.Background(), input.input) if input.expectedError { @@ -1151,6 +1196,32 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { } } +func (s *handlerSuite) TestDescribeHistoryHost() { + request := &types.DescribeHistoryHostRequest{ + HostAddress: common.StringPtr("test"), + } + + mockStatus := map[string]int32{ + "initialized": 0, + "started": 1, + "stopped": 2, + } + + for status, value := range mockStatus { + s.mockResource.DomainCache.EXPECT().GetCacheSize().Return(int64(2), int64(3)).Times(1) + s.mockShardController.EXPECT().Status().Return(value).Times(1) + s.mockShardController.EXPECT().NumShards().Return(1) + s.mockShardController.EXPECT().ShardIDs().Return([]int32{0}) + resp, err := s.handler.DescribeHistoryHost(context.Background(), request) + s.NoError(err) + s.Equal(resp.DomainCache, &types.DomainCacheInfo{ + NumOfItemsInCacheByID: 2, + NumOfItemsInCacheByName: 3, + }) + s.Equal(resp.ShardControllerStatus, status) + } +} + func (s *handlerSuite) TestGetCrossClusterTasks() { numShards := 10 targetCluster := cluster.TestAlternativeClusterName From 24a07ad49ae0f583db0412cc7f3ac8aa515299e0 Mon Sep 17 00:00:00 2001 From: Tim Li Date: Mon, 6 May 2024 19:12:02 -0700 Subject: [PATCH 3/4] Respond to comments --- service/history/handler/handler_test.go | 358 +++++++++++------------- 1 file changed, 168 insertions(+), 190 deletions(-) diff --git a/service/history/handler/handler_test.go b/service/history/handler/handler_test.go index 21396939036..42a2d131c4e 100644 --- a/service/history/handler/handler_test.go +++ b/service/history/handler/handler_test.go @@ -765,32 +765,40 @@ func (s *handlerSuite) TestRespondActivityTaskFailed() { } func (s *handlerSuite) TestRespondActivityTaskCanceled() { + validInput := &types.HistoryRespondActivityTaskCanceledRequest{ + DomainUUID: testDomainID, + CancelRequest: &types.RespondActivityTaskCanceledRequest{ + TaskToken: []byte("task-token"), + Details: []byte("Details"), + Identity: "identity", + }, + } testInput := map[string]struct { - caseName string input *types.HistoryRespondActivityTaskCanceledRequest expectedError bool + mockFn func() }{ "valid input": { - caseName: "valid input", - input: &types.HistoryRespondActivityTaskCanceledRequest{ - DomainUUID: testDomainID, - CancelRequest: &types.RespondActivityTaskCanceledRequest{ - TaskToken: []byte("task-token"), - Details: []byte("Details"), - Identity: "identity", - }, - }, + input: validInput, expectedError: false, + mockFn: func() { + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), validInput).Return(nil).Times(1) + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + }, }, "empty domainID": { - caseName: "empty domainID", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: "", }, expectedError: true, + mockFn: func() {}, }, "ratelimit exceeded": { - caseName: "ratelimit exceeded", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: testDomainID, CancelRequest: &types.RespondActivityTaskCanceledRequest{ @@ -800,9 +808,11 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { }, }, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + }, }, "token deserialization error": { - caseName: "token deserialization error", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: testDomainID, CancelRequest: &types.RespondActivityTaskCanceledRequest{ @@ -812,9 +822,12 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { }, }, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + }, }, "invalid task token": { - caseName: "invalid task token", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: testDomainID, CancelRequest: &types.RespondActivityTaskCanceledRequest{ @@ -824,9 +837,15 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { }, }, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) + }, }, "get engine error": { - caseName: "get engine error", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: testDomainID, CancelRequest: &types.RespondActivityTaskCanceledRequest{ @@ -836,9 +855,16 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { }, }, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) + }, }, "engine error": { - caseName: "engine error", input: &types.HistoryRespondActivityTaskCanceledRequest{ DomainUUID: testDomainID, CancelRequest: &types.RespondActivityTaskCanceledRequest{ @@ -848,48 +874,21 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { }, }, expectedError: true, - }, - } - - for name, input := range testInput { - s.Run(name, func() { - switch input.caseName { - case "valid input": + mockFn: func() { s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ WorkflowID: testWorkflowID, RunID: testValidUUID, }, nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), input.input).Return(nil).Times(1) - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - case "empty domainID": - case "ratelimit exceeded": - s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) - case "token deserialization error": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) - case "invalid task token": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: "", - RunID: "", - }, nil).Times(1) - case "get engine error": + s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), validInput).Return(errors.New("error")).Times(1) s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) - case "engine error": - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondActivityTaskCanceled(gomock.Any(), input.input).Return(errors.New("error")).Times(1) - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - } + }, + }, + } + + for name, input := range testInput { + s.Run(name, func() { + input.mockFn() err := s.handler.RespondActivityTaskCanceled(context.Background(), input.input) if input.expectedError { s.Error(err) @@ -901,57 +900,57 @@ func (s *handlerSuite) TestRespondActivityTaskCanceled() { } func (s *handlerSuite) TestRespondDecisionTaskCompleted() { + validReq := &types.HistoryRespondDecisionTaskCompletedRequest{ + DomainUUID: testDomainID, + CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ + TaskToken: []byte("task-token"), + Decisions: []*types.Decision{ + { + DecisionType: types.DecisionTypeScheduleActivityTask.Ptr(), + }, + }, + ExecutionContext: nil, + Identity: "identity", + }, + } validResp := &types.HistoryRespondDecisionTaskCompletedResponse{ StartedResponse: &types.RecordDecisionTaskStartedResponse{ WorkflowType: &types.WorkflowType{}, }, } testInput := map[string]struct { - caseName string input *types.HistoryRespondDecisionTaskCompletedRequest expectedError bool + mockFn func() }{ "valid input": { - caseName: "valid input", - input: &types.HistoryRespondDecisionTaskCompletedRequest{ - DomainUUID: testDomainID, - CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ - TaskToken: []byte("task-token"), - Decisions: []*types.Decision{ - { - DecisionType: types.DecisionTypeScheduleActivityTask.Ptr(), - }, - }, - ExecutionContext: nil, - Identity: "identity", - }, - }, + input: validReq, expectedError: false, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), validReq).Return(validResp, nil).Times(1) + }, }, "empty domainID": { - caseName: "empty domainID", input: &types.HistoryRespondDecisionTaskCompletedRequest{ DomainUUID: "", }, expectedError: true, + mockFn: func() {}, }, "ratelimit exceeded": { - caseName: "ratelimit exceeded", - input: &types.HistoryRespondDecisionTaskCompletedRequest{ - DomainUUID: testDomainID, - CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ - TaskToken: []byte("task-token"), - Decisions: []*types.Decision{ - { - DecisionType: types.DecisionTypeScheduleActivityTask.Ptr(), - }, - }, - }, - }, + input: validReq, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + }, }, "token deserialization error": { - caseName: "token deserialization error", input: &types.HistoryRespondDecisionTaskCompletedRequest{ DomainUUID: testDomainID, CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ @@ -960,9 +959,12 @@ func (s *handlerSuite) TestRespondDecisionTaskCompleted() { }, }, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + }, }, "invalid task token": { - caseName: "invalid task token", input: &types.HistoryRespondDecisionTaskCompletedRequest{ DomainUUID: testDomainID, CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ @@ -971,20 +973,15 @@ func (s *handlerSuite) TestRespondDecisionTaskCompleted() { }, }, expectedError: true, - }, - "get engine error": { - caseName: "get engine error", - input: &types.HistoryRespondDecisionTaskCompletedRequest{ - DomainUUID: testDomainID, - CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ - TaskToken: []byte("task-token"), - Decisions: []*types.Decision{}, - }, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) }, - expectedError: true, }, - "engine error": { - caseName: "engine error", + "get engine error": { input: &types.HistoryRespondDecisionTaskCompletedRequest{ DomainUUID: testDomainID, CompleteRequest: &types.RespondDecisionTaskCompletedRequest{ @@ -993,48 +990,33 @@ func (s *handlerSuite) TestRespondDecisionTaskCompleted() { }, }, expectedError: true, - }, - } - - for name, input := range testInput { - s.Run(name, func() { - switch input.caseName { - case "valid input": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), input.input).Return(validResp, nil).Times(1) - case "empty domainID": - case "ratelimit exceeded": - s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) - case "token deserialization error": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) - case "invalid task token": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: "", - RunID: "", - }, nil).Times(1) - case "get engine error": + mockFn: func() { s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ WorkflowID: testWorkflowID, RunID: testValidUUID, }, nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) - case "engine error": + }, + }, + "engine error": { + input: validReq, + expectedError: true, + mockFn: func() { s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ WorkflowID: testWorkflowID, RunID: testValidUUID, }, nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), input.input).Return(nil, errors.New("error")).Times(1) - } + s.mockEngine.EXPECT().RespondDecisionTaskCompleted(gomock.Any(), validReq).Return(nil, errors.New("error")).Times(1) + }, + }, + } + + for name, input := range testInput { + s.Run(name, func() { + input.mockFn() resp, err := s.handler.RespondDecisionTaskCompleted(context.Background(), input.input) if input.expectedError { s.Nil(resp) @@ -1059,49 +1041,82 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { }, } testInput := map[string]struct { - caseName string input *types.HistoryRespondDecisionTaskFailedRequest expectedError bool + mockFn func() }{ "valid input": { - caseName: "valid input", input: validInput, expectedError: false, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(nil).Times(1) + }, }, "empty domainID": { - caseName: "empty domainID", input: &types.HistoryRespondDecisionTaskFailedRequest{ DomainUUID: "", }, expectedError: true, + mockFn: func() {}, }, "ratelimit exceeded": { - caseName: "ratelimit exceeded", input: validInput, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) + }, }, "token deserialization error": { - caseName: "token deserialization error", input: validInput, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) + }, }, "invalid task token": { - caseName: "invalid task token", input: validInput, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: "", + RunID: "", + }, nil).Times(1) + }, }, "get engine error": { - caseName: "get engine error", input: validInput, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) + }, }, "engine error": { - caseName: "engine error", input: validInput, expectedError: true, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(errors.New("error")).Times(1) + }, }, "special failure": { - caseName: "special failure", input: &types.HistoryRespondDecisionTaskFailedRequest{ DomainUUID: testDomainID, FailedRequest: &types.RespondDecisionTaskFailedRequest{ @@ -1112,9 +1127,17 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { }, }, expectedError: false, + mockFn: func() { + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ + WorkflowID: testWorkflowID, + RunID: testValidUUID, + }, nil).Times(1) + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(nil).Times(1) + }, }, "special failure2": { - caseName: "special failure", input: &types.HistoryRespondDecisionTaskFailedRequest{ DomainUUID: testDomainID, FailedRequest: &types.RespondDecisionTaskFailedRequest{ @@ -1125,48 +1148,7 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { }, }, expectedError: false, - }, - } - - for name, input := range testInput { - s.Run(name, func() { - switch input.caseName { - case "valid input": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) - case "empty domainID": - case "ratelimit exceeded": - s.mockRatelimiter.EXPECT().Allow().Return(false).Times(1) - case "token deserialization error": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(nil, errors.New("some random error")).Times(1) - case "invalid task token": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: "", - RunID: "", - }, nil).Times(1) - case "get engine error": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(nil, errors.New("error")).Times(1) - case "engine error": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(errors.New("error")).Times(1) - case "special failure": + mockFn: func() { s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ WorkflowID: testWorkflowID, @@ -1174,18 +1156,14 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { }, nil).Times(1) s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return(testDomainID, nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) - case "special failure2": - s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) - s.mockTokenSerializer.EXPECT().Deserialize(gomock.Any()).Return(&common.TaskToken{ - WorkflowID: testWorkflowID, - RunID: testValidUUID, - }, nil).Times(1) - s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return("", errors.New("error")).Times(1) - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), input.input).Return(nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(nil).Times(1) + }, + }, + } - } + for name, input := range testInput { + s.Run(name, func() { + input.mockFn() err := s.handler.RespondDecisionTaskFailed(context.Background(), input.input) if input.expectedError { s.Error(err) @@ -1338,9 +1316,9 @@ func (s *handlerSuite) TestStartWorkflowExecution() { RunID: testWorkflowRunID, } - s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).AnyTimes() - s.mockRatelimiter.EXPECT().Allow().Return(true).AnyTimes() - s.mockEngine.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any()).Return(expectedResponse, nil).AnyTimes() + s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) + s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) + s.mockEngine.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any()).Return(expectedResponse, nil).Times(1) response, err := s.handler.StartWorkflowExecution(context.Background(), request) s.Equal(expectedResponse, response) From 5655c0fdf62f22f723fd744c47e3b1f869d6be9a Mon Sep 17 00:00:00 2001 From: Tim Li Date: Tue, 7 May 2024 09:03:35 -0700 Subject: [PATCH 4/4] Fix unit test --- service/history/handler/handler_test.go | 40 +++++++++++-------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/service/history/handler/handler_test.go b/service/history/handler/handler_test.go index 42a2d131c4e..250dd7842d9 100644 --- a/service/history/handler/handler_test.go +++ b/service/history/handler/handler_test.go @@ -1040,6 +1040,15 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { Identity: "identity", }, } + specialInput := &types.HistoryRespondDecisionTaskFailedRequest{ + DomainUUID: testDomainID, + FailedRequest: &types.RespondDecisionTaskFailedRequest{ + TaskToken: []byte("task-token"), + Cause: types.DecisionTaskFailedCauseUnhandledDecision.Ptr(), + Details: []byte("Details"), + Identity: "identity", + }, + } testInput := map[string]struct { input *types.HistoryRespondDecisionTaskFailedRequest expectedError bool @@ -1116,16 +1125,8 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(errors.New("error")).Times(1) }, }, - "special failure": { - input: &types.HistoryRespondDecisionTaskFailedRequest{ - DomainUUID: testDomainID, - FailedRequest: &types.RespondDecisionTaskFailedRequest{ - TaskToken: []byte("task-token"), - Cause: types.DecisionTaskFailedCauseUnhandledDecision.Ptr(), - Details: []byte("Details"), - Identity: "identity", - }, - }, + "special domain": { + input: specialInput, expectedError: false, mockFn: func() { s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) @@ -1133,20 +1134,13 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { WorkflowID: testWorkflowID, RunID: testValidUUID, }, nil).Times(1) + s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return("name", nil).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), specialInput).Return(nil).Times(1) }, }, - "special failure2": { - input: &types.HistoryRespondDecisionTaskFailedRequest{ - DomainUUID: testDomainID, - FailedRequest: &types.RespondDecisionTaskFailedRequest{ - TaskToken: []byte("task-token"), - Cause: types.DecisionTaskFailedCauseUnhandledDecision.Ptr(), - Details: []byte("Details"), - Identity: "identity", - }, - }, + "special domain2": { + input: specialInput, expectedError: false, mockFn: func() { s.mockRatelimiter.EXPECT().Allow().Return(true).Times(1) @@ -1154,9 +1148,9 @@ func (s *handlerSuite) TestRespondDecisionTaskFailed() { WorkflowID: testWorkflowID, RunID: testValidUUID, }, nil).Times(1) - s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return(testDomainID, nil).Times(1) + s.mockResource.DomainCache.EXPECT().GetDomainName(gomock.Any()).Return("", errors.New("error")).Times(1) s.mockShardController.EXPECT().GetEngine(testWorkflowID).Return(s.mockEngine, nil).Times(1) - s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), validInput).Return(nil).Times(1) + s.mockEngine.EXPECT().RespondDecisionTaskFailed(gomock.Any(), specialInput).Return(nil).Times(1) }, }, }