From 1bc3abd7e648d2d41034c2fe97d5be38d141b640 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Mon, 28 Oct 2024 15:01:04 -0700 Subject: [PATCH 01/12] Admin_db_clean_cmd test --- tools/cli/admin_db_clean_command_test.go | 123 +++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 tools/cli/admin_db_clean_command_test.go diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go new file mode 100644 index 00000000000..0098ca3677b --- /dev/null +++ b/tools/cli/admin_db_clean_command_test.go @@ -0,0 +1,123 @@ +package cli + +import ( + "flag" + "github.com/stretchr/testify/assert" + "github.com/urfave/cli/v2" + "os" + "testing" +) + +func TestAdminDBClean_errorCases(t *testing.T) { + tests := []struct { + name string + setupContext func(app *cli.App) *cli.Context + inputFileData string // Simulate the content of the input file + expectedOutput string + expectedError string + }{ + { + name: "MissingRequiredFlagScanType", + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagInputFile, "", "Input file flag") + // Missing FlagScanType + _ = set.Set(FlagInputFile, "input.json") + return cli.NewContext(app, set, nil) + }, + inputFileData: ``, + expectedOutput: "", + expectedError: "Required flag not found:", + }, + { + name: "UnknownScanType", + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + _ = set.Set(FlagScanType, "unknown") + _ = set.Set(FlagInputFile, "input.json") + return cli.NewContext(app, set, nil) + }, + inputFileData: ``, + expectedOutput: "", + expectedError: "unknown scan type", + }, + { + name: "InvalidInvariantCollection", + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + // Define FlagScanType and FlagInputFile + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + + // Simulate the collection slice with multiple collections (including an invalid one) + set.Var(cli.NewStringSlice("invalid_collection", "history"), FlagInvariantCollection, "invariant collection flag") + + // Set actual values for the flags + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, "input.json") + return cli.NewContext(app, set, nil) + }, + inputFileData: ``, + expectedOutput: "", + expectedError: "unknown invariant collection", + }, + { + name: "NoInvariantsError", + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInvariantCollection, "", "invariant collection flag") + set.String(FlagInputFile, "", "Input file flag") + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInvariantCollection, "invalid_collection") // Collection will trigger no invariants + _ = set.Set(FlagInputFile, "input.json") + return cli.NewContext(app, set, nil) + }, + inputFileData: `[{"Execution": {"ShardID": 1}}]`, // Simulate the content of input file + expectedOutput: "", + expectedError: "no invariants for scantype", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp input file with the test's input data + inputFile, err := os.CreateTemp("", "test_input_*.json") + assert.NoError(t, err) + defer os.Remove(inputFile.Name()) // Clean up after test + + // Write input data to the temp file + if tt.inputFileData != "" { + _, err = inputFile.WriteString(tt.inputFileData) + assert.NoError(t, err) + } + inputFile.Close() + + // Create test IO handler to capture output + ioHandler := &testIOHandler{} + + // Set up the CLI app + app := NewCliApp(nil, WithIOHandler(ioHandler)) + + // Set up the CLI context + c := tt.setupContext(app) + + // Overwrite the FlagInputFile with the actual temp file path + _ = c.Set(FlagInputFile, inputFile.Name()) + + // Call AdminDBClean and validate output and errors + err = AdminDBClean(c) + if tt.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.expectedError) + } else { + assert.NoError(t, err) + } + + // Validate the captured output + assert.Contains(t, ioHandler.outputBytes.String(), tt.expectedOutput) + }) + } +} From a93ad1925a2e73281a6b51c0293d58ddc7762402 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Mon, 28 Oct 2024 15:07:02 -0700 Subject: [PATCH 02/12] fmt --- tools/cli/admin_db_clean_command_test.go | 27 ++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index 0098ca3677b..df8b891402d 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -1,11 +1,34 @@ +// The MIT License (MIT) + +// Copyright (c) 2017-2020 Uber Technologies Inc. + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + package cli import ( "flag" - "github.com/stretchr/testify/assert" - "github.com/urfave/cli/v2" "os" "testing" + + "github.com/stretchr/testify/assert" + "github.com/urfave/cli/v2" ) func TestAdminDBClean_errorCases(t *testing.T) { From e274fa409257038009805b01329d9bacddee8b7f Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Mon, 28 Oct 2024 15:13:03 -0700 Subject: [PATCH 03/12] delete redundant test file after run --- tools/cli/admin_elastic_search_commands_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/cli/admin_elastic_search_commands_test.go b/tools/cli/admin_elastic_search_commands_test.go index 059af005708..7da14513f09 100644 --- a/tools/cli/admin_elastic_search_commands_test.go +++ b/tools/cli/admin_elastic_search_commands_test.go @@ -1039,6 +1039,8 @@ func TestGenerateReport(t *testing.T) { // Validate the output captured by testIOHandler assert.Equal(t, tt.expectedOutput, ioHandler.outputBytes.String()) } + + os.Remove("test-report.csv") }) } } From 957fccfc75eabb91d141d9892278c1565fd1d2d0 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Mon, 28 Oct 2024 16:23:57 -0700 Subject: [PATCH 04/12] Add more tests for admin_db_clean_cmds --- tools/cli/admin_db_clean_command.go | 16 ++- tools/cli/admin_db_clean_command_test.go | 151 +++++++++++++++++++++++ 2 files changed, 162 insertions(+), 5 deletions(-) diff --git a/tools/cli/admin_db_clean_command.go b/tools/cli/admin_db_clean_command.go index 2992ec8a187..cfabc4dffb9 100644 --- a/tools/cli/admin_db_clean_command.go +++ b/tools/cli/admin_db_clean_command.go @@ -106,7 +106,7 @@ func AdminDBClean(c *cli.Context) error { } for _, e := range data { - result, err := fixExecution(c, invariants, e) + result, err := fixExecution(c, invariants, e, initializeExecutionStore, initializeHistoryManager) if err != nil { return commoncli.Problem("Error in fix execution: ", err) } @@ -130,14 +130,20 @@ func fixExecution( c *cli.Context, invariants []executions.InvariantFactory, execution *store.ScanOutputEntity, + initializeExecutionStoreFn func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), + initializeHistoryManagerFn func(c *cli.Context) (persistence.HistoryManager, error), ) (invariant.ManagerFixResult, error) { - execManager, err := initializeExecutionStore(c, execution.Execution.(entity.Entity).GetShardID()) - defer execManager.Close() + execManager, err := initializeExecutionStoreFn(c, execution.Execution.(entity.Entity).GetShardID()) + if execManager != nil { + defer execManager.Close() + } if err != nil { return invariant.ManagerFixResult{}, fmt.Errorf("Error in fix execution: %w", err) } - historyV2Mgr, err := initializeHistoryManager(c) - defer historyV2Mgr.Close() + historyV2Mgr, err := initializeHistoryManagerFn(c) + if historyV2Mgr != nil { + defer historyV2Mgr.Close() + } if err != nil { return invariant.ManagerFixResult{}, fmt.Errorf("Error in fix execution: %w", err) } diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index df8b891402d..63bb31452bc 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -24,6 +24,14 @@ package cli import ( "flag" + "fmt" + "github.com/golang/mock/gomock" + "github.com/uber/cadence/common/cache" + "github.com/uber/cadence/common/persistence" + "github.com/uber/cadence/common/reconciliation/entity" + "github.com/uber/cadence/common/reconciliation/invariant" + "github.com/uber/cadence/common/reconciliation/store" + "github.com/uber/cadence/service/worker/scanner/executions" "os" "testing" @@ -144,3 +152,146 @@ func TestAdminDBClean_errorCases(t *testing.T) { }) } } + +func TestFixExecution(t *testing.T) { + tests := []struct { + name string + setupMock func(ctrl *gomock.Controller) (initializeExecutionStoreFn func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), initializeHistoryManagerFn func(c *cli.Context) (persistence.HistoryManager, error), invariants []executions.InvariantFactory) + setupContext func(app *cli.App) *cli.Context + execution *store.ScanOutputEntity + expectedError string + expectedResult invariant.ManagerFixResult + }{ + { + name: "Success", + setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { + // Mock ExecutionManager + mockExecManager := persistence.NewMockExecutionManager(ctrl) + mockExecManager.EXPECT().Close().Times(1) + + // Mock HistoryManager + mockHistoryManager := persistence.NewMockHistoryManager(ctrl) + mockHistoryManager.EXPECT().Close().Times(1) + + // Mock Invariant and wrap into InvariantFactory + mockInvariant := invariant.NewMockInvariant(ctrl) + mockInvariant.EXPECT().Fix(gomock.Any(), gomock.Any()).Return(invariant.FixResult{ + FixResultType: invariant.FixResultTypeFixed, + }).Times(1) + + // Wrap mockInvariant into InvariantFactory + invariants := []executions.InvariantFactory{ + func(pr persistence.Retryer, dc cache.DomainCache) invariant.Invariant { + return mockInvariant + }, + } + + return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { + return mockExecManager, nil + }, func(c *cli.Context) (persistence.HistoryManager, error) { + return mockHistoryManager, nil + }, invariants + }, + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, "input.json") + return cli.NewContext(app, set, nil) + }, + execution: &store.ScanOutputEntity{ + Execution: &entity.ConcreteExecution{ + Execution: entity.Execution{ // Initialize the embedded Execution struct + ShardID: 1, + }, + }, + }, + expectedError: "", + expectedResult: invariant.ManagerFixResult{ + FixResultType: invariant.FixResultTypeFixed, + FixResults: []invariant.FixResult{ + {FixResultType: invariant.FixResultTypeFixed, InvariantName: ""}, + }, + // Set DeterminingInvariantName as a pointer to an empty string + DeterminingInvariantName: func() *invariant.Name { + name := invariant.Name("") + return &name + }(), + }, + }, + { + name: "ExecutionManagerError", + setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { + + return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { + return nil, fmt.Errorf("execution manager error") + }, nil, nil + }, + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + _ = set.Set(FlagScanType, "ConcreteExecutionType") + return cli.NewContext(app, set, nil) + }, + execution: &store.ScanOutputEntity{ + Execution: &entity.ConcreteExecution{ + Execution: entity.Execution{ // Initialize the embedded Execution struct + ShardID: 1, + }, + }, + }, + expectedError: "Error in fix execution", + }, + { + name: "HistoryManagerError", + setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { + // Mock ExecutionManager + mockExecManager := persistence.NewMockExecutionManager(ctrl) + mockExecManager.EXPECT().Close().Times(1) + + return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { + return mockExecManager, nil + }, func(c *cli.Context) (persistence.HistoryManager, error) { + return nil, fmt.Errorf("history manager error") + }, nil + }, + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + _ = set.Set(FlagScanType, "ConcreteExecutionType") + return cli.NewContext(app, set, nil) + }, + execution: &store.ScanOutputEntity{ + Execution: &entity.ConcreteExecution{ + Execution: entity.Execution{ // Initialize the embedded Execution struct + ShardID: 1, + }, + }, + }, + expectedError: "Error in fix execution", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + + // Setup mock functions and dependencies + initializeExecutionStoreFn, initializeHistoryManagerFn, invariants := tt.setupMock(ctrl) + + // Setup the CLI context + td := newCLITestData(t) + cliCtx := tt.setupContext(td.app) + + // Call fixExecution + result, err := fixExecution(cliCtx, invariants, tt.execution, initializeExecutionStoreFn, initializeHistoryManagerFn) + + // Validate results + if tt.expectedError != "" { + assert.ErrorContains(t, err, tt.expectedError) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expectedResult, result) + } + }) + } +} From 494354375c1d29e79efa19e567e78928e2f45d56 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Tue, 29 Oct 2024 17:25:49 -0700 Subject: [PATCH 05/12] add test for success case --- tools/cli/admin_db_clean_command_test.go | 168 ++++++++--------------- 1 file changed, 55 insertions(+), 113 deletions(-) diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index 2ea87714430..d8b6baebe4e 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -24,14 +24,10 @@ package cli import ( "flag" - "fmt" "github.com/golang/mock/gomock" - "github.com/uber/cadence/common/cache" "github.com/uber/cadence/common/persistence" - "github.com/uber/cadence/common/reconciliation/entity" "github.com/uber/cadence/common/reconciliation/invariant" - "github.com/uber/cadence/common/reconciliation/store" - "github.com/uber/cadence/service/worker/scanner/executions" + "github.com/uber/cadence/common/types" "os" "testing" @@ -153,145 +149,91 @@ func TestAdminDBClean_errorCases(t *testing.T) { } } -func TestFixExecution(t *testing.T) { +func TestAdminDBClean(t *testing.T) { tests := []struct { name string - setupMock func(ctrl *gomock.Controller) (initializeExecutionStoreFn func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), initializeHistoryManagerFn func(c *cli.Context) (persistence.HistoryManager, error), invariants []executions.InvariantFactory) - setupContext func(app *cli.App) *cli.Context - execution *store.ScanOutputEntity + contextSetup func(td *cliTestData) *cli.Context + mockSetup func(td *cliTestData) + inputFileData string expectedError string - expectedResult invariant.ManagerFixResult + expectedOutput string }{ { name: "Success", - setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { - // Mock ExecutionManager - mockExecManager := persistence.NewMockExecutionManager(ctrl) - mockExecManager.EXPECT().Close().Times(1) - - // Mock HistoryManager - mockHistoryManager := persistence.NewMockHistoryManager(ctrl) - mockHistoryManager.EXPECT().Close().Times(1) - - // Mock Invariant and wrap into InvariantFactory - mockInvariant := invariant.NewMockInvariant(ctrl) - mockInvariant.EXPECT().Fix(gomock.Any(), gomock.Any()).Return(invariant.FixResult{ - FixResultType: invariant.FixResultTypeFixed, - }).Times(1) - - // Wrap mockInvariant into InvariantFactory - invariants := []executions.InvariantFactory{ - func(pr persistence.Retryer, dc cache.DomainCache) invariant.Invariant { - return mockInvariant - }, - } - - return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { - return mockExecManager, nil - }, func(c *cli.Context) (persistence.HistoryManager, error) { - return mockHistoryManager, nil - }, invariants - }, - setupContext: func(app *cli.App) *cli.Context { + contextSetup: func(td *cliTestData) *cli.Context { set := flag.NewFlagSet("test", 0) + // Define flags set.String(FlagScanType, "", "scan type flag") + set.String(FlagInvariantCollection, "", "invariant collection flag") set.String(FlagInputFile, "", "Input file flag") + + // Set values for flags _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInvariantCollection, "CollectionHistory") // Valid collection to ensure invariants are generated _ = set.Set(FlagInputFile, "input.json") - return cli.NewContext(app, set, nil) - }, - execution: &store.ScanOutputEntity{ - Execution: &entity.ConcreteExecution{ - Execution: entity.Execution{ // Initialize the embedded Execution struct - ShardID: 1, - }, - }, - }, - expectedError: "", - expectedResult: invariant.ManagerFixResult{ - FixResultType: invariant.FixResultTypeFixed, - FixResults: []invariant.FixResult{ - {FixResultType: invariant.FixResultTypeFixed, InvariantName: ""}, - }, - // Set DeterminingInvariantName as a pointer to an empty string - DeterminingInvariantName: func() *invariant.Name { - name := invariant.Name("") - return &name - }(), - }, - }, - { - name: "ExecutionManagerError", - setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { - return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { - return nil, fmt.Errorf("execution manager error") - }, nil, nil - }, - setupContext: func(app *cli.App) *cli.Context { - set := flag.NewFlagSet("test", 0) - _ = set.Set(FlagScanType, "ConcreteExecutionType") - return cli.NewContext(app, set, nil) - }, - execution: &store.ScanOutputEntity{ - Execution: &entity.ConcreteExecution{ - Execution: entity.Execution{ // Initialize the embedded Execution struct - ShardID: 1, - }, - }, + return cli.NewContext(td.app, set, nil) }, - expectedError: "Error in fix execution", - }, - { - name: "HistoryManagerError", - setupMock: func(ctrl *gomock.Controller) (func(c *cli.Context, shardID int) (persistence.ExecutionManager, error), func(c *cli.Context) (persistence.HistoryManager, error), []executions.InvariantFactory) { - // Mock ExecutionManager + mockSetup: func(td *cliTestData) { + // Mock the PersistenceManagerFactory + ctrl := gomock.NewController(t) mockExecManager := persistence.NewMockExecutionManager(ctrl) + mockHistoryManager := persistence.NewMockHistoryManager(ctrl) + + // Mock ExecutionManager and HistoryManager close methods mockExecManager.EXPECT().Close().Times(1) + mockHistoryManager.EXPECT().Close().Times(1) - return func(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { - return mockExecManager, nil - }, func(c *cli.Context) (persistence.HistoryManager, error) { - return nil, fmt.Errorf("history manager error") - }, nil - }, - setupContext: func(app *cli.App) *cli.Context { - set := flag.NewFlagSet("test", 0) - _ = set.Set(FlagScanType, "ConcreteExecutionType") - return cli.NewContext(app, set, nil) - }, - execution: &store.ScanOutputEntity{ - Execution: &entity.ConcreteExecution{ - Execution: entity.Execution{ // Initialize the embedded Execution struct - ShardID: 1, - }, - }, + // Mock initializeExecutionStore and initializeHistoryManager correctly + td.mockPersistenceManagerFactory.EXPECT().initializeExecutionStore(gomock.Any(), gomock.Any()).Return(mockExecManager, nil).AnyTimes() + td.mockPersistenceManagerFactory.EXPECT().initializeHistoryManager(gomock.Any()).Return(mockHistoryManager, nil).AnyTimes() + + // Mock fixExecution by setting up a valid InvariantFactory + mockInvariant := invariant.NewMockInvariant(ctrl) + mockInvariant.EXPECT().Fix(gomock.Any(), gomock.Any()).Return(invariant.FixResult{ + FixResultType: invariant.FixResultTypeFixed, + }).Times(1) + + // Ensure that ToInvariants returns a non-empty list of factories + td.mockFrontendClient.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any()).Return(&types.StartWorkflowExecutionResponse{RunID: "test-run-id"}, nil).AnyTimes() }, - expectedError: "Error in fix execution", + inputFileData: `{"Execution": {"ShardID": 1}, "Result": {}}`, + expectedOutput: `{"Execution":{"ShardID":1},"Input":{"Execution":{"ShardID":1}},"Result":{}}`, + expectedError: "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) + // Create test data and set up mocks + td := newCLITestData(t) + tt.mockSetup(td) - // Setup mock functions and dependencies - initializeExecutionStoreFn, initializeHistoryManagerFn, invariants := tt.setupMock(ctrl) + // Create temp input file + inputFile, err := os.CreateTemp("", "test_input_*.json") + assert.NoError(t, err) + defer os.Remove(inputFile.Name()) // Clean up after test - // Setup the CLI context - td := newCLITestData(t) - cliCtx := tt.setupContext(td.app) + // Write input data to the temp file + if tt.inputFileData != "" { + _, err = inputFile.WriteString(tt.inputFileData) + assert.NoError(t, err) + } + inputFile.Close() - // Call fixExecution - result, err := fixExecution(cliCtx, invariants, tt.execution, initializeExecutionStoreFn, initializeHistoryManagerFn) + // Prepare the CLI context + c := tt.contextSetup(td) - // Validate results + // Run AdminDBClean + err = AdminDBClean(c) if tt.expectedError != "" { assert.ErrorContains(t, err, tt.expectedError) } else { assert.NoError(t, err) - assert.Equal(t, tt.expectedResult, result) } + + // Validate output + assert.Equal(t, tt.expectedOutput, td.consoleOutput()) }) } } From 32917ca3789a8f85a17e0fa9c06c86cf3ccc5f07 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Tue, 29 Oct 2024 17:26:37 -0700 Subject: [PATCH 06/12] resolve conflict --- tools/cli/admin_commands_test.go | 13 ++++++++----- tools/cli/app.go | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/tools/cli/admin_commands_test.go b/tools/cli/admin_commands_test.go index 8f6e893c788..cb17afedf8b 100644 --- a/tools/cli/admin_commands_test.go +++ b/tools/cli/admin_commands_test.go @@ -47,10 +47,11 @@ const ( ) type cliTestData struct { - mockFrontendClient *frontend.MockClient - mockAdminClient *admin.MockClient - ioHandler *testIOHandler - app *cli.App + mockFrontendClient *frontend.MockClient + mockAdminClient *admin.MockClient + ioHandler *testIOHandler + app *cli.App + mockPersistenceManagerFactory *MockPersistenceManagerFactory } func newCLITestData(t *testing.T) *cliTestData { @@ -60,15 +61,17 @@ func newCLITestData(t *testing.T) *cliTestData { td.mockFrontendClient = frontend.NewMockClient(ctrl) td.mockAdminClient = admin.NewMockClient(ctrl) + td.mockPersistenceManagerFactory = NewMockPersistenceManagerFactory(ctrl) td.ioHandler = &testIOHandler{} - // make a new CLI App with client factory returning our frontend and admin clients + // Create a new CLI app with client factory and persistence manager factory td.app = NewCliApp( &clientFactoryMock{ serverFrontendClient: td.mockFrontendClient, serverAdminClient: td.mockAdminClient, }, WithIOHandler(td.ioHandler), + WithPersistenceManagerFactory(td.mockPersistenceManagerFactory), // Inject the mocked persistence manager factory ) return &td } diff --git a/tools/cli/app.go b/tools/cli/app.go index 7ba4cde1f23..e73dd210720 100644 --- a/tools/cli/app.go +++ b/tools/cli/app.go @@ -50,6 +50,22 @@ func WithIOHandler(h IOHandler) CLIAppOptions { } } +// WithPersistenceManagerFactory sets the PersistenceManagerFactory for the CLI app. +func WithPersistenceManagerFactory(factory PersistenceManagerFactory) CLIAppOptions { + return func(app *cli.App) { + if app.Metadata == nil { + return + } + + d, ok := app.Metadata[depsKey].(*deps) + if !ok { + return + } + + d.PersistenceManagerFactory = factory + } +} + // NewCliApp instantiates a new instance of the CLI application func NewCliApp(cf ClientFactory, opts ...CLIAppOptions) *cli.App { version := fmt.Sprintf("CLI feature version: %v \n"+ From 4bdd1b62226961790fc23945918cd09497d8823e Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Tue, 29 Oct 2024 19:28:04 -0700 Subject: [PATCH 07/12] add more tests inside of fixExecution --- tools/cli/admin_commands.go | 2 +- tools/cli/admin_commands_test.go | 14 +-- tools/cli/admin_db_clean_command.go | 18 ++- tools/cli/admin_db_clean_command_test.go | 151 +++++++++++++++++------ tools/cli/admin_db_scan_command.go | 4 +- tools/cli/admin_timers.go | 2 +- tools/cli/app.go | 12 +- tools/cli/database.go | 24 ++-- tools/cli/database_mock.go | 74 ++++++----- 9 files changed, 205 insertions(+), 96 deletions(-) diff --git a/tools/cli/admin_commands.go b/tools/cli/admin_commands.go index f551684369f..7bf24973a89 100644 --- a/tools/cli/admin_commands.go +++ b/tools/cli/admin_commands.go @@ -308,7 +308,7 @@ func AdminDeleteWorkflow(c *cli.Context) error { if err != nil { return commoncli.Problem("Error in Admin delete WF: ", err) } - exeStore, err := getDeps(c).initializeExecutionStore(c, shardIDInt) + exeStore, err := getDeps(c).initializeExecutionManager(c, shardIDInt) if err != nil { return commoncli.Problem("Error in Admin delete WF: ", err) } diff --git a/tools/cli/admin_commands_test.go b/tools/cli/admin_commands_test.go index cb17afedf8b..6135b278f28 100644 --- a/tools/cli/admin_commands_test.go +++ b/tools/cli/admin_commands_test.go @@ -47,11 +47,11 @@ const ( ) type cliTestData struct { - mockFrontendClient *frontend.MockClient - mockAdminClient *admin.MockClient - ioHandler *testIOHandler - app *cli.App - mockPersistenceManagerFactory *MockPersistenceManagerFactory + mockFrontendClient *frontend.MockClient + mockAdminClient *admin.MockClient + ioHandler *testIOHandler + app *cli.App + mockManagerFactory *MockManagerFactory } func newCLITestData(t *testing.T) *cliTestData { @@ -61,7 +61,7 @@ func newCLITestData(t *testing.T) *cliTestData { td.mockFrontendClient = frontend.NewMockClient(ctrl) td.mockAdminClient = admin.NewMockClient(ctrl) - td.mockPersistenceManagerFactory = NewMockPersistenceManagerFactory(ctrl) + td.mockManagerFactory = NewMockManagerFactory(ctrl) td.ioHandler = &testIOHandler{} // Create a new CLI app with client factory and persistence manager factory @@ -71,7 +71,7 @@ func newCLITestData(t *testing.T) *cliTestData { serverAdminClient: td.mockAdminClient, }, WithIOHandler(td.ioHandler), - WithPersistenceManagerFactory(td.mockPersistenceManagerFactory), // Inject the mocked persistence manager factory + WithPersistenceManagerFactory(td.mockManagerFactory), // Inject the mocked persistence manager factory ) return &td } diff --git a/tools/cli/admin_db_clean_command.go b/tools/cli/admin_db_clean_command.go index 675af26bac7..a026fa153da 100644 --- a/tools/cli/admin_db_clean_command.go +++ b/tools/cli/admin_db_clean_command.go @@ -121,7 +121,8 @@ func AdminDBClean(c *cli.Context) error { continue } - fmt.Println(string(data)) + output := getDeps(c).Output() + output.Write([]byte(string(data) + "\n")) } return nil } @@ -131,19 +132,19 @@ func fixExecution( invariants []executions.InvariantFactory, execution *store.ScanOutputEntity, ) (invariant.ManagerFixResult, error) { - execManager, err := getDeps(c).initializeExecutionStore(c, execution.Execution.(entity.Entity).GetShardID()) + execManager, err := getDeps(c).initializeExecutionManager(c, execution.Execution.(entity.Entity).GetShardID()) if execManager != nil { defer execManager.Close() } if err != nil { - return invariant.ManagerFixResult{}, fmt.Errorf("Error in fix execution: %w", err) + return invariant.ManagerFixResult{}, err } historyV2Mgr, err := getDeps(c).initializeHistoryManager(c) if historyV2Mgr != nil { defer historyV2Mgr.Close() } if err != nil { - return invariant.ManagerFixResult{}, fmt.Errorf("Error in fix execution: %w", err) + return invariant.ManagerFixResult{}, err } pr := persistence.NewPersistenceRetryer( execManager, @@ -160,7 +161,12 @@ func fixExecution( ctx, cancel, err := newContext(c) defer cancel() if err != nil { - return invariant.ManagerFixResult{}, fmt.Errorf("Context not created: %w", err) + return invariant.ManagerFixResult{}, err } - return invariant.NewInvariantManager(ivs).RunFixes(ctx, execution.Execution), nil + invariantManager, err := getDeps(c).initializeInvariantManager(ivs) + if err != nil { + return invariant.ManagerFixResult{}, err + } + + return invariantManager.RunFixes(ctx, execution.Execution.(entity.Entity)), nil } diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index d8b6baebe4e..9d44e57e136 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -24,18 +24,19 @@ package cli import ( "flag" - "github.com/golang/mock/gomock" - "github.com/uber/cadence/common/persistence" - "github.com/uber/cadence/common/reconciliation/invariant" - "github.com/uber/cadence/common/types" + "fmt" "os" "testing" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/urfave/cli/v2" + + "github.com/uber/cadence/common/persistence" + "github.com/uber/cadence/common/reconciliation/invariant" ) -func TestAdminDBClean_errorCases(t *testing.T) { +func TestAdminDBClean_noFixExecution(t *testing.T) { tests := []struct { name string setupContext func(app *cli.App) *cli.Context @@ -43,6 +44,26 @@ func TestAdminDBClean_errorCases(t *testing.T) { expectedOutput string expectedError string }{ + { + name: "SuccessCase_emptyResult", + setupContext: func(app *cli.App) *cli.Context { + set := flag.NewFlagSet("test", 0) + // Define flags + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + // Use a StringSliceFlag to simulate multiple collection values + set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") + + // Set actual values for the flags + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, "input.json") + + return cli.NewContext(app, set, nil) + }, + inputFileData: `[{"Execution": {"ShardID": 1}}]`, // Simulate the content of input file + expectedOutput: ``, + expectedError: "", + }, { name: "MissingRequiredFlagScanType", setupContext: func(app *cli.App) *cli.Context { @@ -149,10 +170,10 @@ func TestAdminDBClean_errorCases(t *testing.T) { } } -func TestAdminDBClean(t *testing.T) { +func TestAdminDBClean_inFixExecution(t *testing.T) { tests := []struct { name string - contextSetup func(td *cliTestData) *cli.Context + contextSetup func(td *cliTestData, inputFilePath string) *cli.Context mockSetup func(td *cliTestData) inputFileData string expectedError string @@ -160,71 +181,132 @@ func TestAdminDBClean(t *testing.T) { }{ { name: "Success", - contextSetup: func(td *cliTestData) *cli.Context { + contextSetup: func(td *cliTestData, inputFilePath string) *cli.Context { set := flag.NewFlagSet("test", 0) - // Define flags set.String(FlagScanType, "", "scan type flag") - set.String(FlagInvariantCollection, "", "invariant collection flag") set.String(FlagInputFile, "", "Input file flag") + set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - // Set values for flags _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInvariantCollection, "CollectionHistory") // Valid collection to ensure invariants are generated - _ = set.Set(FlagInputFile, "input.json") + _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + + return cli.NewContext(td.app, set, nil) + }, + mockSetup: func(td *cliTestData) { + ctrl := gomock.NewController(t) + mockExecManager := persistence.NewMockExecutionManager(ctrl) + mockHistoryManager := persistence.NewMockHistoryManager(ctrl) + mockInvariantManager := invariant.NewMockManager(ctrl) + + mockExecManager.EXPECT().Close().Times(1) + mockHistoryManager.EXPECT().Close().Times(1) + + mockInvariantManager.EXPECT().RunFixes(gomock.Any(), gomock.Any()).Return(invariant.ManagerFixResult{}).Times(1) + + td.mockManagerFactory.EXPECT().initializeExecutionManager(gomock.Any(), gomock.Any()).Return(mockExecManager, nil).Times(1) + td.mockManagerFactory.EXPECT().initializeHistoryManager(gomock.Any()).Return(mockHistoryManager, nil).Times(1) + td.mockManagerFactory.EXPECT().initializeInvariantManager(gomock.Any()).Return(mockInvariantManager, nil).Times(1) + + }, + inputFileData: `{"Execution": {"ShardID": 1}, "Result": {}}`, + expectedOutput: `{"Execution":{"BranchToken":null,"TreeID":"","BranchID":"","ShardID":1,"DomainID":"","WorkflowID":"","RunID":"","State":0},"Input":{"Execution":{"BranchToken":null,"TreeID":"","BranchID":"","ShardID":1,"DomainID":"","WorkflowID":"","RunID":"","State":0},"Result":{"CheckResultType":"","DeterminingInvariantType":null,"CheckResults":null}},"Result":{"FixResultType":"","DeterminingInvariantName":null,"FixResults":null}} +`, + expectedError: "", + }, + { + name: "init invariant manager error", + contextSetup: func(td *cliTestData, inputFilePath string) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") + + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 return cli.NewContext(td.app, set, nil) }, mockSetup: func(td *cliTestData) { - // Mock the PersistenceManagerFactory ctrl := gomock.NewController(t) mockExecManager := persistence.NewMockExecutionManager(ctrl) mockHistoryManager := persistence.NewMockHistoryManager(ctrl) - // Mock ExecutionManager and HistoryManager close methods mockExecManager.EXPECT().Close().Times(1) mockHistoryManager.EXPECT().Close().Times(1) - // Mock initializeExecutionStore and initializeHistoryManager correctly - td.mockPersistenceManagerFactory.EXPECT().initializeExecutionStore(gomock.Any(), gomock.Any()).Return(mockExecManager, nil).AnyTimes() - td.mockPersistenceManagerFactory.EXPECT().initializeHistoryManager(gomock.Any()).Return(mockHistoryManager, nil).AnyTimes() + td.mockManagerFactory.EXPECT().initializeExecutionManager(gomock.Any(), gomock.Any()).Return(mockExecManager, nil).Times(1) + td.mockManagerFactory.EXPECT().initializeHistoryManager(gomock.Any()).Return(mockHistoryManager, nil).Times(1) + td.mockManagerFactory.EXPECT().initializeInvariantManager(gomock.Any()).Return(nil, fmt.Errorf("init invariant manager error")).Times(1) + + }, + inputFileData: `{"Execution": {"ShardID": 1}, "Result": {}}`, + expectedOutput: ``, + expectedError: "Error in fix execution: : init invariant manager error", + }, + { + name: "init execution manager error", + contextSetup: func(td *cliTestData, inputFilePath string) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - // Mock fixExecution by setting up a valid InvariantFactory - mockInvariant := invariant.NewMockInvariant(ctrl) - mockInvariant.EXPECT().Fix(gomock.Any(), gomock.Any()).Return(invariant.FixResult{ - FixResultType: invariant.FixResultTypeFixed, - }).Times(1) + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 - // Ensure that ToInvariants returns a non-empty list of factories - td.mockFrontendClient.EXPECT().StartWorkflowExecution(gomock.Any(), gomock.Any()).Return(&types.StartWorkflowExecutionResponse{RunID: "test-run-id"}, nil).AnyTimes() + return cli.NewContext(td.app, set, nil) + }, + mockSetup: func(td *cliTestData) { + td.mockManagerFactory.EXPECT().initializeExecutionManager(gomock.Any(), gomock.Any()).Return(nil, fmt.Errorf("init execution manager error")).Times(1) }, inputFileData: `{"Execution": {"ShardID": 1}, "Result": {}}`, - expectedOutput: `{"Execution":{"ShardID":1},"Input":{"Execution":{"ShardID":1}},"Result":{}}`, - expectedError: "", + expectedOutput: ``, + expectedError: "Error in fix execution: : init execution manager error", + }, + { + name: "init history manager error", + contextSetup: func(td *cliTestData, inputFilePath string) *cli.Context { + set := flag.NewFlagSet("test", 0) + set.String(FlagScanType, "", "scan type flag") + set.String(FlagInputFile, "", "Input file flag") + set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") + + _ = set.Set(FlagScanType, "ConcreteExecutionType") + _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + + return cli.NewContext(td.app, set, nil) + }, + mockSetup: func(td *cliTestData) { + ctrl := gomock.NewController(t) + mockExecManager := persistence.NewMockExecutionManager(ctrl) + mockExecManager.EXPECT().Close().Times(1) + + td.mockManagerFactory.EXPECT().initializeExecutionManager(gomock.Any(), gomock.Any()).Return(mockExecManager, nil).Times(1) + td.mockManagerFactory.EXPECT().initializeHistoryManager(gomock.Any()).Return(nil, fmt.Errorf("init history manager error")).Times(1) + }, + inputFileData: `{"Execution": {"ShardID": 1}, "Result": {}}`, + expectedOutput: ``, + expectedError: "Error in fix execution: : init history manager error", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create test data and set up mocks td := newCLITestData(t) tt.mockSetup(td) - // Create temp input file inputFile, err := os.CreateTemp("", "test_input_*.json") assert.NoError(t, err) - defer os.Remove(inputFile.Name()) // Clean up after test + defer os.Remove(inputFile.Name()) - // Write input data to the temp file if tt.inputFileData != "" { _, err = inputFile.WriteString(tt.inputFileData) assert.NoError(t, err) } inputFile.Close() - // Prepare the CLI context - c := tt.contextSetup(td) + c := tt.contextSetup(td, inputFile.Name()) - // Run AdminDBClean err = AdminDBClean(c) if tt.expectedError != "" { assert.ErrorContains(t, err, tt.expectedError) @@ -232,7 +314,6 @@ func TestAdminDBClean(t *testing.T) { assert.NoError(t, err) } - // Validate output assert.Equal(t, tt.expectedOutput, td.consoleOutput()) }) } diff --git a/tools/cli/admin_db_scan_command.go b/tools/cli/admin_db_scan_command.go index 5781cb067e0..5cf7d99f35b 100644 --- a/tools/cli/admin_db_scan_command.go +++ b/tools/cli/admin_db_scan_command.go @@ -139,7 +139,7 @@ func checkExecution( invariants []executions.InvariantFactory, fetcher executions.ExecutionFetcher, ) (interface{}, invariant.ManagerCheckResult, error) { - execManager, err := getDeps(c).initializeExecutionStore(c, common.WorkflowIDToHistoryShard(req.WorkflowID, numberOfShards)) + execManager, err := getDeps(c).initializeExecutionManager(c, common.WorkflowIDToHistoryShard(req.WorkflowID, numberOfShards)) defer execManager.Close() if err != nil { return nil, invariant.ManagerCheckResult{}, fmt.Errorf("Error in execution check: %w", err) @@ -200,7 +200,7 @@ func listExecutionsByShardID( outputFile *os.File, ) error { - client, err := getDeps(c).initializeExecutionStore(c, shardID) + client, err := getDeps(c).initializeExecutionManager(c, shardID) defer client.Close() if err != nil { commoncli.Problem("Error in Admin DB unsupported WF scan: ", err) diff --git a/tools/cli/admin_timers.go b/tools/cli/admin_timers.go index a09772ffaea..427b4e9ca9e 100644 --- a/tools/cli/admin_timers.go +++ b/tools/cli/admin_timers.go @@ -80,7 +80,7 @@ func NewDBLoadCloser(c *cli.Context) (LoadCloser, error) { return nil, fmt.Errorf("error in NewDBLoadCloser: failed to get shard ID: %w", err) } - executionManager, err := getDeps(c).initializeExecutionStore(c, shardID) + executionManager, err := getDeps(c).initializeExecutionManager(c, shardID) if err != nil { return nil, fmt.Errorf("error in NewDBLoadCloser: failed to initialize execution store: %w", err) } diff --git a/tools/cli/app.go b/tools/cli/app.go index e73dd210720..53328992625 100644 --- a/tools/cli/app.go +++ b/tools/cli/app.go @@ -50,8 +50,8 @@ func WithIOHandler(h IOHandler) CLIAppOptions { } } -// WithPersistenceManagerFactory sets the PersistenceManagerFactory for the CLI app. -func WithPersistenceManagerFactory(factory PersistenceManagerFactory) CLIAppOptions { +// WithPersistenceManagerFactory sets the ManagerFactory for the CLI app. +func WithPersistenceManagerFactory(factory ManagerFactory) CLIAppOptions { return func(app *cli.App) { if app.Metadata == nil { return @@ -62,7 +62,7 @@ func WithPersistenceManagerFactory(factory PersistenceManagerFactory) CLIAppOpti return } - d.PersistenceManagerFactory = factory + d.ManagerFactory = factory } } @@ -79,7 +79,7 @@ func NewCliApp(cf ClientFactory, opts ...CLIAppOptions) *cli.App { app.Usage = "A command-line tool for cadence users" app.Version = version app.Metadata = map[string]any{ - depsKey: &deps{ClientFactory: cf, IOHandler: &defaultIOHandler{app: app}, PersistenceManagerFactory: &defaultPersistenceManagerFactory{}}, + depsKey: &deps{ClientFactory: cf, IOHandler: &defaultIOHandler{app: app}, ManagerFactory: &defaultManagerFactory{}}, } app.Flags = []cli.Flag{ &cli.StringFlag{ @@ -271,7 +271,7 @@ func getDeps(ctx *cli.Context) cliDeps { type cliDeps interface { ClientFactory IOHandler - PersistenceManagerFactory + ManagerFactory } type IOHandler interface { @@ -322,5 +322,5 @@ var _ cliDeps = &deps{} type deps struct { ClientFactory IOHandler - PersistenceManagerFactory + ManagerFactory } diff --git a/tools/cli/database.go b/tools/cli/database.go index 4cb4c787960..58e15dd03de 100644 --- a/tools/cli/database.go +++ b/tools/cli/database.go @@ -37,6 +37,7 @@ import ( "github.com/uber/cadence/common/persistence/client" "github.com/uber/cadence/common/persistence/nosql/nosqlplugin/cassandra" "github.com/uber/cadence/common/persistence/sql" + "github.com/uber/cadence/common/reconciliation/invariant" "github.com/uber/cadence/tools/common/flag" ) @@ -152,19 +153,20 @@ func getDBFlags() []cli.Flag { } } -type PersistenceManagerFactory interface { - initializeExecutionStore(c *cli.Context, shardID int) (persistence.ExecutionManager, error) +type ManagerFactory interface { + initializeExecutionManager(c *cli.Context, shardID int) (persistence.ExecutionManager, error) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) initPersistenceFactory(c *cli.Context) (client.Factory, error) + initializeInvariantManager(ivs []invariant.Invariant) (invariant.Manager, error) } -type defaultPersistenceManagerFactory struct { +type defaultManagerFactory struct { persistenceFactory client.Factory } -func (f *defaultPersistenceManagerFactory) initializeExecutionStore(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { +func (f *defaultManagerFactory) initializeExecutionManager(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { factory, err := f.getPersistenceFactory(c) if err != nil { return nil, fmt.Errorf("Failed to get persistence factory: %w", err) @@ -176,7 +178,7 @@ func (f *defaultPersistenceManagerFactory) initializeExecutionStore(c *cli.Conte return executionManager, nil } -func (f *defaultPersistenceManagerFactory) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) { +func (f *defaultManagerFactory) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) { factory, err := f.getPersistenceFactory(c) if err != nil { return nil, fmt.Errorf("Failed to get persistence factory: %w", err) @@ -188,7 +190,7 @@ func (f *defaultPersistenceManagerFactory) initializeHistoryManager(c *cli.Conte return historyManager, nil } -func (f *defaultPersistenceManagerFactory) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) { +func (f *defaultManagerFactory) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) { factory, err := f.getPersistenceFactory(c) if err != nil { return nil, fmt.Errorf("Failed to get persistence factory: %w", err) @@ -200,7 +202,7 @@ func (f *defaultPersistenceManagerFactory) initializeShardManager(c *cli.Context return shardManager, nil } -func (f *defaultPersistenceManagerFactory) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) { +func (f *defaultManagerFactory) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) { factory, err := f.getPersistenceFactory(c) if err != nil { return nil, fmt.Errorf("Failed to get persistence factory: %w", err) @@ -212,7 +214,7 @@ func (f *defaultPersistenceManagerFactory) initializeDomainManager(c *cli.Contex return domainManager, nil } -func (f *defaultPersistenceManagerFactory) getPersistenceFactory(c *cli.Context) (client.Factory, error) { +func (f *defaultManagerFactory) getPersistenceFactory(c *cli.Context) (client.Factory, error) { var err error if f.persistenceFactory == nil { f.persistenceFactory, err = getDeps(c).initPersistenceFactory(c) @@ -223,7 +225,7 @@ func (f *defaultPersistenceManagerFactory) getPersistenceFactory(c *cli.Context) return f.persistenceFactory, nil } -func (f *defaultPersistenceManagerFactory) initPersistenceFactory(c *cli.Context) (client.Factory, error) { +func (f *defaultManagerFactory) initPersistenceFactory(c *cli.Context) (client.Factory, error) { cfg, err := getDeps(c).ServerConfig(c) if err != nil { @@ -265,6 +267,10 @@ func (f *defaultPersistenceManagerFactory) initPersistenceFactory(c *cli.Context ), nil } +func (f *defaultManagerFactory) initializeInvariantManager(ivs []invariant.Invariant) (invariant.Manager, error) { + return invariant.NewInvariantManager(ivs), nil +} + func overrideDataStore(c *cli.Context, ds config.DataStore) (config.DataStore, error) { if c.IsSet(FlagDBType) { // overriding DBType will wipe out all settings, everything will be set from flags only diff --git a/tools/cli/database_mock.go b/tools/cli/database_mock.go index 045175a8c05..491a14e9fcc 100644 --- a/tools/cli/database_mock.go +++ b/tools/cli/database_mock.go @@ -34,33 +34,34 @@ import ( persistence "github.com/uber/cadence/common/persistence" client "github.com/uber/cadence/common/persistence/client" + invariant "github.com/uber/cadence/common/reconciliation/invariant" ) -// MockPersistenceManagerFactory is a mock of PersistenceManagerFactory interface. -type MockPersistenceManagerFactory struct { +// MockManagerFactory is a mock of ManagerFactory interface. +type MockManagerFactory struct { ctrl *gomock.Controller - recorder *MockPersistenceManagerFactoryMockRecorder + recorder *MockManagerFactoryMockRecorder } -// MockPersistenceManagerFactoryMockRecorder is the mock recorder for MockPersistenceManagerFactory. -type MockPersistenceManagerFactoryMockRecorder struct { - mock *MockPersistenceManagerFactory +// MockManagerFactoryMockRecorder is the mock recorder for MockManagerFactory. +type MockManagerFactoryMockRecorder struct { + mock *MockManagerFactory } -// NewMockPersistenceManagerFactory creates a new mock instance. -func NewMockPersistenceManagerFactory(ctrl *gomock.Controller) *MockPersistenceManagerFactory { - mock := &MockPersistenceManagerFactory{ctrl: ctrl} - mock.recorder = &MockPersistenceManagerFactoryMockRecorder{mock} +// NewMockManagerFactory creates a new mock instance. +func NewMockManagerFactory(ctrl *gomock.Controller) *MockManagerFactory { + mock := &MockManagerFactory{ctrl: ctrl} + mock.recorder = &MockManagerFactoryMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockPersistenceManagerFactory) EXPECT() *MockPersistenceManagerFactoryMockRecorder { +func (m *MockManagerFactory) EXPECT() *MockManagerFactoryMockRecorder { return m.recorder } // initPersistenceFactory mocks base method. -func (m *MockPersistenceManagerFactory) initPersistenceFactory(c *cli.Context) (client.Factory, error) { +func (m *MockManagerFactory) initPersistenceFactory(c *cli.Context) (client.Factory, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "initPersistenceFactory", c) ret0, _ := ret[0].(client.Factory) @@ -69,13 +70,13 @@ func (m *MockPersistenceManagerFactory) initPersistenceFactory(c *cli.Context) ( } // initPersistenceFactory indicates an expected call of initPersistenceFactory. -func (mr *MockPersistenceManagerFactoryMockRecorder) initPersistenceFactory(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initPersistenceFactory(c interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initPersistenceFactory", reflect.TypeOf((*MockPersistenceManagerFactory)(nil).initPersistenceFactory), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initPersistenceFactory", reflect.TypeOf((*MockManagerFactory)(nil).initPersistenceFactory), c) } // initializeDomainManager mocks base method. -func (m *MockPersistenceManagerFactory) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) { +func (m *MockManagerFactory) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "initializeDomainManager", c) ret0, _ := ret[0].(persistence.DomainManager) @@ -84,28 +85,28 @@ func (m *MockPersistenceManagerFactory) initializeDomainManager(c *cli.Context) } // initializeDomainManager indicates an expected call of initializeDomainManager. -func (mr *MockPersistenceManagerFactoryMockRecorder) initializeDomainManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeDomainManager(c interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeDomainManager", reflect.TypeOf((*MockPersistenceManagerFactory)(nil).initializeDomainManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeDomainManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeDomainManager), c) } -// initializeExecutionStore mocks base method. -func (m *MockPersistenceManagerFactory) initializeExecutionStore(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { +// initializeExecutionManager mocks base method. +func (m *MockManagerFactory) initializeExecutionManager(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeExecutionStore", c, shardID) + ret := m.ctrl.Call(m, "initializeExecutionManager", c, shardID) ret0, _ := ret[0].(persistence.ExecutionManager) ret1, _ := ret[1].(error) return ret0, ret1 } -// initializeExecutionStore indicates an expected call of initializeExecutionStore. -func (mr *MockPersistenceManagerFactoryMockRecorder) initializeExecutionStore(c, shardID interface{}) *gomock.Call { +// initializeExecutionManager indicates an expected call of initializeExecutionManager. +func (mr *MockManagerFactoryMockRecorder) initializeExecutionManager(c, shardID interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeExecutionStore", reflect.TypeOf((*MockPersistenceManagerFactory)(nil).initializeExecutionStore), c, shardID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeExecutionManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeExecutionManager), c, shardID) } // initializeHistoryManager mocks base method. -func (m *MockPersistenceManagerFactory) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) { +func (m *MockManagerFactory) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "initializeHistoryManager", c) ret0, _ := ret[0].(persistence.HistoryManager) @@ -114,13 +115,28 @@ func (m *MockPersistenceManagerFactory) initializeHistoryManager(c *cli.Context) } // initializeHistoryManager indicates an expected call of initializeHistoryManager. -func (mr *MockPersistenceManagerFactoryMockRecorder) initializeHistoryManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeHistoryManager(c interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeHistoryManager", reflect.TypeOf((*MockPersistenceManagerFactory)(nil).initializeHistoryManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeHistoryManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeHistoryManager), c) +} + +// initializeInvariantManager mocks base method. +func (m *MockManagerFactory) initializeInvariantManager(ivs []invariant.Invariant) (invariant.Manager, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "initializeInvariantManager", ivs) + ret0, _ := ret[0].(invariant.Manager) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// initializeInvariantManager indicates an expected call of initializeInvariantManager. +func (mr *MockManagerFactoryMockRecorder) initializeInvariantManager(ivs interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeInvariantManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeInvariantManager), ivs) } // initializeShardManager mocks base method. -func (m *MockPersistenceManagerFactory) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) { +func (m *MockManagerFactory) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "initializeShardManager", c) ret0, _ := ret[0].(persistence.ShardManager) @@ -129,7 +145,7 @@ func (m *MockPersistenceManagerFactory) initializeShardManager(c *cli.Context) ( } // initializeShardManager indicates an expected call of initializeShardManager. -func (mr *MockPersistenceManagerFactoryMockRecorder) initializeShardManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeShardManager(c interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeShardManager", reflect.TypeOf((*MockPersistenceManagerFactory)(nil).initializeShardManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeShardManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeShardManager), c) } From aa27130bd0c385882030cbada8051572aa505237 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Tue, 29 Oct 2024 19:33:25 -0700 Subject: [PATCH 08/12] fix naming --- tools/cli/admin_commands_test.go | 2 +- tools/cli/app.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/cli/admin_commands_test.go b/tools/cli/admin_commands_test.go index 6135b278f28..f16b5b07659 100644 --- a/tools/cli/admin_commands_test.go +++ b/tools/cli/admin_commands_test.go @@ -71,7 +71,7 @@ func newCLITestData(t *testing.T) *cliTestData { serverAdminClient: td.mockAdminClient, }, WithIOHandler(td.ioHandler), - WithPersistenceManagerFactory(td.mockManagerFactory), // Inject the mocked persistence manager factory + WithManagerFactory(td.mockManagerFactory), // Inject the mocked persistence manager factory ) return &td } diff --git a/tools/cli/app.go b/tools/cli/app.go index 53328992625..ce370259028 100644 --- a/tools/cli/app.go +++ b/tools/cli/app.go @@ -50,8 +50,8 @@ func WithIOHandler(h IOHandler) CLIAppOptions { } } -// WithPersistenceManagerFactory sets the ManagerFactory for the CLI app. -func WithPersistenceManagerFactory(factory ManagerFactory) CLIAppOptions { +// WithManagerFactory sets the ManagerFactory for the CLI app. +func WithManagerFactory(factory ManagerFactory) CLIAppOptions { return func(app *cli.App) { if app.Metadata == nil { return From 451ac734e0971b0bb1fc2b9a1bb0734be897ac4a Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Wed, 30 Oct 2024 10:17:49 -0700 Subject: [PATCH 09/12] remove nil check before manager close; add error check in context set flags --- tools/cli/admin_db_clean_command.go | 10 +++---- tools/cli/admin_db_clean_command_test.go | 38 +++++++++++++----------- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/tools/cli/admin_db_clean_command.go b/tools/cli/admin_db_clean_command.go index a026fa153da..298b00953c4 100644 --- a/tools/cli/admin_db_clean_command.go +++ b/tools/cli/admin_db_clean_command.go @@ -133,19 +133,17 @@ func fixExecution( execution *store.ScanOutputEntity, ) (invariant.ManagerFixResult, error) { execManager, err := getDeps(c).initializeExecutionManager(c, execution.Execution.(entity.Entity).GetShardID()) - if execManager != nil { - defer execManager.Close() - } if err != nil { return invariant.ManagerFixResult{}, err } + defer execManager.Close() + historyV2Mgr, err := getDeps(c).initializeHistoryManager(c) - if historyV2Mgr != nil { - defer historyV2Mgr.Close() - } if err != nil { return invariant.ManagerFixResult{}, err } + defer historyV2Mgr.Close() + pr := persistence.NewPersistenceRetryer( execManager, historyV2Mgr, diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index 9d44e57e136..67ec7c020c7 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -25,6 +25,7 @@ package cli import ( "flag" "fmt" + "github.com/stretchr/testify/require" "os" "testing" @@ -55,8 +56,8 @@ func TestAdminDBClean_noFixExecution(t *testing.T) { set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") // Set actual values for the flags - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, "input.json") + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, "input.json")) return cli.NewContext(app, set, nil) }, @@ -83,8 +84,8 @@ func TestAdminDBClean_noFixExecution(t *testing.T) { set := flag.NewFlagSet("test", 0) set.String(FlagScanType, "", "scan type flag") set.String(FlagInputFile, "", "Input file flag") - _ = set.Set(FlagScanType, "unknown") - _ = set.Set(FlagInputFile, "input.json") + require.NoError(t, set.Set(FlagScanType, "unknown")) + require.NoError(t, set.Set(FlagInputFile, "input.json")) return cli.NewContext(app, set, nil) }, inputFileData: ``, @@ -103,8 +104,9 @@ func TestAdminDBClean_noFixExecution(t *testing.T) { set.Var(cli.NewStringSlice("invalid_collection", "history"), FlagInvariantCollection, "invariant collection flag") // Set actual values for the flags - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, "input.json") + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, "input.json")) + return cli.NewContext(app, set, nil) }, inputFileData: ``, @@ -118,9 +120,11 @@ func TestAdminDBClean_noFixExecution(t *testing.T) { set.String(FlagScanType, "", "scan type flag") set.String(FlagInvariantCollection, "", "invariant collection flag") set.String(FlagInputFile, "", "Input file flag") - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInvariantCollection, "invalid_collection") // Collection will trigger no invariants - _ = set.Set(FlagInputFile, "input.json") + + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, "input.json")) + require.NoError(t, set.Set(FlagInvariantCollection, "invalid_collection")) + return cli.NewContext(app, set, nil) }, inputFileData: `[{"Execution": {"ShardID": 1}}]`, @@ -187,8 +191,8 @@ func TestAdminDBClean_inFixExecution(t *testing.T) { set.String(FlagInputFile, "", "Input file flag") set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, inputFilePath)) return cli.NewContext(td.app, set, nil) }, @@ -221,8 +225,8 @@ func TestAdminDBClean_inFixExecution(t *testing.T) { set.String(FlagInputFile, "", "Input file flag") set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, inputFilePath)) return cli.NewContext(td.app, set, nil) }, @@ -251,8 +255,8 @@ func TestAdminDBClean_inFixExecution(t *testing.T) { set.String(FlagInputFile, "", "Input file flag") set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, inputFilePath)) return cli.NewContext(td.app, set, nil) }, @@ -271,8 +275,8 @@ func TestAdminDBClean_inFixExecution(t *testing.T) { set.String(FlagInputFile, "", "Input file flag") set.Var(cli.NewStringSlice("CollectionHistory", "CollectionDomain"), FlagInvariantCollection, "invariant collection flag") - _ = set.Set(FlagScanType, "ConcreteExecutionType") - _ = set.Set(FlagInputFile, inputFilePath) // 传递临时文件路径 + require.NoError(t, set.Set(FlagScanType, "ConcreteExecutionType")) + require.NoError(t, set.Set(FlagInputFile, inputFilePath)) return cli.NewContext(td.app, set, nil) }, From 669ee2474b0394c0ae92bebd5deb9f585b3f3c08 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Wed, 30 Oct 2024 10:52:45 -0700 Subject: [PATCH 10/12] add test for opt functions; rename database mock --- tools/cli/admin_db_clean_command_test.go | 2 +- tools/cli/app_test.go | 129 ++++++++++++++++++ tools/cli/database.go | 2 +- ...tabase_mock.go => mock_manager_factory.go} | 50 +++---- 4 files changed, 156 insertions(+), 27 deletions(-) rename tools/cli/{database_mock.go => mock_manager_factory.go} (77%) diff --git a/tools/cli/admin_db_clean_command_test.go b/tools/cli/admin_db_clean_command_test.go index 67ec7c020c7..a2ef7f483ee 100644 --- a/tools/cli/admin_db_clean_command_test.go +++ b/tools/cli/admin_db_clean_command_test.go @@ -25,12 +25,12 @@ package cli import ( "flag" "fmt" - "github.com/stretchr/testify/require" "os" "testing" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/urfave/cli/v2" "github.com/uber/cadence/common/persistence" diff --git a/tools/cli/app_test.go b/tools/cli/app_test.go index 932d168b3cd..ab1e31855e1 100644 --- a/tools/cli/app_test.go +++ b/tools/cli/app_test.go @@ -925,3 +925,132 @@ func (s *cliAppSuite) TestConvertArray() { s.Nil(res) } } + +func TestWithManagerFactory(t *testing.T) { + tests := []struct { + name string + app *cli.App + expectFactory bool + expectNilMeta bool + expectDepsType bool + }{ + { + name: "Valid Metadata with deps key", + app: &cli.App{ + Metadata: map[string]interface{}{ + depsKey: &deps{}, + }, + }, + expectFactory: true, + expectNilMeta: false, + expectDepsType: true, + }, + { + name: "No Metadata", + app: &cli.App{Metadata: nil}, + expectFactory: false, + expectNilMeta: true, + expectDepsType: false, + }, + { + name: "Incorrect deps key type", + app: &cli.App{ + Metadata: map[string]interface{}{ + depsKey: "invalid_type", + }, + }, + expectFactory: false, + expectNilMeta: false, + expectDepsType: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockFactory := NewMockManagerFactory(ctrl) + + option := WithManagerFactory(mockFactory) + option(tt.app) + + if tt.expectNilMeta { + assert.Nil(t, tt.app.Metadata, "Expected Metadata to be nil") + } else { + d, ok := tt.app.Metadata[depsKey].(*deps) + if tt.expectDepsType { + assert.True(t, ok, "Expected depsKey to be of type *deps") + if tt.expectFactory { + assert.Equal(t, mockFactory, d.ManagerFactory, "Expected ManagerFactory to be set correctly") + } + } else { + assert.False(t, ok, "Expected depsKey not to be of type *deps") + } + } + }) + } +} + +func TestWithIOHandler(t *testing.T) { + tests := []struct { + name string + app *cli.App + expectHandler bool + expectNilMeta bool + expectDepsType bool + }{ + { + name: "Valid Metadata with deps key", + app: &cli.App{ + Metadata: map[string]interface{}{ + depsKey: &deps{}, + }, + }, + expectHandler: true, + expectNilMeta: false, + expectDepsType: true, + }, + { + name: "No Metadata", + app: &cli.App{Metadata: nil}, + expectHandler: false, + expectNilMeta: true, + expectDepsType: false, + }, + { + name: "Incorrect deps key type", + app: &cli.App{ + Metadata: map[string]interface{}{ + depsKey: "invalid_type", + }, + }, + expectHandler: false, + expectNilMeta: false, + expectDepsType: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testHandler := testIOHandler{} + + option := WithIOHandler(&testHandler) + option(tt.app) + + if tt.expectNilMeta { + assert.Nil(t, tt.app.Metadata, "Expected Metadata to be nil") + } else { + d, ok := tt.app.Metadata[depsKey].(*deps) + if tt.expectDepsType { + assert.True(t, ok, "Expected depsKey to be of type *deps") + if tt.expectHandler { + assert.Equal(t, &testHandler, d.IOHandler, "Expected IOHandler to be set correctly") + } + } else { + assert.False(t, ok, "Expected depsKey not to be of type *deps") + } + } + }) + } +} diff --git a/tools/cli/database.go b/tools/cli/database.go index 58e15dd03de..a049ff44032 100644 --- a/tools/cli/database.go +++ b/tools/cli/database.go @@ -18,7 +18,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -//go:generate mockgen -package $GOPACKAGE -source $GOFILE -destination database_mock.go -self_package github.com/uber/cadence/tools/cli +//go:generate mockgen -package $GOPACKAGE -destination mock_manager_factory.go -self_package github.com/uber/cadence/tools/cli github.com/uber/cadence/tools/cli ManagerFactory package cli diff --git a/tools/cli/database_mock.go b/tools/cli/mock_manager_factory.go similarity index 77% rename from tools/cli/database_mock.go rename to tools/cli/mock_manager_factory.go index 491a14e9fcc..fcbfec356fd 100644 --- a/tools/cli/database_mock.go +++ b/tools/cli/mock_manager_factory.go @@ -21,7 +21,7 @@ // SOFTWARE. // Code generated by MockGen. DO NOT EDIT. -// Source: database.go +// Source: github.com/uber/cadence/tools/cli (interfaces: ManagerFactory) // Package cli is a generated GoMock package. package cli @@ -61,91 +61,91 @@ func (m *MockManagerFactory) EXPECT() *MockManagerFactoryMockRecorder { } // initPersistenceFactory mocks base method. -func (m *MockManagerFactory) initPersistenceFactory(c *cli.Context) (client.Factory, error) { +func (m *MockManagerFactory) initPersistenceFactory(arg0 *cli.Context) (client.Factory, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initPersistenceFactory", c) + ret := m.ctrl.Call(m, "initPersistenceFactory", arg0) ret0, _ := ret[0].(client.Factory) ret1, _ := ret[1].(error) return ret0, ret1 } // initPersistenceFactory indicates an expected call of initPersistenceFactory. -func (mr *MockManagerFactoryMockRecorder) initPersistenceFactory(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initPersistenceFactory(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initPersistenceFactory", reflect.TypeOf((*MockManagerFactory)(nil).initPersistenceFactory), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initPersistenceFactory", reflect.TypeOf((*MockManagerFactory)(nil).initPersistenceFactory), arg0) } // initializeDomainManager mocks base method. -func (m *MockManagerFactory) initializeDomainManager(c *cli.Context) (persistence.DomainManager, error) { +func (m *MockManagerFactory) initializeDomainManager(arg0 *cli.Context) (persistence.DomainManager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeDomainManager", c) + ret := m.ctrl.Call(m, "initializeDomainManager", arg0) ret0, _ := ret[0].(persistence.DomainManager) ret1, _ := ret[1].(error) return ret0, ret1 } // initializeDomainManager indicates an expected call of initializeDomainManager. -func (mr *MockManagerFactoryMockRecorder) initializeDomainManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeDomainManager(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeDomainManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeDomainManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeDomainManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeDomainManager), arg0) } // initializeExecutionManager mocks base method. -func (m *MockManagerFactory) initializeExecutionManager(c *cli.Context, shardID int) (persistence.ExecutionManager, error) { +func (m *MockManagerFactory) initializeExecutionManager(arg0 *cli.Context, arg1 int) (persistence.ExecutionManager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeExecutionManager", c, shardID) + ret := m.ctrl.Call(m, "initializeExecutionManager", arg0, arg1) ret0, _ := ret[0].(persistence.ExecutionManager) ret1, _ := ret[1].(error) return ret0, ret1 } // initializeExecutionManager indicates an expected call of initializeExecutionManager. -func (mr *MockManagerFactoryMockRecorder) initializeExecutionManager(c, shardID interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeExecutionManager(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeExecutionManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeExecutionManager), c, shardID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeExecutionManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeExecutionManager), arg0, arg1) } // initializeHistoryManager mocks base method. -func (m *MockManagerFactory) initializeHistoryManager(c *cli.Context) (persistence.HistoryManager, error) { +func (m *MockManagerFactory) initializeHistoryManager(arg0 *cli.Context) (persistence.HistoryManager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeHistoryManager", c) + ret := m.ctrl.Call(m, "initializeHistoryManager", arg0) ret0, _ := ret[0].(persistence.HistoryManager) ret1, _ := ret[1].(error) return ret0, ret1 } // initializeHistoryManager indicates an expected call of initializeHistoryManager. -func (mr *MockManagerFactoryMockRecorder) initializeHistoryManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeHistoryManager(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeHistoryManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeHistoryManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeHistoryManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeHistoryManager), arg0) } // initializeInvariantManager mocks base method. -func (m *MockManagerFactory) initializeInvariantManager(ivs []invariant.Invariant) (invariant.Manager, error) { +func (m *MockManagerFactory) initializeInvariantManager(arg0 []invariant.Invariant) (invariant.Manager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeInvariantManager", ivs) + ret := m.ctrl.Call(m, "initializeInvariantManager", arg0) ret0, _ := ret[0].(invariant.Manager) ret1, _ := ret[1].(error) return ret0, ret1 } // initializeInvariantManager indicates an expected call of initializeInvariantManager. -func (mr *MockManagerFactoryMockRecorder) initializeInvariantManager(ivs interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeInvariantManager(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeInvariantManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeInvariantManager), ivs) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeInvariantManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeInvariantManager), arg0) } // initializeShardManager mocks base method. -func (m *MockManagerFactory) initializeShardManager(c *cli.Context) (persistence.ShardManager, error) { +func (m *MockManagerFactory) initializeShardManager(arg0 *cli.Context) (persistence.ShardManager, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "initializeShardManager", c) + ret := m.ctrl.Call(m, "initializeShardManager", arg0) ret0, _ := ret[0].(persistence.ShardManager) ret1, _ := ret[1].(error) return ret0, ret1 } // initializeShardManager indicates an expected call of initializeShardManager. -func (mr *MockManagerFactoryMockRecorder) initializeShardManager(c interface{}) *gomock.Call { +func (mr *MockManagerFactoryMockRecorder) initializeShardManager(arg0 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeShardManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeShardManager), c) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "initializeShardManager", reflect.TypeOf((*MockManagerFactory)(nil).initializeShardManager), arg0) } From 5cff73348b622f17779981cbcf056c4499f0b9a5 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Wed, 30 Oct 2024 12:58:54 -0700 Subject: [PATCH 11/12] add database test file --- tools/cli/database_test.go | 138 +++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 tools/cli/database_test.go diff --git a/tools/cli/database_test.go b/tools/cli/database_test.go new file mode 100644 index 00000000000..af35dd1b969 --- /dev/null +++ b/tools/cli/database_test.go @@ -0,0 +1,138 @@ +package cli + +import ( + "fmt" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/uber/cadence/common/persistence" + "github.com/uber/cadence/common/persistence/client" + "github.com/urfave/cli/v2" +) + +func TestDefaultManagerFactory(t *testing.T) { + tests := []struct { + name string + setupMocks func(*client.MockFactory, *gomock.Controller) interface{} + methodToTest func(*defaultManagerFactory, *cli.Context) (interface{}, error) + expectError bool + }{ + { + name: "initializeExecutionManager - success", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockExecutionManager := persistence.NewMockExecutionManager(ctrl) + mockFactory.EXPECT().NewExecutionManager(gomock.Any()).Return(mockExecutionManager, nil) + return mockExecutionManager + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeExecutionManager(ctx, 1) + }, + expectError: false, + }, + { + name: "initializeExecutionManager - error", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockFactory.EXPECT().NewExecutionManager(gomock.Any()).Return(nil, fmt.Errorf("some error")) + return nil + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeExecutionManager(ctx, 1) + }, + expectError: true, + }, + { + name: "initializeHistoryManager - success", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockHistoryManager := persistence.NewMockHistoryManager(ctrl) + mockFactory.EXPECT().NewHistoryManager().Return(mockHistoryManager, nil) + return mockHistoryManager + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeHistoryManager(ctx) + }, + expectError: false, + }, + { + name: "initializeHistoryManager - error", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockFactory.EXPECT().NewHistoryManager().Return(nil, fmt.Errorf("some error")) + return nil + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeHistoryManager(ctx) + }, + expectError: true, + }, + { + name: "initializeShardManager - success", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockShardManager := persistence.NewMockShardManager(ctrl) + mockFactory.EXPECT().NewShardManager().Return(mockShardManager, nil) + return mockShardManager + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeShardManager(ctx) + }, + expectError: false, + }, + { + name: "initializeShardManager - error", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockFactory.EXPECT().NewShardManager().Return(nil, fmt.Errorf("some error")) + return nil + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeShardManager(ctx) + }, + expectError: true, + }, + { + name: "initializeDomainManager - success", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockDomainManager := persistence.NewMockDomainManager(ctrl) + mockFactory.EXPECT().NewDomainManager().Return(mockDomainManager, nil) + return mockDomainManager + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeDomainManager(ctx) + }, + expectError: false, + }, + { + name: "initializeDomainManager - error", + setupMocks: func(mockFactory *client.MockFactory, ctrl *gomock.Controller) interface{} { + mockFactory.EXPECT().NewDomainManager().Return(nil, fmt.Errorf("some error")) + return nil + }, + methodToTest: func(f *defaultManagerFactory, ctx *cli.Context) (interface{}, error) { + return f.initializeDomainManager(ctx) + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockFactory := client.NewMockFactory(ctrl) + expectedManager := tt.setupMocks(mockFactory, ctrl) + + f := &defaultManagerFactory{persistenceFactory: mockFactory} + app := &cli.App{} + ctx := cli.NewContext(app, nil, nil) + + result, err := tt.methodToTest(f, ctx) + + if tt.expectError { + assert.Error(t, err) + assert.Nil(t, result) + } else { + assert.NoError(t, err) + assert.Equal(t, expectedManager, result) + } + }) + } +} From 7c52a206d11a01534f1441cb33a85564ae066626 Mon Sep 17 00:00:00 2001 From: Bowen Xiao Date: Wed, 30 Oct 2024 13:19:58 -0700 Subject: [PATCH 12/12] fmt --- tools/cli/database_test.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tools/cli/database_test.go b/tools/cli/database_test.go index af35dd1b969..b52324b5de6 100644 --- a/tools/cli/database_test.go +++ b/tools/cli/database_test.go @@ -1,3 +1,25 @@ +// The MIT License (MIT) + +// Copyright (c) 2017-2020 Uber Technologies Inc. + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + package cli import ( @@ -6,9 +28,10 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/urfave/cli/v2" + "github.com/uber/cadence/common/persistence" "github.com/uber/cadence/common/persistence/client" - "github.com/urfave/cli/v2" ) func TestDefaultManagerFactory(t *testing.T) {