Skip to content

Commit 7a57f35

Browse files
samlioksamliok
authored
Refactor Test Block BlockBuilder (#304)
* refactor block builder to expose explicit API * add panics to bb on edge cases * defer stop * hang
1 parent a266380 commit 7a57f35

11 files changed

+199
-234
lines changed

‎epoch_failover_test.go‎

Lines changed: 34 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,7 @@ import (
3131
// we expect the future empty notarization for round 2 to increment the round
3232
func TestEpochLeaderFailoverWithEmptyNotarization(t *testing.T) {
3333
nodes := []NodeID{{1}, {2}, {3}, {4}}
34-
bb := &testutil.TestBlockBuilder{
35-
Out: make(chan *testutil.TestBlock, 2),
36-
BlockShouldBeBuilt: make(chan struct{}, 1),
37-
In: make(chan *testutil.TestBlock, 2),
38-
}
34+
bb := testutil.NewTestBlockBuilder().WithBuiltBuffer(2)
3935
conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
4036

4137
e, err := NewEpoch(conf)
@@ -49,55 +45,45 @@ func TestEpochLeaderFailoverWithEmptyNotarization(t *testing.T) {
4945
// from earlier.
5046

5147
notarizeAndFinalizeRound(t, e, bb)
52-
5348
block0, _, err := storage.Retrieve(0)
5449
require.NoError(t, err)
5550

56-
block1, ok := bb.BuildBlock(context.Background(), ProtocolMetadata{
51+
block1 := testutil.NewTestBlock(ProtocolMetadata{
5752
Round: 1,
5853
Prev: block0.BlockHeader().Digest,
5954
Seq: 1,
6055
}, emptyBlacklist)
61-
require.True(t, ok)
6256

63-
block2, ok := bb.BuildBlock(context.Background(), ProtocolMetadata{
57+
block2 := testutil.NewTestBlock(ProtocolMetadata{
6458
Round: 3,
6559
Prev: block1.BlockHeader().Digest,
6660
Seq: 2,
6761
}, emptyBlacklist)
68-
require.True(t, ok)
6962

70-
// Artificially force the block builder to output the blocks we want.
71-
for len(bb.Out) > 0 {
72-
<-bb.Out
73-
}
74-
for _, block := range []VerifiedBlock{block1, block2} {
75-
bb.Out <- block.(*testutil.TestBlock)
76-
bb.In <- block.(*testutil.TestBlock)
77-
}
63+
notarizeAndFinalizeRound(t, e, bb)
7864

7965
emptyNotarization := testutil.NewEmptyNotarization(nodes[:3], 2)
8066
e.HandleMessage(&Message{
8167
EmptyNotarization: emptyNotarization,
8268
}, nodes[1])
8369

84-
notarizeAndFinalizeRound(t, e, bb)
70+
notarizeAndFinalizeRoundWithMetadata(t, e, bb, &block1.Metadata)
8571

8672
wal.AssertNotarization(2)
8773
nextBlockSeqToCommit := uint64(2)
8874
nextRoundToCommit := uint64(3)
8975

9076
runCrashAndRestartExecution(t, e, bb, wal, storage, func(t *testing.T, e *Epoch, bb *testutil.TestBlockBuilder, storage *testutil.InMemStorage, wal *testutil.TestWAL) {
9177
// Ensure our node proposes block with sequence 3 for round 4
92-
block, _ := notarizeAndFinalizeRound(t, e, bb)
78+
block, _ := notarizeAndFinalizeRoundWithMetadata(t, e, bb, &block2.Metadata)
9379
require.Equal(t, nextBlockSeqToCommit, block.BlockHeader().Seq)
9480
require.Equal(t, nextRoundToCommit, block.BlockHeader().Round)
9581
require.Equal(t, uint64(3), storage.NumBlocks())
9682
})
9783
}
9884

9985
func TestEpochRebroadcastsEmptyVoteAfterBlockProposalReceived(t *testing.T) {
100-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
86+
bb := testutil.NewTestBlockBuilder()
10187
nodes := []NodeID{{1}, {2}, {3}, {4}}
10288

10389
comm := newRebroadcastComm(nodes)
@@ -115,16 +101,15 @@ func TestEpochRebroadcastsEmptyVoteAfterBlockProposalReceived(t *testing.T) {
115101

116102
// receive the block proposal for round 0
117103
md := e.Metadata()
118-
_, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist)
104+
block, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist)
119105
require.True(t, ok)
120-
block := <-bb.Out
121106

122107
vote, err := testutil.NewTestVote(block, nodes[0])
123108
require.NoError(t, err)
124109
err = e.HandleMessage(&Message{
125110
BlockMessage: &BlockMessage{
126111
Vote: *vote,
127-
Block: block,
112+
Block: block.(*testutil.TestBlock),
128113
},
129114
}, nodes[0])
130115
require.NoError(t, err)
@@ -143,7 +128,7 @@ func TestEpochRebroadcastsEmptyVoteAfterBlockProposalReceived(t *testing.T) {
143128
}
144129

145130
func TestEpochLeaderFailoverReceivesEmptyVotesEarly(t *testing.T) {
146-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
131+
bb := testutil.NewTestBlockBuilder()
147132
nodes := []NodeID{{1}, {2}, {3}, {4}}
148133
quorum := Quorum(len(nodes))
149134

@@ -182,6 +167,8 @@ func TestEpochLeaderFailoverReceivesEmptyVotesEarly(t *testing.T) {
182167

183168
testutil.WaitForBlockProposerTimeout(t, e, &e.StartTime, e.Metadata().Round)
184169

170+
block := bb.GetBuiltBlock()
171+
185172
runCrashAndRestartExecution(t, e, bb, wal, storage, func(t *testing.T, e *Epoch, bb *testutil.TestBlockBuilder, storage *testutil.InMemStorage, wal *testutil.TestWAL) {
186173
walContent, err := wal.ReadAll()
187174
require.NoError(t, err)
@@ -203,8 +190,6 @@ func TestEpochLeaderFailoverReceivesEmptyVotesEarly(t *testing.T) {
203190
require.Equal(t, uint64(3), header.Seq)
204191

205192
// Ensure our node proposes block with sequence 3 for round 4
206-
block := <-bb.Out
207-
208193
for i := 1; i <= quorum; i++ {
209194
testutil.InjectTestFinalizeVote(t, e, block, nodes[i])
210195
}
@@ -220,7 +205,7 @@ func TestEpochLeaderFailoverReceivesEmptyVotesEarly(t *testing.T) {
220205

221206
func TestReceiveEmptyNotarizationWithNoQC(t *testing.T) {
222207
nodes := []NodeID{{1}, {2}, {3}, {4}}
223-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
208+
bb := testutil.NewTestBlockBuilder()
224209

225210
conf, _, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[1], testutil.NewNoopComm(nodes), bb)
226211

@@ -237,7 +222,7 @@ func TestReceiveEmptyNotarizationWithNoQC(t *testing.T) {
237222

238223
func TestEpochLeaderFailover(t *testing.T) {
239224
nodes := []NodeID{{1}, {2}, {3}, {4}}
240-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
225+
bb := testutil.NewTestBlockBuilder()
241226
conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
242227

243228
e, err := NewEpoch(conf)
@@ -302,7 +287,7 @@ func TestEpochLeaderFailover(t *testing.T) {
302287

303288
func TestEpochLeaderFailoverDoNotPersistEmptyRoundTwice(t *testing.T) {
304289
nodes := []NodeID{{1}, {2}, {3}, {4}}
305-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
290+
bb := testutil.NewTestBlockBuilder()
306291
conf, wal, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
307292
numRounds := uint64(2)
308293
e, err := NewEpoch(conf)
@@ -359,7 +344,7 @@ func TestEpochLeaderFailoverDoNotPersistEmptyRoundTwice(t *testing.T) {
359344

360345
func TestEpochLeaderRecursivelyFetchNotarizedBlocks(t *testing.T) {
361346
nodes := []NodeID{{1}, {2}, {3}, {4}}
362-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
347+
bb := testutil.NewTestBlockBuilder()
363348

364349
recordedMessages := make(chan *Message, 100)
365350

@@ -410,7 +395,7 @@ func TestEpochLeaderRecursivelyFetchNotarizedBlocks(t *testing.T) {
410395
func TestEpochLeaderFailoverInLeaderRound(t *testing.T) {
411396
nodes := []NodeID{{1}, {2}, {3}, {4}}
412397

413-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
398+
bb := testutil.NewTestBlockBuilder()
414399
recordedMessages := make(chan *Message, 100)
415400
comm := &recordingComm{Communication: testutil.NewNoopComm(nodes), BroadcastMessages: recordedMessages}
416401
conf, _, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[3], comm, bb)
@@ -444,7 +429,7 @@ func TestEpochLeaderFailoverInLeaderRound(t *testing.T) {
444429
}
445430
}
446431

447-
block := <-bb.Out
432+
block := bb.GetBuiltBlock()
448433
md := EmptyVoteMetadata{
449434
Round: block.Metadata.Round,
450435
}
@@ -465,7 +450,7 @@ func TestEpochLeaderFailoverInLeaderRound(t *testing.T) {
465450
}
466451

467452
func TestEpochNoFinalizationAfterEmptyVote(t *testing.T) {
468-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
453+
bb := testutil.NewTestBlockBuilder()
469454
nodes := []NodeID{{1}, {2}, {3}, {4}}
470455
quorum := Quorum(len(nodes))
471456

@@ -491,21 +476,19 @@ func TestEpochNoFinalizationAfterEmptyVote(t *testing.T) {
491476
require.NoError(t, err)
492477

493478
leader := LeaderForRound(nodes, 1)
494-
_, ok := bb.BuildBlock(context.Background(), ProtocolMetadata{
479+
block, ok := bb.BuildBlock(context.Background(), ProtocolMetadata{
495480
Prev: b.BlockHeader().Digest,
496481
Round: 1,
497482
Seq: 1,
498483
}, emptyBlacklist)
499484
require.True(t, ok)
500485

501-
block := <-bb.Out
502-
503486
vote, err := testutil.NewTestVote(block, leader)
504487
require.NoError(t, err)
505488
err = e.HandleMessage(&Message{
506489
BlockMessage: &BlockMessage{
507490
Vote: *vote,
508-
Block: block,
491+
Block: block.(*testutil.TestBlock),
509492
},
510493
}, leader)
511494
require.NoError(t, err)
@@ -535,7 +518,7 @@ func TestEpochNoFinalizationAfterEmptyVote(t *testing.T) {
535518
}
536519

537520
func TestEpochLeaderFailoverAfterProposal(t *testing.T) {
538-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
521+
bb := testutil.NewTestBlockBuilder()
539522
nodes := []NodeID{{1}, {2}, {3}, {4}}
540523

541524
conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
@@ -560,11 +543,10 @@ func TestEpochLeaderFailoverAfterProposal(t *testing.T) {
560543
leader := LeaderForRound(nodes, 3)
561544
md := e.Metadata()
562545
_, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist)
546+
block := bb.GetBuiltBlock()
563547
require.True(t, ok)
564548
require.Equal(t, md.Round, md.Seq)
565549

566-
block := <-bb.Out
567-
568550
vote, err := testutil.NewTestVote(block, leader)
569551
require.NoError(t, err)
570552
err = e.HandleMessage(&Message{
@@ -625,7 +607,7 @@ func TestEpochLeaderFailoverAfterProposal(t *testing.T) {
625607
}
626608

627609
func TestEpochLeaderFailoverTwice(t *testing.T) {
628-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
610+
bb := testutil.NewTestBlockBuilder()
629611
nodes := []NodeID{{1}, {2}, {3}, {4}}
630612

631613
conf, wal, storage := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
@@ -713,7 +695,7 @@ func TestEpochLeaderFailoverTwice(t *testing.T) {
713695
}
714696

715697
func TestEpochLeaderFailoverGarbageCollectedEmptyVotes(t *testing.T) {
716-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
698+
bb := testutil.NewTestBlockBuilder()
717699
nodes := []NodeID{{1}, {2}, {3}, {4}}
718700
conf, _, _ := testutil.DefaultTestNodeEpochConfig(t, nodes[0], testutil.NewNoopComm(nodes), bb)
719701

@@ -777,7 +759,7 @@ func TestEpochLeaderFailoverBecauseOfBadBlock(t *testing.T) {
777759
// This test ensures that if a block is proposed by a node, but it is invalid,
778760
// the node will immediately proceed to notarize the empty block.
779761

780-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
762+
bb := testutil.NewTestBlockBuilder()
781763
nodes := []NodeID{{1}, {2}, {3}, {4}}
782764

783765
recordedMessages := make(chan *Message, 100)
@@ -797,7 +779,7 @@ func TestEpochLeaderFailoverBecauseOfBadBlock(t *testing.T) {
797779

798780
_, ok := bb.BuildBlock(context.Background(), e.Metadata(), emptyBlacklist)
799781
require.True(t, ok)
800-
block := <-bb.Out
782+
block := bb.GetBuiltBlock()
801783
block.VerificationError = errors.New("invalid block")
802784

803785
vote, err := testutil.NewTestVote(block, nodes[1])
@@ -855,7 +837,7 @@ func TestEpochLeaderFailoverNotNeeded(t *testing.T) {
855837
return nil
856838
})
857839

858-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
840+
bb := testutil.NewTestBlockBuilder()
859841
nodes := []NodeID{{1}, {2}, {3}, {4}}
860842
quorum := Quorum(len(nodes))
861843

@@ -880,7 +862,7 @@ func TestEpochLeaderFailoverNotNeeded(t *testing.T) {
880862
_, ok := bb.BuildBlock(context.Background(), md, emptyBlacklist)
881863
require.True(t, ok)
882864

883-
block := <-bb.Out
865+
block := bb.GetBuiltBlock()
884866

885867
vote, err := testutil.NewTestVote(block, nodes[3])
886868
require.NoError(t, err)
@@ -907,7 +889,7 @@ func TestEpochLeaderFailoverNotNeeded(t *testing.T) {
907889

908890
func TestEpochBlacklist(t *testing.T) {
909891
blacklistedLeaderInLogs := make(chan struct{})
910-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 3)}
892+
bb := testutil.NewTestBlockBuilder().WithBlockShouldBeBuiltBuffer(3)
911893

912894
nodes := []NodeID{{1}, {2}, {3}, {4}}
913895

@@ -1024,7 +1006,7 @@ func TestEpochBlacklist(t *testing.T) {
10241006
// Now it's our turn to propose a new block.
10251007
bb.BlockShouldBeBuilt <- struct{}{}
10261008

1027-
block = <-bb.Out
1009+
block = bb.GetBuiltBlock()
10281010

10291011
// Inject specifically votes from the last two nodes, to ensure the blacklisted node will be redeemed
10301012
// the next time we will propose a block.
@@ -1063,7 +1045,7 @@ func TestEpochBlacklist(t *testing.T) {
10631045
// Now it's our turn to propose a new block.
10641046
bb.BlockShouldBeBuilt <- struct{}{}
10651047

1066-
block = <-bb.Out
1048+
block = bb.GetBuiltBlock()
10671049

10681050
require.Equal(t, Blacklist{
10691051
NodeCount: 4,
@@ -1142,7 +1124,7 @@ func (r *rebroadcastComm) Broadcast(msg *Message) {
11421124
}
11431125

11441126
func TestEpochRebroadcastsEmptyVote(t *testing.T) {
1145-
bb := &testutil.TestBlockBuilder{Out: make(chan *testutil.TestBlock, 1), BlockShouldBeBuilt: make(chan struct{}, 1)}
1127+
bb := testutil.NewTestBlockBuilder()
11461128
nodes := []NodeID{{1}, {2}, {3}, {4}}
11471129

11481130
comm := newRebroadcastComm(nodes)
@@ -1218,11 +1200,7 @@ func runCrashAndRestartExecution(t *testing.T, e *Epoch, bb *testutil.TestBlockB
12181200
nodes := e.Comm.Nodes()
12191201

12201202
// Clone the block builder
1221-
bbAfterCrash := &testutil.TestBlockBuilder{
1222-
Out: cloneBlockChan(bb.Out),
1223-
In: cloneBlockChan(bb.In),
1224-
BlockShouldBeBuilt: make(chan struct{}, cap(bb.BlockShouldBeBuilt)),
1225-
}
1203+
bbAfterCrash := testutil.NewTestBlockBuilder().WithBlockShouldBeBuiltBuffer(uint64(cap(bb.BlockShouldBeBuilt)))
12261204

12271205
// Case 1:
12281206
t.Run(fmt.Sprintf("%s-no-crash", t.Name()), func(t *testing.T) {
@@ -1243,23 +1221,6 @@ func runCrashAndRestartExecution(t *testing.T, e *Epoch, bb *testutil.TestBlockB
12431221
})
12441222
}
12451223

1246-
func cloneBlockChan(in chan *testutil.TestBlock) chan *testutil.TestBlock {
1247-
tmp := make(chan *testutil.TestBlock, cap(in))
1248-
out := make(chan *testutil.TestBlock, cap(in))
1249-
1250-
for len(in) > 0 {
1251-
block := <-in
1252-
tmp <- block
1253-
out <- block
1254-
}
1255-
1256-
for len(tmp) > 0 {
1257-
in <- <-tmp
1258-
}
1259-
1260-
return out
1261-
}
1262-
12631224
type recordingComm struct {
12641225
Communication
12651226
BroadcastMessages chan *Message

0 commit comments

Comments
 (0)