Skip to content

Commit d70c4fa

Browse files
janoszelig
janos
authored and
zelig
committed
swarm: Fix T.Fatal inside a goroutine in tests (#18409)
* swarm/storage: fix T.Fatal inside a goroutine * swarm/network/simulation: fix T.Fatal inside a goroutine * swarm/network/stream: fix T.Fatal inside a goroutine * swarm/network/simulation: consistent failures in TestPeerEventsTimeout * swarm/network/simulation: rename sendRunSignal to triggerSimulationRun
1 parent 81f04fa commit d70c4fa

File tree

7 files changed

+167
-68
lines changed

7 files changed

+167
-68
lines changed

‎swarm/network/simulation/events_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ func TestPeerEventsTimeout(t *testing.T) {
8181
events := sim.PeerEvents(ctx, sim.NodeIDs())
8282

8383
done := make(chan struct{})
84+
errC := make(chan error)
8485
go func() {
8586
for e := range events {
8687
if e.Error == context.Canceled {
@@ -90,14 +91,16 @@ func TestPeerEventsTimeout(t *testing.T) {
9091
close(done)
9192
return
9293
} else {
93-
t.Fatal(e.Error)
94+
errC <- e.Error
9495
}
9596
}
9697
}()
9798

9899
select {
99100
case <-time.After(time.Second):
100-
t.Error("no context deadline received")
101+
t.Fatal("no context deadline received")
102+
case err := <-errC:
103+
t.Fatal(err)
101104
case <-done:
102105
// all good, context deadline detected
103106
}

‎swarm/network/simulation/http_test.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ func TestSimulationWithHTTPServer(t *testing.T) {
7373
//this time the timeout should be long enough so that it doesn't kick in too early
7474
ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second)
7575
defer cancel2()
76-
go sendRunSignal(t)
76+
errC := make(chan error, 1)
77+
go triggerSimulationRun(t, errC)
7778
result = sim.Run(ctx, func(ctx context.Context, sim *Simulation) error {
7879
log.Debug("This run waits for the run signal from `frontend`...")
7980
//ensure with a Sleep that simulation doesn't terminate before the signal is received
@@ -83,27 +84,27 @@ func TestSimulationWithHTTPServer(t *testing.T) {
8384
if result.Error != nil {
8485
t.Fatal(result.Error)
8586
}
87+
if err := <-errC; err != nil {
88+
t.Fatal(err)
89+
}
8690
log.Debug("Test terminated successfully")
8791
}
8892

89-
func sendRunSignal(t *testing.T) {
93+
func triggerSimulationRun(t *testing.T, errC chan error) {
9094
//We need to first wait for the sim HTTP server to start running...
9195
time.Sleep(2 * time.Second)
9296
//then we can send the signal
9397

9498
log.Debug("Sending run signal to simulation: POST /runsim...")
9599
resp, err := http.Post(fmt.Sprintf("http://localhost%s/runsim", DefaultHTTPSimAddr), "application/json", nil)
96100
if err != nil {
97-
t.Fatalf("Request failed: %v", err)
101+
errC <- fmt.Errorf("Request failed: %v", err)
102+
return
98103
}
99-
defer func() {
100-
err := resp.Body.Close()
101-
if err != nil {
102-
log.Error("Error closing response body", "err", err)
103-
}
104-
}()
105104
log.Debug("Signal sent")
106105
if resp.StatusCode != http.StatusOK {
107-
t.Fatalf("err %s", resp.Status)
106+
errC <- fmt.Errorf("err %s", resp.Status)
107+
return
108108
}
109+
errC <- resp.Body.Close()
109110
}

‎swarm/network/stream/delivery_test.go‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ package stream
1919
import (
2020
"bytes"
2121
"context"
22+
"errors"
2223
"fmt"
2324
"os"
2425
"sync"
26+
"sync/atomic"
2527
"testing"
2628
"time"
2729

@@ -500,7 +502,7 @@ func testDeliveryFromNodes(t *testing.T, nodes, chunkCount int, skipCheck bool)
500502

501503
log.Info("Starting simulation")
502504
ctx := context.Background()
503-
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error {
505+
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) (err error) {
504506
nodeIDs := sim.UpNodeIDs()
505507
//determine the pivot node to be the first node of the simulation
506508
pivot := nodeIDs[0]
@@ -553,14 +555,13 @@ func testDeliveryFromNodes(t *testing.T, nodes, chunkCount int, skipCheck bool)
553555
}
554556
pivotFileStore := item.(*storage.FileStore)
555557
log.Debug("Starting retrieval routine")
558+
retErrC := make(chan error)
556559
go func() {
557560
// start the retrieval on the pivot node - this will spawn retrieve requests for missing chunks
558561
// we must wait for the peer connections to have started before requesting
559562
n, err := readAll(pivotFileStore, fileHash)
560563
log.Info(fmt.Sprintf("retrieved %v", fileHash), "read", n, "err", err)
561-
if err != nil {
562-
t.Fatalf("requesting chunks action error: %v", err)
563-
}
564+
retErrC <- err
564565
}()
565566

566567
log.Debug("Watching for disconnections")
@@ -570,11 +571,19 @@ func testDeliveryFromNodes(t *testing.T, nodes, chunkCount int, skipCheck bool)
570571
simulation.NewPeerEventsFilter().Drop(),
571572
)
572573

574+
var disconnected atomic.Value
573575
go func() {
574576
for d := range disconnections {
575577
if d.Error != nil {
576578
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
577-
t.Fatal(d.Error)
579+
disconnected.Store(true)
580+
}
581+
}
582+
}()
583+
defer func() {
584+
if err != nil {
585+
if yes, ok := disconnected.Load().(bool); ok && yes {
586+
err = errors.New("disconnect events received")
578587
}
579588
}
580589
}()
@@ -595,6 +604,9 @@ func testDeliveryFromNodes(t *testing.T, nodes, chunkCount int, skipCheck bool)
595604
if !success {
596605
return fmt.Errorf("Test failed, chunks not available on all nodes")
597606
}
607+
if err := <-retErrC; err != nil {
608+
t.Fatalf("requesting chunks: %v", err)
609+
}
598610
log.Debug("Test terminated successfully")
599611
return nil
600612
})
@@ -675,7 +687,7 @@ func benchmarkDeliveryFromNodes(b *testing.B, nodes, chunkCount int, skipCheck b
675687
}
676688

677689
ctx := context.Background()
678-
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error {
690+
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) (err error) {
679691
nodeIDs := sim.UpNodeIDs()
680692
node := nodeIDs[len(nodeIDs)-1]
681693

@@ -702,11 +714,19 @@ func benchmarkDeliveryFromNodes(b *testing.B, nodes, chunkCount int, skipCheck b
702714
simulation.NewPeerEventsFilter().Drop(),
703715
)
704716

717+
var disconnected atomic.Value
705718
go func() {
706719
for d := range disconnections {
707720
if d.Error != nil {
708721
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
709-
b.Fatal(d.Error)
722+
disconnected.Store(true)
723+
}
724+
}
725+
}()
726+
defer func() {
727+
if err != nil {
728+
if yes, ok := disconnected.Load().(bool); ok && yes {
729+
err = errors.New("disconnect events received")
710730
}
711731
}
712732
}()

‎swarm/network/stream/intervals_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ package stream
1919
import (
2020
"context"
2121
"encoding/binary"
22+
"errors"
2223
"fmt"
2324
"os"
2425
"sync"
26+
"sync/atomic"
2527
"testing"
2628
"time"
2729

@@ -117,7 +119,7 @@ func testIntervals(t *testing.T, live bool, history *Range, skipCheck bool) {
117119
t.Fatal(err)
118120
}
119121

120-
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error {
122+
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) (err error) {
121123
nodeIDs := sim.UpNodeIDs()
122124
storer := nodeIDs[0]
123125
checker := nodeIDs[1]
@@ -162,11 +164,19 @@ func testIntervals(t *testing.T, live bool, history *Range, skipCheck bool) {
162164
return err
163165
}
164166

167+
var disconnected atomic.Value
165168
go func() {
166169
for d := range disconnections {
167170
if d.Error != nil {
168171
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
169-
t.Fatal(d.Error)
172+
disconnected.Store(true)
173+
}
174+
}
175+
}()
176+
defer func() {
177+
if err != nil {
178+
if yes, ok := disconnected.Load().(bool); ok && yes {
179+
err = errors.New("disconnect events received")
170180
}
171181
}
172182
}()

‎swarm/network/stream/snapshot_sync_test.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"os"
2222
"runtime"
2323
"sync"
24+
"sync/atomic"
2425
"testing"
2526
"time"
2627

@@ -213,11 +214,13 @@ func testSyncingViaGlobalSync(t *testing.T, chunkCount int, nodeCount int) {
213214
simulation.NewPeerEventsFilter().Drop(),
214215
)
215216

217+
var disconnected atomic.Value
216218
go func() {
217219
for d := range disconnections {
218-
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
219-
t.Fatal("unexpected disconnect")
220-
cancelSimRun()
220+
if d.Error != nil {
221+
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
222+
disconnected.Store(true)
223+
}
221224
}
222225
}()
223226

@@ -226,6 +229,9 @@ func testSyncingViaGlobalSync(t *testing.T, chunkCount int, nodeCount int) {
226229
if result.Error != nil {
227230
t.Fatal(result.Error)
228231
}
232+
if yes, ok := disconnected.Load().(bool); ok && yes {
233+
t.Fatal("disconnect events received")
234+
}
229235
log.Info("Simulation ended")
230236
}
231237

@@ -395,11 +401,13 @@ func testSyncingViaDirectSubscribe(t *testing.T, chunkCount int, nodeCount int)
395401
simulation.NewPeerEventsFilter().Drop(),
396402
)
397403

404+
var disconnected atomic.Value
398405
go func() {
399406
for d := range disconnections {
400-
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
401-
t.Fatal("unexpected disconnect")
402-
cancelSimRun()
407+
if d.Error != nil {
408+
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
409+
disconnected.Store(true)
410+
}
403411
}
404412
}()
405413

@@ -514,6 +522,9 @@ func testSyncingViaDirectSubscribe(t *testing.T, chunkCount int, nodeCount int)
514522
return result.Error
515523
}
516524

525+
if yes, ok := disconnected.Load().(bool); ok && yes {
526+
t.Fatal("disconnect events received")
527+
}
517528
log.Info("Simulation ended")
518529
return nil
519530
}

‎swarm/network/stream/syncer_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ package stream
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"io/ioutil"
2324
"math"
2425
"os"
2526
"sync"
27+
"sync/atomic"
2628
"testing"
2729
"time"
2830

@@ -129,7 +131,7 @@ func testSyncBetweenNodes(t *testing.T, nodes, chunkCount int, skipCheck bool, p
129131
if err != nil {
130132
t.Fatal(err)
131133
}
132-
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) error {
134+
result := sim.Run(ctx, func(ctx context.Context, sim *simulation.Simulation) (err error) {
133135
nodeIDs := sim.UpNodeIDs()
134136

135137
nodeIndex := make(map[enode.ID]int)
@@ -143,11 +145,19 @@ func testSyncBetweenNodes(t *testing.T, nodes, chunkCount int, skipCheck bool, p
143145
simulation.NewPeerEventsFilter().Drop(),
144146
)
145147

148+
var disconnected atomic.Value
146149
go func() {
147150
for d := range disconnections {
148151
if d.Error != nil {
149152
log.Error("peer drop", "node", d.NodeID, "peer", d.PeerID)
150-
t.Fatal(d.Error)
153+
disconnected.Store(true)
154+
}
155+
}
156+
}()
157+
defer func() {
158+
if err != nil {
159+
if yes, ok := disconnected.Load().(bool); ok && yes {
160+
err = errors.New("disconnect events received")
151161
}
152162
}
153163
}()

0 commit comments

Comments
 (0)