Add testing harness for e2e application logic tests #850

codchen · 2023-06-07T15:03:08Z

Describe your changes and provide context

Add a testing harness to simplify writing application-level e2e test. The goal is to allow testing the full application write path that roots from FinalizeBlock and Commit, since more granular tests like ones on keeper level lack the capability to uncover bugs caused by improper interactions between modules or invalid assumptions across transactions and Begin/Mid/EndBlock logics. Specifically the harness includes:

A wrapper around the core Sei App object, which provides simple interfaces for processing a block and transaction signing. [testutils/processblock/common.go]
A set of helpers to define the "genesis" state of a test. [testutils/processblock/genesis*.go]
A set of helpers to create messages to test with. [testutils/processblock/msgs/*.go]
A set of verifiers to assert test outcome. [testutils/processblock/verify/*.go]

A test written using this harness typically includes three phases:

Initialization - create the test app wrapper object and call the genesis helper to initialize the app state
Msg Creation - create messages to be tested, group messages into transactions, and sign the transactions
Run and Verify - group transactions into blocks and run the blocks through the app wrapper, and verify state using the verifiers.
tests/template_test.go provided a template for writing such tests, and tests/dex_test.go is a concrete examples.

Next steps:

Add message helpers for other core functionalities
Add verifiers for other core functionalities
Add tests for other core functionalities

Testing performed to validate your change

tests

testutil/processblock/common.go

+		panic(err)
+	}
+	_, err = a.InitChain(context.Background(), &types.RequestInitChain{
+		Time:            time.Now(),


testutil/processblock/common.go

+		Hash:                []byte("abc"), // no needed for application logic
+		Height:              a.height,
+		ProposerAddress:     getValAddress(a.GetProposer()),
+		Time:                time.Now(),


testutil/processblock/verify/dex.go

+		for market := range markets {
+			orderPlacements := []*dextypes.Order{}
+			if o, ok := orderPlacementsByMarket[market]; ok {
+				orderPlacements = o
+			}
+			orderCancellations := []*dextypes.Cancellation{}
+			if c, ok := orderCancellationsByMarket[market]; ok {
+				orderCancellations = c
+			}
+			parts := strings.Split(market, ",")
+			longBook, shortBook := expectedOrdersForMarket(ctx, keeper, orderPlacements, orderCancellations, parts[0], parts[1], parts[2])
+			expectedLongBookByMarket[market] = longBook
+			expectedShortBookByMarket[market] = shortBook
+		}


testutil/processblock/verify/dex.go

+		for market, longBook := range expectedLongBookByMarket {
+			parts := strings.Split(market, ",")
+			contract := parts[0]
+			priceDenom := parts[1]
+			assetDenom := parts[2]
+			require.Equal(t, len(longBook), len(keeper.GetAllLongBookForPair(ctx, contract, priceDenom, assetDenom)))
+			for price, entry := range longBook {
+				actual, found := keeper.GetLongOrderBookEntryByPrice(ctx, contract, price, priceDenom, assetDenom)
+				require.True(t, found)
+				require.Equal(t, *entry, *(actual.GetOrderEntry()))
+			}
+		}


testutil/processblock/verify/dex.go

+		for market, shortBook := range expectedShortBookByMarket {
+			parts := strings.Split(market, ",")
+			contract := parts[0]
+			priceDenom := parts[1]
+			assetDenom := parts[2]
+			require.Equal(t, len(shortBook), len(keeper.GetAllShortBookForPair(ctx, contract, priceDenom, assetDenom)))
+			for price, entry := range shortBook {
+				actual, found := keeper.GetShortOrderBookEntryByPrice(ctx, contract, price, priceDenom, assetDenom)
+				require.True(t, found)
+				require.Equal(t, *entry, *(actual.GetOrderEntry()))
+			}
+		}


testutil/processblock/verify/dex.go

codecov · 2023-06-08T09:04:41Z

Codecov Report

Merging #850 (40ee107) into master (95da49b) will increase coverage by 0.11%.
The diff coverage is 75.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #850      +/-   ##
==========================================
+ Coverage   63.41%   63.52%   +0.11%     
==========================================
  Files         250      250              
  Lines       15533    15533              
==========================================
+ Hits         9850     9868      +18     
+ Misses       5219     5197      -22     
- Partials      464      468       +4

Impacted Files	Coverage Δ
x/dex/contract/abci.go	`3.58% <0.00%> (ø)`
app/test_helpers.go	`46.98% <100.00%> (ø)`

... and 3 files with indirect coverage changes

testutil/processblock/verify/bank.go

+		for denom, changes := range expectedChanges {
+			expectedBalances[denom] = map[string]int64{}
+			for account, delta := range changes {
+				balance := app.BankKeeper.GetBalance(app.Ctx(), sdk.MustAccAddressFromBech32(account), denom)
+				expectedBalances[denom][account] = balance.Amount.Int64() + delta
+			}
+		}


testutil/processblock/verify/bank.go

+			for account, delta := range changes {
+				balance := app.BankKeeper.GetBalance(app.Ctx(), sdk.MustAccAddressFromBech32(account), denom)
+				expectedBalances[denom][account] = balance.Amount.Int64() + delta
+			}


testutil/processblock/verify/bank.go

+		for denom, expectedBalances := range expectedBalances {
+			for account, expectedBalance := range expectedBalances {
+				actualBalance := app.BankKeeper.GetBalance(app.Ctx(), sdk.MustAccAddressFromBech32(account), denom)
+				require.Equal(t, expectedBalance, actualBalance.Amount.Int64())
+			}
+		}


testutil/processblock/verify/bank.go

+			for account, expectedBalance := range expectedBalances {
+				actualBalance := app.BankKeeper.GetBalance(app.Ctx(), sdk.MustAccAddressFromBech32(account), denom)
+				require.Equal(t, expectedBalance, actualBalance.Amount.Int64())
+			}


testutil/processblock/verify/dex.go

+			for price, entry := range longBook {
+				actual, found := app.DexKeeper.GetLongOrderBookEntryByPrice(app.Ctx(), contract, price, priceDenom, assetDenom)
+				require.True(t, found)
+				require.Equal(t, *entry, *(actual.GetOrderEntry()))
+			}


testutil/processblock/verify/dex.go

+			for price, entry := range shortBook {
+				actual, found := app.DexKeeper.GetShortOrderBookEntryByPrice(app.Ctx(), contract, price, priceDenom, assetDenom)
+				require.True(t, found)
+				require.Equal(t, *entry, *(actual.GetOrderEntry()))
+			}


BrandonWeng · 2023-06-08T18:16:23Z

testutil/processblock/verify/bank.go

@@ -0,0 +1,84 @@
+package verify


Some things that might help with verification are the implicit actions that are preformed each block like:

epoch hooks (mint events)

fee/delegation distribution

slashing/jailing

Added verifiers and tests for epoch/mint/distribution. Slashing/jailing is a bit more complicated so I plan to add it in a separate PR

Sounds good thank you!

BrandonWeng · 2023-06-08T18:18:47Z

tests/dex_test.go

+	blockRunner := func() []uint32 { return app.RunBlock(block2) } // 2nd block to place order
+	blockRunner = verify.DexOrders(t, app, blockRunner, block2)
+	blockRunner = verify.Balance(t, app, blockRunner, block2)
+
+	require.Equal(t, []uint32{0}, blockRunner())


So when we need to apply verify methods we need to create a blockRunner variable?

yeah it's basically a wrapper around RunBlock but with verifier chained together:

verifierA_load_pre_execution_state verifierB_load_pre_execution_state RunBlock verifierB_check_post_execution_state verifierA_check_post_execution_state

testutil/processblock/genesismint.go

+)
+
+func (a *App) NewMinter(amount uint64) {
+	today := time.Now()


testutil/processblock/verify/distribution.go

+		for val, reward := range expectedOutstandingRewards {
+			require.True(t, reward.Equal(actualOutstandingRewards[val]))
+		}


yzang2019 · 2023-06-12T12:47:20Z

tests/template_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestTemplate(t *testing.T) {


Is there an easy way to modify the template so that it could support the best practice golang table driven testing norm such as:

type TestCase { description string input Input verifier func expectedResult Result } for _, scenario := range []TestCase{ { description: "test case1", input: Input{}, verifier: func(){}, expectedResult: []uint32{0,0}, }, { description: "test case2", input: Input{}, verifier: func(){}, expectedResult: []uint32{0,1}, }, } { t.Run(scenario.description, func(t *testing.T) { ... }) }

@yzang2019 it should be doable, but not sure if it'd be very readable given the amount of different setup needed for different tests. I'll give it a try

yzang2019 · 2023-06-12T12:52:59Z

tests/dex_test.go

+	"github.com/stretchr/testify/require"
+)
+
+func TestOrders(t *testing.T) {


can we have a more specific name for the Test function instead of calling TestOrders? Basically I would like to know what are we testing for orders? Which specific logic are we verifying here, let's break it down into smaller pieces, same thing applied to other modules

github-advanced-security bot found potential problems Jun 7, 2023

View reviewed changes

codchen force-pushed the tony-chen-testing-harness branch from 22e6ac3 to 17a2ac4 Compare June 8, 2023 08:57

codchen requested a review from philipsu522 June 8, 2023 09:08

codchen changed the title ~~Add testing harness for e2e go tests~~ Add testing harness for e2e application logic tests Jun 8, 2023

github-advanced-security bot found potential problems Jun 8, 2023

View reviewed changes

BrandonWeng reviewed Jun 8, 2023

View reviewed changes

codchen force-pushed the tony-chen-testing-harness branch from 17a2ac4 to 286eff4 Compare June 12, 2023 07:30

github-advanced-security bot found potential problems Jun 12, 2023

View reviewed changes

yzang2019 reviewed Jun 12, 2023

View reviewed changes

codchen force-pushed the tony-chen-testing-harness branch from 0f9f7aa to aca206d Compare June 13, 2023 08:49

yzang2019 approved these changes Jun 13, 2023

View reviewed changes

codchen added 6 commits June 13, 2023 23:33

Add testing harness for e2e go tests

4787c25

polish

c3a410d

add mint/epoch/distribution tests/helpers

c18f6f3

dex test

d8382a9

refactor

d5a6ad8

rebase

40ee107

codchen force-pushed the tony-chen-testing-harness branch from aca206d to 40ee107 Compare June 13, 2023 15:37

codchen merged commit 1315ce5 into master Jun 13, 2023

codchen deleted the tony-chen-testing-harness branch June 13, 2023 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add testing harness for e2e application logic tests #850

Add testing harness for e2e application logic tests #850

codchen commented Jun 7, 2023 •

edited

Loading

codecov bot commented Jun 8, 2023 •

edited

Loading

BrandonWeng Jun 8, 2023

codchen Jun 12, 2023

BrandonWeng Jun 12, 2023

BrandonWeng Jun 8, 2023

codchen Jun 9, 2023

yzang2019 Jun 12, 2023 •

edited

Loading

codchen Jun 12, 2023

yzang2019 Jun 12, 2023

Add testing harness for e2e application logic tests #850

Add testing harness for e2e application logic tests #850

Conversation

codchen commented Jun 7, 2023 • edited Loading

Describe your changes and provide context

Testing performed to validate your change

codecov bot commented Jun 8, 2023 • edited Loading

Codecov Report

BrandonWeng Jun 8, 2023

Choose a reason for hiding this comment

codchen Jun 12, 2023

Choose a reason for hiding this comment

BrandonWeng Jun 12, 2023

Choose a reason for hiding this comment

BrandonWeng Jun 8, 2023

Choose a reason for hiding this comment

codchen Jun 9, 2023

Choose a reason for hiding this comment

yzang2019 Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

codchen Jun 12, 2023

Choose a reason for hiding this comment

yzang2019 Jun 12, 2023

Choose a reason for hiding this comment

codchen commented Jun 7, 2023 •

edited

Loading

codecov bot commented Jun 8, 2023 •

edited

Loading

yzang2019 Jun 12, 2023 •

edited

Loading