Skip to content

Commit c15fdee

Browse files
AryanGodaraAryanGodara
authored
Refactor: Extract out repitition of start-stop node pattern (#1544)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview I've extracted out the common pattern of 1. Initialize nodes 2. Start nodes 3. Check failures 4. Test cleanup / defer call to stop node and free resources into separate helper functions to remove redundancy and reduce scope of common error in these. <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [ ] Linked issues closed with keywords <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Tests** - Enhanced test suite for node operations with streamlined startup and cleanup processes. - Improved error handling and maintainability across various node-related test functions. - Introduced `NodeType` enum to clarify node types in tests. - Refactored numerous test functions to use centralized initialization and cleanup logic, improving test readability. - Adjusted logic in test functions for more accurate simulation of node behaviors and interactions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 65da2f4 commit c15fdee

6 files changed

Lines changed: 89 additions & 222 deletions

File tree

‎node/full_client_test.go‎

Lines changed: 22 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,7 @@ func TestGenesisChunked(t *testing.T) {
187187
assert.Error(err)
188188
assert.Nil(gc)
189189

190-
err = rpc.node.Start()
191-
require.NoError(t, err)
192-
defer func() {
193-
assert.NoError(rpc.node.Stop())
194-
}()
190+
startNodeWithCleanup(t, rpc.node)
195191
expectedID = 0
196192
gc2, err := rpc.GenesisChunked(context.Background(), expectedID)
197193
gotID := gc2.ChunkNumber
@@ -212,11 +208,7 @@ func TestBroadcastTxAsync(t *testing.T) {
212208
mockApp, rpc := getRPC(t)
213209
mockApp.On("CheckTx", mock.Anything, &abci.RequestCheckTx{Tx: expectedTx}).Return(&abci.ResponseCheckTx{}, nil)
214210

215-
err := rpc.node.Start()
216-
require.NoError(t, err)
217-
defer func() {
218-
assert.NoError(rpc.node.Stop())
219-
}()
211+
startNodeWithCleanup(t, rpc.node)
220212
res, err := rpc.BroadcastTxAsync(context.Background(), expectedTx)
221213
assert.NoError(err)
222214
assert.NotNil(res)
@@ -245,11 +237,7 @@ func TestBroadcastTxSync(t *testing.T) {
245237

246238
mockApp, rpc := getRPC(t)
247239

248-
err := rpc.node.Start()
249-
require.NoError(t, err)
250-
defer func() {
251-
assert.NoError(rpc.node.Stop())
252-
}()
240+
startNodeWithCleanup(t, rpc.node)
253241
mockApp.On("CheckTx", mock.Anything, &abci.RequestCheckTx{Tx: expectedTx}).Return(&expectedResponse, nil)
254242

255243
res, err := rpc.BroadcastTxSync(context.Background(), expectedTx)
@@ -329,14 +317,10 @@ func TestGetBlock(t *testing.T) {
329317
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
330318
mockApp.On("Commit", mock.Anything, mock.Anything).Return(&abci.ResponseCommit{}, nil)
331319

332-
err := rpc.node.Start()
333-
require.NoError(err)
334-
defer func() {
335-
assert.NoError(rpc.node.Stop())
336-
}()
320+
startNodeWithCleanup(t, rpc.node)
337321
ctx := context.Background()
338322
block := types.GetRandomBlock(1, 10)
339-
err = rpc.node.Store.SaveBlock(ctx, block, &types.Commit{})
323+
err := rpc.node.Store.SaveBlock(ctx, block, &types.Commit{})
340324
rpc.node.Store.SetHeight(ctx, block.Height())
341325
require.NoError(err)
342326

@@ -356,14 +340,10 @@ func TestGetCommit(t *testing.T) {
356340

357341
blocks := []*types.Block{types.GetRandomBlock(1, 5), types.GetRandomBlock(2, 6), types.GetRandomBlock(3, 8), types.GetRandomBlock(4, 10)}
358342

359-
err := rpc.node.Start()
360-
require.NoError(err)
361-
defer func() {
362-
assert.NoError(rpc.node.Stop())
363-
}()
343+
startNodeWithCleanup(t, rpc.node)
364344
ctx := context.Background()
365345
for _, b := range blocks {
366-
err = rpc.node.Store.SaveBlock(ctx, b, &types.Commit{})
346+
err := rpc.node.Store.SaveBlock(ctx, b, &types.Commit{})
367347
rpc.node.Store.SetHeight(ctx, b.Height())
368348
require.NoError(err)
369349
}
@@ -401,17 +381,12 @@ func TestCometBFTLightClientCompability(t *testing.T) {
401381

402382
blocks := []*types.Block{block1, block2, block3}
403383

404-
// start the node for testing
405-
err := rpc.node.Start()
406-
require.NoError(err)
407-
defer func() {
408-
assert.NoError(rpc.node.Stop())
409-
}()
384+
startNodeWithCleanup(t, rpc.node)
410385
ctx := context.Background()
411386

412387
// save the 3 blocks
413388
for _, b := range blocks {
414-
err = rpc.node.Store.SaveBlock(ctx, b, &b.SignedHeader.Commit)
389+
err := rpc.node.Store.SaveBlock(ctx, b, &b.SignedHeader.Commit)
415390
rpc.node.Store.SetHeight(ctx, b.Height())
416391
require.NoError(err)
417392
}
@@ -522,14 +497,10 @@ func TestGetBlockByHash(t *testing.T) {
522497
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
523498
mockApp.On("Commit", mock.Anything, mock.Anything).Return(&abci.ResponseCommit{}, nil)
524499

525-
err := rpc.node.Start()
526-
require.NoError(err)
527-
defer func() {
528-
assert.NoError(rpc.node.Stop())
529-
}()
500+
startNodeWithCleanup(t, rpc.node)
530501
ctx := context.Background()
531502
block := types.GetRandomBlock(1, 10)
532-
err = rpc.node.Store.SaveBlock(ctx, block, &types.Commit{})
503+
err := rpc.node.Store.SaveBlock(ctx, block, &types.Commit{})
533504
require.NoError(err)
534505
abciBlock, err := abciconv.ToABCIBlock(block)
535506
require.NoError(err)
@@ -596,11 +567,7 @@ func TestTx(t *testing.T) {
596567
mockApp.On("CheckTx", mock.Anything, mock.Anything).Return(&abci.ResponseCheckTx{}, nil)
597568
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
598569

599-
err = rpc.node.Start()
600-
require.NoError(err)
601-
defer func() {
602-
assert.NoError(rpc.node.Stop())
603-
}()
570+
startNodeWithCleanup(t, rpc.node)
604571
tx1 := cmtypes.Tx("tx1")
605572
res, err := rpc.BroadcastTxSync(ctx, tx1)
606573
assert.NoError(err)
@@ -641,17 +608,12 @@ func TestUnconfirmedTxs(t *testing.T) {
641608
for _, c := range cases {
642609
t.Run(c.name, func(t *testing.T) {
643610
assert := assert.New(t)
644-
require := require.New(t)
645611

646612
mockApp, rpc := getRPC(t)
647613
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
648614
mockApp.On("CheckTx", mock.Anything, mock.Anything).Return(&abci.ResponseCheckTx{}, nil)
649615

650-
err := rpc.node.Start()
651-
require.NoError(err)
652-
defer func() {
653-
assert.NoError(rpc.node.Stop())
654-
}()
616+
startNodeWithCleanup(t, rpc.node)
655617

656618
for _, tx := range c.txs {
657619
res, err := rpc.BroadcastTxAsync(context.Background(), tx)
@@ -680,17 +642,12 @@ func TestUnconfirmedTxs(t *testing.T) {
680642

681643
func TestUnconfirmedTxsLimit(t *testing.T) {
682644
assert := assert.New(t)
683-
require := require.New(t)
684645

685646
mockApp, rpc := getRPC(t)
686647
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
687648
mockApp.On("CheckTx", mock.Anything, mock.Anything).Return(&abci.ResponseCheckTx{}, nil)
688649

689-
err := rpc.node.Start()
690-
require.NoError(err)
691-
defer func() {
692-
assert.NoError(rpc.node.Stop())
693-
}()
650+
startNodeWithCleanup(t, rpc.node)
694651

695652
tx1 := cmtypes.Tx("tx1")
696653
tx2 := cmtypes.Tx("another tx")
@@ -872,19 +829,12 @@ func TestMempool2Nodes(t *testing.T) {
872829
require.NoError(err)
873830
require.NotNil(node2)
874831

875-
err = node1.Start()
876-
require.NoError(err)
832+
startNodeWithCleanup(t, node1)
877833
require.NoError(waitForFirstBlock(node1, Store))
878834

879-
defer func() {
880-
assert.NoError(node1.Stop())
881-
}()
882-
err = node2.Start()
883-
require.NoError(err)
884-
defer func() {
885-
assert.NoError(node2.Stop())
886-
}()
835+
startNodeWithCleanup(t, node2)
887836
require.NoError(waitForAtLeastNBlocks(node2, 1, Store))
837+
888838
timeoutCtx, timeoutCancel := context.WithTimeout(context.Background(), 3*time.Second)
889839
defer timeoutCancel()
890840

@@ -969,6 +919,8 @@ func TestStatus(t *testing.T) {
969919
rpc.node.Store.SetHeight(ctx, latestBlock.Height())
970920
require.NoError(err)
971921

922+
startNodeWithCleanup(t, rpc.node)
923+
972924
resp, err := rpc.Status(context.Background())
973925
assert.NoError(err)
974926

@@ -1077,31 +1029,21 @@ func TestFutureGenesisTime(t *testing.T) {
10771029
require.NoError(err)
10781030
require.NotNil(node)
10791031

1080-
err = node.Start()
1081-
require.NoError(err)
1082-
1083-
defer func() {
1084-
assert.NoError(node.Stop())
1085-
}()
1032+
startNodeWithCleanup(t, node)
10861033
wg.Wait()
10871034

10881035
assert.True(beginBlockTime.After(genesisTime))
10891036
}
10901037

10911038
func TestHealth(t *testing.T) {
10921039
assert := assert.New(t)
1093-
require := require.New(t)
10941040

10951041
mockApp, rpc := getRPC(t)
10961042
mockApp.On("FinalizeBlock", mock.Anything, mock.Anything).Return(finalizeBlockResponse)
10971043
mockApp.On("CheckTx", mock.Anything, mock.Anything).Return(abci.ResponseCheckTx{}, nil)
10981044
mockApp.On("Commit", mock.Anything).Return(abci.ResponseCommit{}, nil)
10991045

1100-
err := rpc.node.Start()
1101-
require.NoError(err)
1102-
defer func() {
1103-
assert.NoError(rpc.node.Stop())
1104-
}()
1046+
startNodeWithCleanup(t, rpc.node)
11051047

11061048
resultHealth, err := rpc.Health(context.Background())
11071049
assert.Nil(err)
@@ -1117,11 +1059,7 @@ func TestNetInfo(t *testing.T) {
11171059
mockApp.On("CheckTx", mock.Anything, mock.Anything).Return(&abci.ResponseCheckTx{}, nil)
11181060
mockApp.On("Commit", mock.Anything, mock.Anything).Return(&abci.ResponseCommit{}, nil)
11191061

1120-
err := rpc.node.Start()
1121-
require.NoError(err)
1122-
defer func() {
1123-
assert.NoError(rpc.node.Stop())
1124-
}()
1062+
startNodeWithCleanup(t, rpc.node)
11251063

11261064
netInfo, err := rpc.NetInfo(context.Background())
11271065
require.NoError(err)

0 commit comments

Comments
 (0)