[CLOB-930] Remove Reset method and swap to appOptions/baseAppOptions …

…being explicitly set instead of an appCreatorFn (#697) This will simplify creating e2e test framework that better tests non-determinism/flags/... since the e2e test framework will want to control how the App instance is created.
dydxprotocol · Oct 24, 2023 · ed6429f · ed6429f
1 parent 1f065b4
commit ed6429f
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 60 deletions.
diff --git a/protocol/app/ante/gas_test.go b/protocol/app/ante/gas_test.go
@@ -181,10 +181,10 @@ func TestSubmitTxnWithGas(t *testing.T) {
 			}
 
 			tApp := testapp.NewTestAppBuilder(t).
-				WithAppCreatorFn(
-					testapp.DefaultTestAppCreatorFn(map[string]interface{}{},
-						baseapp.SetMinGasPrices(cmd.MinGasPrice),
-					)).
+				WithAppOptions(
+					map[string]interface{}{},
+					baseapp.SetMinGasPrices(cmd.MinGasPrice),
+				).
 				Build()
 			ctx := tApp.InitChain()
 

diff --git a/protocol/app/prepare/full_node_prepare_proposal_test.go b/protocol/app/prepare/full_node_prepare_proposal_test.go
@@ -28,7 +28,7 @@ func TestFullNodePrepareProposalHandler(t *testing.T) {
 		flags.NonValidatingFullNodeFlag: true,
 		testlog.LoggerInstanceForTest:   logger,
 	}
-	tApp := testApp.NewTestAppBuilder(t).WithAppCreatorFn(testApp.DefaultTestAppCreatorFn(appOpts)).Build()
+	tApp := testApp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 
 	found := false
 	tApp.AdvanceToBlock(2, testApp.AdvanceToBlockOptions{

diff --git a/protocol/testutil/app/app.go b/protocol/testutil/app/app.go
@@ -123,17 +123,6 @@ func DefaultTestApp(customFlags map[string]interface{}, baseAppOptions ...func(*
 	return dydxApp
 }
 
-// DefaultTestAppCreatorFn is a wrapper function around DefaultTestApp using the specified custom flags, and allowing
-// for optional base app options.
-func DefaultTestAppCreatorFn(
-	customFlags map[string]interface{},
-	baseAppOptions ...func(*baseapp.BaseApp),
-) AppCreatorFn {
-	return func() *app.App {
-		return DefaultTestApp(customFlags, baseAppOptions...)
-	}
-}
-
 // DefaultGenesis returns a genesis doc using configuration from the local net with a genesis time
 // equivalent to unix epoch + 1 nanosecond. We specifically use non-zero because stateful orders
 // validate that block time is non-zero (https://github.com/dydxprotocol/v4-chain/protocol/blob/
@@ -257,8 +246,8 @@ func NewTestAppBuilder(t testing.TB) TestAppBuilder {
 	}
 	return TestAppBuilder{
 		genesisDocFn:         DefaultGenesis,
-		appCreatorFn:         DefaultTestAppCreatorFn(nil),
 		usesDefaultAppConfig: true,
+		appOptions:           make(map[string]interface{}),
 		executeCheckTxs: func(ctx sdk.Context, app *app.App) (stop bool) {
 			return true
 		},
@@ -272,8 +261,9 @@ func NewTestAppBuilder(t testing.TB) TestAppBuilder {
 // immutable.
 type TestAppBuilder struct {
 	genesisDocFn         GenesisDocCreatorFn
-	appCreatorFn         func() *app.App
 	usesDefaultAppConfig bool
+	appOptions           map[string]interface{}
+	baseAppOptions       []func(*baseapp.BaseApp)
 	executeCheckTxs      ExecuteCheckTxs
 	t                    testing.TB
 }
@@ -285,10 +275,13 @@ func (tApp TestAppBuilder) WithGenesisDocFn(fn GenesisDocCreatorFn) TestAppBuild
 	return tApp
 }
 
-// WithAppCreatorFn returns a builder like this one with the specified function that will be used to create
-// the application.
-func (tApp TestAppBuilder) WithAppCreatorFn(fn AppCreatorFn) TestAppBuilder {
-	tApp.appCreatorFn = fn
+// WithAppOptions returns a builder like this one with the specified app options.
+func (tApp TestAppBuilder) WithAppOptions(
+	appOptions map[string]interface{},
+	baseAppOptions ...func(*baseapp.BaseApp),
+) TestAppBuilder {
+	tApp.appOptions = appOptions
+	tApp.baseAppOptions = baseAppOptions
 	tApp.usesDefaultAppConfig = false
 	return tApp
 }
@@ -334,7 +327,7 @@ func (tApp *TestApp) Builder() TestAppBuilder {
 // InitChain initializes the chain. Will panic if initialized more than once.
 func (tApp *TestApp) InitChain() sdk.Context {
 	if tApp.App != nil {
-		panic(errors.New("Cannot initialize chain that has been initialized already. Missing a Reset()?"))
+		panic(errors.New("Cannot initialize chain that has been initialized already."))
 	}
 	tApp.initChainIfNeeded()
 	return tApp.App.NewContext(true, tApp.header)
@@ -347,7 +340,7 @@ func (tApp *TestApp) initChainIfNeeded() {
 
 	// Get the initial genesis state and initialize the chain and commit the results of the initialization.
 	tApp.genesis = tApp.builder.genesisDocFn()
-	tApp.App = tApp.builder.appCreatorFn()
+	tApp.App = DefaultTestApp(tApp.builder.appOptions, tApp.builder.baseAppOptions...)
 	if tApp.builder.usesDefaultAppConfig {
 		tApp.App.Server.DisableUpdateMonitoringForTesting()
 	}
@@ -544,20 +537,6 @@ func (tApp *TestApp) AdvanceToBlock(
 	return tApp.App.NewContext(true, tApp.header)
 }
 
-// Reset resets the chain such that it can be initialized and executed again.
-func (tApp *TestApp) Reset() {
-	if tApp.App != nil {
-		if err := tApp.App.Close(); err != nil {
-			tApp.builder.t.Fatal(err)
-		}
-	}
-	tApp.App = nil
-	tApp.genesis = types.GenesisDoc{}
-	tApp.header = tmproto.Header{}
-	tApp.passingCheckTxs = nil
-	tApp.halted = false
-}
-
 // GetHeader fetches the current header of the test app.
 func (tApp *TestApp) GetHeader() tmproto.Header {
 	return tApp.header

diff --git a/protocol/x/clob/e2e/app_test.go b/protocol/x/clob/e2e/app_test.go
@@ -401,7 +401,7 @@ func TestFailsDeliverTxWithIncorrectlySignedPlaceOrderTx(t *testing.T) {
 			appOpts := map[string]interface{}{
 				indexer.MsgSenderInstanceForTest: msgSender,
 			}
-			tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+			tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			tApp.InitChain()
 			ctx := tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})
 
@@ -474,7 +474,7 @@ func TestFailsDeliverTxWithUnsignedTransactions(t *testing.T) {
 			appOpts := map[string]interface{}{
 				indexer.MsgSenderInstanceForTest: msgSender,
 			}
-			tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+			tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			tApp.InitChain()
 			tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})
 
@@ -506,7 +506,7 @@ func TestStats(t *testing.T) {
 	appOpts := map[string]interface{}{
 		indexer.MsgSenderInstanceForTest: msgSender,
 	}
-	tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+	tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 
 	// Epochs start at block height 2.
 	startTime := time.Unix(10, 0).UTC()

diff --git a/protocol/x/clob/e2e/long_term_orders_test.go b/protocol/x/clob/e2e/long_term_orders_test.go
@@ -1221,7 +1221,7 @@ func TestPlaceLongTermOrder(t *testing.T) {
 			appOpts := map[string]interface{}{
 				indexer.MsgSenderInstanceForTest: msgSender,
 			}
-			tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+			tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			ctx := tApp.InitChain()
 
 			for _, ordersAndExpectations := range tc.ordersAndExpectationsPerBlock {

diff --git a/protocol/x/clob/e2e/short_term_orders_test.go b/protocol/x/clob/e2e/short_term_orders_test.go
@@ -1,6 +1,7 @@
 package clob_test
 
 import (
+	"github.com/dydxprotocol/v4-chain/protocol/indexer"
 	"testing"
 
 	"golang.org/x/exp/slices"
@@ -9,7 +10,6 @@ import (
 	"github.com/dydxprotocol/v4-chain/protocol/dtypes"
 	"github.com/dydxprotocol/v4-chain/protocol/lib"
 
-	"github.com/dydxprotocol/v4-chain/protocol/indexer"
 	indexerevents "github.com/dydxprotocol/v4-chain/protocol/indexer/events"
 	"github.com/dydxprotocol/v4-chain/protocol/indexer/indexer_manager"
 	"github.com/dydxprotocol/v4-chain/protocol/indexer/msgsender"
@@ -25,11 +25,7 @@ import (
 )
 
 func TestPlaceOrder(t *testing.T) {
-	msgSender := msgsender.NewIndexerMessageSenderInMemoryCollector()
-	appOpts := map[string]interface{}{
-		indexer.MsgSenderInstanceForTest: msgSender,
-	}
-	tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+	tApp := testapp.NewTestAppBuilder(t).Build()
 	ctx := tApp.InitChain()
 
 	aliceSubaccount := tApp.App.SubaccountsKeeper.GetSubaccount(ctx, constants.Alice_Num0)
@@ -568,10 +564,13 @@ func TestPlaceOrder(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			// Reset for each iteration of the loop
-			tApp.Reset()
-
+			msgSender := msgsender.NewIndexerMessageSenderInMemoryCollector()
+			appOpts := map[string]interface{}{
+				indexer.MsgSenderInstanceForTest: msgSender,
+			}
+			tApp = testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			ctx = tApp.InitChain()
+
 			// Clear any messages produced prior to these checkTx calls.
 			msgSender.Clear()
 			for _, order := range tc.orders {

diff --git a/protocol/x/rewards/keeper/keeper_test.go b/protocol/x/rewards/keeper/keeper_test.go
@@ -80,8 +80,6 @@ func TestSetRewardShare_FailsWithNonpositiveWeight(t *testing.T) {
 }
 
 func TestAddRewardShareToAddress(t *testing.T) {
-	tApp := testapp.NewTestAppBuilder(t).Build()
-
 	tests := map[string]struct {
 		prevRewardShare     *types.RewardShare // nil if no previous share
 		newWeight           *big.Int
@@ -120,7 +118,7 @@ func TestAddRewardShareToAddress(t *testing.T) {
 	// Run tests.
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			tApp.Reset()
+			tApp := testapp.NewTestAppBuilder(t).Build()
 			ctx := tApp.InitChain()
 			k := tApp.App.RewardsKeeper
 
@@ -143,7 +141,6 @@ func TestAddRewardShareToAddress(t *testing.T) {
 }
 
 func TestAddRewardSharesForFill(t *testing.T) {
-	tApp := testapp.NewTestAppBuilder(t).Build()
 	makerAddress := TestAddress1
 	takerAdderss := TestAddress2
 
@@ -268,7 +265,7 @@ func TestAddRewardSharesForFill(t *testing.T) {
 	// Run tests.
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			tApp.Reset()
+			tApp := testapp.NewTestAppBuilder(t).Build()
 			ctx := tApp.InitChain()
 			k := tApp.App.RewardsKeeper
 
@@ -653,8 +650,6 @@ func TestProcessRewardsForBlock(t *testing.T) {
 				)
 				return genesis
 			}).Build()
-
-			tApp.Reset()
 			ctx := tApp.InitChain()
 			k := tApp.App.RewardsKeeper
 

diff --git a/protocol/x/sending/app_test.go b/protocol/x/sending/app_test.go
@@ -95,7 +95,7 @@ func TestMsgDepositToSubaccount(t *testing.T) {
 			appOpts := map[string]interface{}{
 				indexer.MsgSenderInstanceForTest: msgSender,
 			}
-			tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+			tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			ctx := tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})
 			// Clear any messages produced prior to CheckTx calls.
 			msgSender.Clear()
@@ -284,7 +284,7 @@ func TestMsgWithdrawFromSubaccount(t *testing.T) {
 			appOpts := map[string]interface{}{
 				indexer.MsgSenderInstanceForTest: msgSender,
 			}
-			tApp := testapp.NewTestAppBuilder(t).WithAppCreatorFn(testapp.DefaultTestAppCreatorFn(appOpts)).Build()
+			tApp := testapp.NewTestAppBuilder(t).WithAppOptions(appOpts).Build()
 			ctx := tApp.AdvanceToBlock(2, testapp.AdvanceToBlockOptions{})
 			// Clear any messages produced prior to CheckTx calls.
 			msgSender.Clear()