From 4877c73c2e9ca22f325e4f0ed7cb53843f39f80f Mon Sep 17 00:00:00 2001 From: Wondertan Date: Tue, 24 May 2022 16:33:52 +0200 Subject: [PATCH 1/3] tests(core): rewrite tests in core pkg to remove some of the falkeyness --- core/client_test.go | 23 +++++++++++++---------- core/fetcher_test.go | 43 +++++++++++++++++++++++++++++-------------- core/testing.go | 3 ++- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/core/client_test.go b/core/client_test.go index 55e3e42d37..69bc67a527 100644 --- a/core/client_test.go +++ b/core/client_test.go @@ -3,38 +3,41 @@ package core import ( "context" "testing" + "time" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/types" ) func TestRemoteClient_Status(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) t.Cleanup(cancel) _, client := StartTestClient(ctx, t) - status, err := client.Status(ctx) require.NoError(t, err) require.NotNil(t, status) } func TestRemoteClient_StartBlockSubscription_And_GetBlock(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) t.Cleanup(cancel) _, client := StartTestClient(ctx, t) - eventChan, err := client.Subscribe(ctx, newBlockSubscriber, newBlockEventQuery) require.NoError(t, err) for i := 1; i <= 3; i++ { - <-eventChan - // check that `Block` works as intended (passing nil to get block at latest height) - block, err := client.Block(ctx, nil) - require.NoError(t, err) - require.Equal(t, int64(i), block.Block.Height) + select { + case evt := <-eventChan: + h := evt.Data.(types.EventDataNewBlock).Block.Height + block, err := client.Block(ctx, &h) + require.NoError(t, err) + require.GreaterOrEqual(t, block.Block.Height, int64(i)) + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + } } - // unsubscribe to event channel require.NoError(t, client.Unsubscribe(ctx, newBlockSubscriber, newBlockEventQuery)) } diff --git a/core/fetcher_test.go b/core/fetcher_test.go index 837537c048..decb84be3a 100644 --- a/core/fetcher_test.go +++ b/core/fetcher_test.go @@ -3,15 +3,17 @@ package core import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/types" "github.com/tendermint/tendermint/libs/bytes" ) func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) t.Cleanup(cancel) _, client := StartTestClient(ctx, t) @@ -22,21 +24,24 @@ func TestBlockFetcher_GetBlock_and_SubscribeNewBlockEvent(t *testing.T) { require.NoError(t, err) for i := 1; i < 3; i++ { - newBlockFromChan := <-newBlockChan - - block, err := fetcher.GetBlock(ctx, nil) - require.NoError(t, err) - - assert.Equal(t, newBlockFromChan, block) + select { + case newBlockFromChan := <-newBlockChan: + h := newBlockFromChan.Height + block, err := fetcher.GetBlock(ctx, &h) + require.NoError(t, err) + assert.Equal(t, newBlockFromChan, block) + require.GreaterOrEqual(t, block.Height, int64(i)) + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + } } - require.NoError(t, fetcher.UnsubscribeNewBlockEvent(ctx)) } // TestBlockFetcherHeaderValues tests that both the Commit and ValidatorSet // endpoints are working as intended. func TestBlockFetcherHeaderValues(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) t.Cleanup(cancel) _, client := StartTestClient(ctx, t) @@ -46,15 +51,26 @@ func TestBlockFetcherHeaderValues(t *testing.T) { newBlockChan, err := fetcher.SubscribeNewBlockEvent(ctx) require.NoError(t, err) // read once from channel to generate next block - <-newBlockChan + var h int64 + select { + case evt := <-newBlockChan: + h = evt.Header.Height + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + } // get Commit from current height - commit, err := fetcher.Commit(ctx, nil) + commit, err := fetcher.Commit(ctx, &h) require.NoError(t, err) // get ValidatorSet from current height - valSet, err := fetcher.ValidatorSet(ctx, nil) + valSet, err := fetcher.ValidatorSet(ctx, &h) require.NoError(t, err) // get next block - nextBlock := <-newBlockChan + var nextBlock *types.Block + select { + case nextBlock = <-newBlockChan: + case <-ctx.Done(): + require.NoError(t, ctx.Err()) + } // compare LastCommit from next block to Commit from first block height assert.Equal(t, nextBlock.LastCommit.Hash(), commit.Hash()) assert.Equal(t, nextBlock.LastCommit.Height, commit.Height) @@ -64,6 +80,5 @@ func TestBlockFetcherHeaderValues(t *testing.T) { err = hexBytes.Unmarshal(valSet.Hash()) require.NoError(t, err) assert.Equal(t, nextBlock.ValidatorsHash, hexBytes) - require.NoError(t, fetcher.UnsubscribeNewBlockEvent(ctx)) } diff --git a/core/testing.go b/core/testing.go index 8524a8caca..b74a597cf5 100644 --- a/core/testing.go +++ b/core/testing.go @@ -18,7 +18,8 @@ import ( tmtypes "github.com/tendermint/tendermint/types" ) -const defaultRetainBlocks int64 = 50 +// so that we never hit an issue where we request blocks that are removed +const defaultRetainBlocks int64 = 10000 // StartTestNode starts a mock Core node background process and returns it. func StartTestNode(ctx context.Context, t *testing.T, app types.Application, cfg *config.Config) tmservice.Service { From bc56d54c7656b38f074246d2cc1529dfae6d7756 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Tue, 24 May 2022 20:14:44 +0200 Subject: [PATCH 2/3] tests(header/store): fix flaky heightsub test --- header/store/heightsub_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/header/store/heightsub_test.go b/header/store/heightsub_test.go index 5305e45999..5be45d17ef 100644 --- a/header/store/heightsub_test.go +++ b/header/store/heightsub_test.go @@ -31,10 +31,13 @@ func TestHeightSub(t *testing.T) { // assert actual subscription works { go func() { + // fixes flakiness on CI + time.Sleep(time.Millisecond) + h1 := header.RandExtendedHeader(t) h1.Height = 101 h2 := header.RandExtendedHeader(t) - h1.Height = 101 + h2.Height = 102 hs.Pub(h1, h2) }() From 07114e4615f1eb6919855ce7e1edbc0f74a91276 Mon Sep 17 00:00:00 2001 From: Wondertan Date: Wed, 25 May 2022 11:05:03 +0200 Subject: [PATCH 3/3] tests(header/core): instanitiate PubSubs before connecting peers in TestListener to deflake tests --- header/core/listener_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/header/core/listener_test.go b/header/core/listener_test.go index 7d987e58ec..2ac9365f6f 100644 --- a/header/core/listener_test.go +++ b/header/core/listener_test.go @@ -58,6 +58,15 @@ func createMocknetWithTwoPubsubEndpoints(ctx context.Context, t *testing.T) (*pu require.NoError(t, err) host0, host1 := net.Hosts()[0], net.Hosts()[1] + // create pubsub for host + ps0, err := pubsub.NewGossipSub(context.Background(), host0, + pubsub.WithMessageSignaturePolicy(pubsub.StrictNoSign)) + require.NoError(t, err) + // create pubsub for peer-side (to test broadcast comes through network) + ps1, err := pubsub.NewGossipSub(context.Background(), host1, + pubsub.WithMessageSignaturePolicy(pubsub.StrictNoSign)) + require.NoError(t, err) + sub0, err := host0.EventBus().Subscribe(&event.EvtPeerIdentificationCompleted{}) require.NoError(t, err) sub1, err := host1.EventBus().Subscribe(&event.EvtPeerIdentificationCompleted{}) @@ -76,14 +85,6 @@ func createMocknetWithTwoPubsubEndpoints(ctx context.Context, t *testing.T) (*pu } } - // create pubsub for host - ps0, err := pubsub.NewGossipSub(context.Background(), host0, - pubsub.WithMessageSignaturePolicy(pubsub.StrictNoSign)) - require.NoError(t, err) - // create pubsub for peer-side (to test broadcast comes through network) - ps1, err := pubsub.NewGossipSub(context.Background(), host1, - pubsub.WithMessageSignaturePolicy(pubsub.StrictNoSign)) - require.NoError(t, err) return ps0, ps1 }