From f6fefb41e41f344fc6657ac670933ae63e76175c Mon Sep 17 00:00:00 2001 From: Aditi Gautam Date: Fri, 23 Feb 2024 11:06:01 -0800 Subject: [PATCH 1/2] Added more scenarios to createworkflowexecution test --- .../nosql/nosql_execution_store_test.go | 130 +++++++++++------- 1 file changed, 78 insertions(+), 52 deletions(-) diff --git a/common/persistence/nosql/nosql_execution_store_test.go b/common/persistence/nosql/nosql_execution_store_test.go index 4ac192463dd..f40c933da0f 100644 --- a/common/persistence/nosql/nosql_execution_store_test.go +++ b/common/persistence/nosql/nosql_execution_store_test.go @@ -22,7 +22,7 @@ package nosql import ( "context" - "errors" + "github.com/uber/cadence/common/types" "testing" "github.com/golang/mock/gomock" @@ -31,7 +31,6 @@ import ( "github.com/uber/cadence/common/log" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/persistence/nosql/nosqlplugin" - "github.com/uber/cadence/common/types" "github.com/uber/cadence/service/history/constants" ) @@ -48,75 +47,102 @@ func TestCreateWorkflowExecution(t *testing.T) { } ctx := context.Background() - newWorkflowSnapshot := persistence.InternalWorkflowSnapshot{ - VersionHistories: &persistence.DataBlob{}, - ExecutionInfo: &persistence.InternalWorkflowExecutionInfo{ - DomainID: constants.TestDomainID, - WorkflowID: constants.TestWorkflowID, - RunID: constants.TestRunID, - }, + request := &persistence.InternalCreateWorkflowExecutionRequest{ + RangeID: 123, + Mode: persistence.CreateWorkflowModeBrandNew, + PreviousRunID: "previous-run-id", + PreviousLastWriteVersion: 456, + NewWorkflowSnapshot: getNewWorkflowSnapshot(), } - testCases := []struct { - name string - request *persistence.InternalCreateWorkflowExecutionRequest - mockReturnError error - expectedResponse *persistence.CreateWorkflowExecutionResponse - expectedError error + name string + setupMock func() + request *persistence.InternalCreateWorkflowExecutionRequest + expectedError error }{ { name: "success", - request: &persistence.InternalCreateWorkflowExecutionRequest{ - RangeID: 123, - Mode: persistence.CreateWorkflowModeBrandNew, - PreviousRunID: "previous-run-id", - PreviousLastWriteVersion: 456, - NewWorkflowSnapshot: newWorkflowSnapshot, + setupMock: func() { + mockDB.EXPECT(). + InsertWorkflowExecutionWithTasks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(nil).Times(1) }, - mockReturnError: nil, - expectedResponse: &persistence.CreateWorkflowExecutionResponse{}, - expectedError: nil, + request: request, + expectedError: nil, }, { name: "failure - workflow already exists", - request: &persistence.InternalCreateWorkflowExecutionRequest{ - RangeID: 123, - Mode: persistence.CreateWorkflowModeBrandNew, - PreviousRunID: "previous-run-id", - PreviousLastWriteVersion: 456, - NewWorkflowSnapshot: newWorkflowSnapshot, + setupMock: func() { + mockDB.EXPECT(). + InsertWorkflowExecutionWithTasks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&persistence.WorkflowExecutionAlreadyStartedError{}).Times(1) + mockDB.EXPECT().IsNotFoundError(gomock.Any()).Return(false).AnyTimes() // Assuming the error is not a "not found" error + mockDB.EXPECT().IsTimeoutError(gomock.Any()).Return(false).AnyTimes() + mockDB.EXPECT().IsThrottlingError(gomock.Any()).Return(false).AnyTimes() + mockDB.EXPECT().IsDBUnavailableError(gomock.Any()).Return(false).AnyTimes() + }, + request: request, + expectedError: &persistence.WorkflowExecutionAlreadyStartedError{}, + }, + { + name: "shard ownership lost", + setupMock: func() { + mockDB.EXPECT(). + InsertWorkflowExecutionWithTasks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&persistence.ShardOwnershipLostError{ShardID: store.shardID, Msg: "shard ownership lost"}). + Times(1) + }, + request: request, + expectedError: &persistence.ShardOwnershipLostError{}, + }, + { + name: "current workflow condition failed", + setupMock: func() { + mockDB.EXPECT(). + InsertWorkflowExecutionWithTasks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&persistence.CurrentWorkflowConditionFailedError{Msg: "current workflow condition failed"}). + Times(1) + }, + request: request, + expectedError: &persistence.CurrentWorkflowConditionFailedError{}, + }, + { + name: "generic internal service error", + setupMock: func() { + mockDB.EXPECT(). + InsertWorkflowExecutionWithTasks(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(&types.InternalServiceError{Message: "generic internal service error"}). + Times(1) }, - mockReturnError: &persistence.WorkflowExecutionAlreadyStartedError{}, - expectedResponse: nil, - expectedError: &persistence.WorkflowExecutionAlreadyStartedError{}, + request: request, + expectedError: &types.InternalServiceError{}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - mockDB.EXPECT(). - InsertWorkflowExecutionWithTasks( - ctx, - gomock.Any(), - gomock.Any(), - gomock.Any(), - gomock.Any(), - gomock.Any(), - gomock.Any(), - gomock.Any(), - ).Return(tc.mockReturnError).Times(1) - if tc.mockReturnError != nil { - mockDB.EXPECT().IsNotFoundError(gomock.Any()).Return(errors.Is(tc.mockReturnError, &persistence.WorkflowExecutionAlreadyStartedError{})).AnyTimes() - mockDB.EXPECT().IsTimeoutError(gomock.Any()).Return(errors.Is(tc.mockReturnError, &persistence.WorkflowExecutionAlreadyStartedError{})).AnyTimes() - mockDB.EXPECT().IsThrottlingError(gomock.Any()).Return(errors.Is(tc.mockReturnError, &persistence.WorkflowExecutionAlreadyStartedError{})).AnyTimes() - mockDB.EXPECT().IsDBUnavailableError(gomock.Any()).Return(errors.Is(tc.mockReturnError, &persistence.WorkflowExecutionAlreadyStartedError{})).AnyTimes() + if tc.setupMock != nil { + tc.setupMock() } _, err := store.CreateWorkflowExecution(ctx, tc.request) - var internalServiceErr *types.InternalServiceError - if errors.As(err, &internalServiceErr) { - require.Equal(t, "CreateWorkflowExecution operation failed. Error: ", internalServiceErr.Message) + + if tc.expectedError != nil { + require.ErrorAs(t, err, &tc.expectedError) + } else { + require.NoError(t, err) } }) } } + +func getNewWorkflowSnapshot() persistence.InternalWorkflowSnapshot { + return persistence.InternalWorkflowSnapshot{ + VersionHistories: &persistence.DataBlob{}, + ExecutionInfo: &persistence.InternalWorkflowExecutionInfo{ + DomainID: constants.TestDomainID, + WorkflowID: constants.TestWorkflowID, + RunID: constants.TestRunID, + }, + } +} From 3d3f9adaf1e5eabde838cac80d721aa5814b94bc Mon Sep 17 00:00:00 2001 From: Aditi Gautam Date: Fri, 23 Feb 2024 11:11:19 -0800 Subject: [PATCH 2/2] Minore lint fix --- common/persistence/nosql/nosql_execution_store_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/persistence/nosql/nosql_execution_store_test.go b/common/persistence/nosql/nosql_execution_store_test.go index f40c933da0f..b35cd0265d5 100644 --- a/common/persistence/nosql/nosql_execution_store_test.go +++ b/common/persistence/nosql/nosql_execution_store_test.go @@ -22,7 +22,6 @@ package nosql import ( "context" - "github.com/uber/cadence/common/types" "testing" "github.com/golang/mock/gomock" @@ -31,6 +30,7 @@ import ( "github.com/uber/cadence/common/log" "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/persistence/nosql/nosqlplugin" + "github.com/uber/cadence/common/types" "github.com/uber/cadence/service/history/constants" )