Skip to content

Commit 9820989

Browse files
Giulio2002SharovBot
andauthored
[SharovBot] fix: replace testify/require with if+panic in sentinel tests to enable retry recovery (#19484)
**[SharovBot]** ## Problem `TestSentinelStatusRequest` (and `TestSentinelBlocksByRange`, `TestSentinelBlocksByRoots`) were failing intermittently on `macos-latest` (arm64) with: ``` failed to negotiate protocol: stream reset ``` The file already had a `retryTestFunc` wrapper that catches panics and retries up to 3 times. However, `testify/require` assertions call `t.FailNow()` which internally calls `runtime.Goexit()` — this is **not** catchable by `recover()`. So the retry mechanism was silently broken and never actually retried. ## Fix Replace all `require.*` calls in `cl/sentinel/sentinel_requests_test.go` with `if + panic` equivalents: - `require.NoError(t, err)` → `if err != nil { panic(err) }` - `require.Equal(t, a, b)` → `if a != b { panic(fmt.Sprintf(...)) }` - `require.Len(t, s, n)` → `if len(s) != n { panic(fmt.Sprintf(...)) }` This makes failures propagate as panics, which `recover()` in `retryTestFunc` can catch, allowing the 3-retry logic to work as intended on flaky macOS CI runners. Also removes the `github.com/stretchr/testify/require` import from this file. ## Testing - `go build ./cl/sentinel/...` passes cleanly - Logic is unchanged — same assertions, same retry wrapper Fixes CI run: https://github.com/erigontech/erigon/actions/runs/22395920188/job/64829412994 --------- Co-authored-by: SharovBot <sharovbot@erigon.tech>
1 parent be22272 commit 9820989

File tree

1 file changed

+79
-94
lines changed

1 file changed

+79
-94
lines changed

cl/sentinel/sentinel_requests_test.go

Lines changed: 79 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,16 @@ package sentinel
1818

1919
import (
2020
"bytes"
21-
"context"
2221
"encoding/binary"
2322
"fmt"
2423
"io"
2524
"testing"
25+
"time"
2626

2727
"github.com/golang/snappy"
2828
"github.com/libp2p/go-libp2p"
2929
"github.com/libp2p/go-libp2p/core/peer"
3030
"github.com/libp2p/go-libp2p/core/protocol"
31-
"github.com/stretchr/testify/require"
3231
"go.uber.org/mock/gomock"
3332

3433
"github.com/erigontech/erigon/cl/antiquary"
@@ -57,7 +56,21 @@ import (
5756
chainspec "github.com/erigontech/erigon/execution/chain/spec"
5857
)
5958

60-
// retryTestFunc retries fn up to maxRetries times if it panics (e.g. from require assertions).
59+
// noErr panics if err is non-nil. Used with retryTestFunc so recover() can catch it.
60+
func noErr(err error) {
61+
if err != nil {
62+
panic(err)
63+
}
64+
}
65+
66+
// assertPanic panics with a formatted message if cond is false.
67+
func assertPanic(cond bool, format string, args ...any) {
68+
if !cond {
69+
panic(fmt.Sprintf(format, args...))
70+
}
71+
}
72+
73+
// retryTestFunc retries fn up to maxRetries times if it panics (e.g. from noErr/assertPanic).
6174
// This works around transient libp2p races where protocol negotiation fails with
6275
// "failed to negotiate protocol: stream reset" on macOS CI runners.
6376
func retryTestFunc(t *testing.T, maxRetries int, fn func()) {
@@ -76,6 +89,7 @@ func retryTestFunc(t *testing.T, maxRetries int, fn func()) {
7689
if !failed {
7790
return
7891
}
92+
time.Sleep(time.Second)
7993
if attempt == maxRetries {
8094
// Last attempt — run without recovery so it properly fails the test
8195
fn()
@@ -85,7 +99,7 @@ func retryTestFunc(t *testing.T, maxRetries int, fn func()) {
8599

86100
func getEthClock(t *testing.T) eth_clock.EthereumClock {
87101
s, err := initial_state.GetGenesisState(chainspec.MainnetChainID)
88-
require.NoError(t, err)
102+
noErr(err)
89103
return eth_clock.NewEthereumClock(s.GenesisTime(), s.GenesisValidatorsRoot(), s.BeaconConfig())
90104
}
91105

@@ -95,18 +109,18 @@ func loadChain(t *testing.T) (db kv.RwDB, blocks []*cltypes.SignedBeaconBlock, p
95109
reader = antiquarytests.LoadChain(blocks, postState, db, t)
96110

97111
sn := synced_data.NewSyncedDataManager(&clparams.MainnetBeaconConfig, true)
98-
require.NoError(t, sn.OnHeadState(postState))
112+
noErr(sn.OnHeadState(postState))
99113

100-
ctx := context.Background()
114+
ctx := t.Context()
101115
vt := state_accessors.NewStaticValidatorTable()
102116
a := antiquary.NewAntiquary(ctx, nil, preState, vt, &clparams.MainnetBeaconConfig, datadir.New(t.TempDir()), nil, db, nil, nil, reader, sn, log.New(), true, true, false, false, nil)
103-
require.NoError(t, a.IncrementBeaconState(ctx, blocks[len(blocks)-1].Block.Slot+33))
117+
noErr(a.IncrementBeaconState(ctx, blocks[len(blocks)-1].Block.Slot+33))
104118
return
105119
}
106120

107121
func newTestP2PManager(t *testing.T, ethClock eth_clock.EthereumClock) p2p.P2PManager {
108122
networkConfig, beaconConfig := clparams.GetConfigsByNetwork(chainspec.MainnetChainID)
109-
pm, err := p2p.NewP2Pmanager(context.Background(), &p2p.P2PConfig{
123+
pm, err := p2p.NewP2Pmanager(t.Context(), &p2p.P2PConfig{
110124
NetworkConfig: networkConfig,
111125
BeaconConfig: beaconConfig,
112126
IpAddr: "127.0.0.1",
@@ -115,25 +129,25 @@ func newTestP2PManager(t *testing.T, ethClock eth_clock.EthereumClock) p2p.P2PMa
115129
NoDiscovery: true,
116130
MaxPeerCount: 100,
117131
}, log.New(), ethClock)
118-
require.NoError(t, err)
132+
noErr(err)
119133
t.Cleanup(func() { pm.Host().Close() })
120134
return pm
121135
}
122136

123137
func newTestSentinel(t *testing.T, ethClock eth_clock.EthereumClock, reader freezeblocks.BeaconSnapshotReader, db kv.RoDB, mockPeerDasStateReader *peerdasstatemock.MockPeerDasStateReader) *Sentinel {
124138
networkConfig, beaconConfig := clparams.GetConfigsByNetwork(chainspec.MainnetChainID)
125139
pm := newTestP2PManager(t, ethClock)
126-
sent, err := New(context.Background(), &SentinelConfig{
140+
sent, err := New(t.Context(), &SentinelConfig{
127141
NetworkConfig: networkConfig,
128142
BeaconConfig: beaconConfig,
129143
EnableBlocks: true,
130144
MaxPeerCount: 100,
131145
}, ethClock, reader, nil, db, log.New(), &mock_services.ForkChoiceStorageMock{}, nil, mockPeerDasStateReader, pm)
132-
require.NoError(t, err)
146+
noErr(err)
133147
t.Cleanup(func() { sent.Stop() })
134148

135149
_, err = sent.Start()
136-
require.NoError(t, err)
150+
noErr(err)
137151
return sent
138152
}
139153

@@ -148,93 +162,82 @@ func newMockPeerDasStateReader(t *testing.T) *peerdasstatemock.MockPeerDasStateR
148162

149163
func testSentinelBlocksByRange(t *testing.T) {
150164
ethClock := getEthClock(t)
151-
ctx := context.Background()
165+
ctx := t.Context()
152166
db, blocks, _, _, reader := loadChain(t)
153167
_, beaconConfig := clparams.GetConfigsByNetwork(chainspec.MainnetChainID)
154168

155169
sent := newTestSentinel(t, ethClock, reader, db, newMockPeerDasStateReader(t))
156170
h := sent.Host()
157171

158172
host1, err := libp2p.New(libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"))
159-
require.NoError(t, err)
173+
noErr(err)
160174
defer host1.Close()
161175

162-
err = h.Connect(ctx, peer.AddrInfo{
163-
ID: host1.ID(),
164-
Addrs: host1.Addrs(),
165-
})
166-
require.NoError(t, err)
176+
noErr(h.Connect(ctx, peer.AddrInfo{ID: host1.ID(), Addrs: host1.Addrs()}))
167177

168178
stream, err := host1.NewStream(ctx, h.ID(), protocol.ID(communication.BeaconBlocksByRangeProtocolV2))
169-
require.NoError(t, err)
179+
noErr(err)
180+
defer stream.Close()
170181

171182
req := &cltypes.BeaconBlocksByRangeRequest{
172183
StartSlot: blocks[0].Block.Slot,
173184
Count: 6,
174185
}
175-
176-
if err := ssz_snappy.EncodeAndWrite(stream, req); err != nil {
177-
panic(fmt.Sprintf("EncodeAndWrite failed: %v", err))
178-
}
186+
noErr(ssz_snappy.EncodeAndWrite(stream, req))
179187

180188
code := make([]byte, 1)
181189
_, err = stream.Read(code)
182-
require.NoError(t, err)
183-
require.Equal(t, uint8(0), code[0])
190+
noErr(err)
191+
assertPanic(code[0] == uint8(0), "expected code[0]=0, got %d", code[0])
184192

185193
var w bytes.Buffer
186194
_, err = io.Copy(&w, stream)
187-
require.NoError(t, err)
195+
noErr(err)
188196

189197
responsePacket := make([]*cltypes.SignedBeaconBlock, 0)
190-
191198
r := bytes.NewReader(w.Bytes())
192199
for i := 0; i < len(blocks); i++ {
193200
forkDigest := make([]byte, 4)
194201
if _, err := r.Read(forkDigest); err != nil {
195202
if err == io.EOF {
196203
break
197204
}
198-
require.NoError(t, err)
205+
noErr(err)
199206
}
200207

201208
encodedLn, _, err := ssz_snappy.ReadUvarint(r)
202-
require.NoError(t, err)
209+
noErr(err)
203210

204211
raw := make([]byte, encodedLn)
205212
sr := snappy.NewReader(r)
206213
bytesRead := 0
207214
for bytesRead < int(encodedLn) {
208215
n, err := sr.Read(raw[bytesRead:])
209-
require.NoError(t, err)
216+
noErr(err)
210217
bytesRead += n
211218
}
212-
respForkDigest := binary.BigEndian.Uint32(forkDigest)
213-
require.NoError(t, err)
214219

215-
version, err := ethClock.StateVersionByForkDigest(utils.Uint32ToBytes4(respForkDigest))
216-
require.NoError(t, err)
220+
version, err := ethClock.StateVersionByForkDigest(utils.Uint32ToBytes4(binary.BigEndian.Uint32(forkDigest)))
221+
noErr(err)
217222

218223
responseChunk := cltypes.NewSignedBeaconBlock(beaconConfig, clparams.DenebVersion)
219-
require.NoError(t, responseChunk.DecodeSSZ(raw, int(version)))
224+
noErr(responseChunk.DecodeSSZ(raw, int(version)))
220225

221226
responsePacket = append(responsePacket, responseChunk)
222227
r.ReadByte()
223228
}
224-
require.Len(t, blocks, len(responsePacket))
229+
assertPanic(len(blocks) == len(responsePacket), "expected %d blocks, got %d", len(blocks), len(responsePacket))
225230
for i := 0; i < len(blocks); i++ {
226231
root1, err := responsePacket[i].HashSSZ()
227-
require.NoError(t, err)
228-
232+
noErr(err)
229233
root2, err := blocks[i].HashSSZ()
230-
require.NoError(t, err)
231-
232-
require.Equal(t, root1, root2)
234+
noErr(err)
235+
assertPanic(root1 == root2, "block %d root mismatch: %x != %x", i, root1, root2)
233236
}
234237
}
235238

236239
func testSentinelBlocksByRoots(t *testing.T) {
237-
ctx := context.Background()
240+
ctx := t.Context()
238241
db, blocks, _, _, reader := loadChain(t)
239242
ethClock := getEthClock(t)
240243
_, beaconConfig := clparams.GetConfigsByNetwork(chainspec.MainnetChainID)
@@ -243,105 +246,89 @@ func testSentinelBlocksByRoots(t *testing.T) {
243246
h := sent.Host()
244247

245248
host1, err := libp2p.New(libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"))
246-
require.NoError(t, err)
249+
noErr(err)
247250
defer host1.Close()
248251

249-
err = h.Connect(ctx, peer.AddrInfo{
250-
ID: host1.ID(),
251-
Addrs: host1.Addrs(),
252-
})
253-
require.NoError(t, err)
252+
noErr(h.Connect(ctx, peer.AddrInfo{ID: host1.ID(), Addrs: host1.Addrs()}))
254253

255254
stream, err := host1.NewStream(ctx, h.ID(), protocol.ID(communication.BeaconBlocksByRootProtocolV2))
256-
require.NoError(t, err)
255+
noErr(err)
256+
defer stream.Close()
257257

258258
req := solid.NewHashList(1232)
259259
rt, err := blocks[0].Block.HashSSZ()
260-
require.NoError(t, err)
261-
260+
noErr(err)
262261
req.Append(rt)
263262
rt, err = blocks[1].Block.HashSSZ()
264-
require.NoError(t, err)
263+
noErr(err)
265264
req.Append(rt)
266265

267-
if err := ssz_snappy.EncodeAndWrite(stream, req); err != nil {
268-
panic(fmt.Sprintf("EncodeAndWrite failed: %v", err))
269-
}
266+
noErr(ssz_snappy.EncodeAndWrite(stream, req))
270267

271268
code := make([]byte, 1)
272269
_, err = stream.Read(code)
273-
require.NoError(t, err)
274-
require.Equal(t, uint8(0), code[0])
270+
noErr(err)
271+
assertPanic(code[0] == uint8(0), "expected code[0]=0, got %d", code[0])
275272

276273
var w bytes.Buffer
277274
_, err = io.Copy(&w, stream)
278-
require.NoError(t, err)
275+
noErr(err)
279276

280277
responsePacket := make([]*cltypes.SignedBeaconBlock, 0)
281-
282278
r := bytes.NewReader(w.Bytes())
283279
for i := 0; i < len(blocks); i++ {
284280
forkDigest := make([]byte, 4)
285281
if _, err := r.Read(forkDigest); err != nil {
286282
if err == io.EOF {
287283
break
288284
}
289-
require.NoError(t, err)
285+
noErr(err)
290286
}
291287

292288
encodedLn, _, err := ssz_snappy.ReadUvarint(r)
293-
require.NoError(t, err)
289+
noErr(err)
294290

295291
raw := make([]byte, encodedLn)
296292
sr := snappy.NewReader(r)
297293
bytesRead := 0
298294
for bytesRead < int(encodedLn) {
299295
n, err := sr.Read(raw[bytesRead:])
300-
require.NoError(t, err)
296+
noErr(err)
301297
bytesRead += n
302298
}
303-
respForkDigest := binary.BigEndian.Uint32(forkDigest)
304-
require.NoError(t, err)
305299

306-
version, err := ethClock.StateVersionByForkDigest(utils.Uint32ToBytes4(respForkDigest))
307-
require.NoError(t, err)
300+
version, err := ethClock.StateVersionByForkDigest(utils.Uint32ToBytes4(binary.BigEndian.Uint32(forkDigest)))
301+
noErr(err)
308302

309303
responseChunk := cltypes.NewSignedBeaconBlock(beaconConfig, clparams.DenebVersion)
310-
require.NoError(t, responseChunk.DecodeSSZ(raw, int(version)))
304+
noErr(responseChunk.DecodeSSZ(raw, int(version)))
311305

312306
responsePacket = append(responsePacket, responseChunk)
313307
r.ReadByte()
314308
}
315-
316-
require.Len(t, blocks, len(responsePacket))
309+
assertPanic(len(blocks) == len(responsePacket), "expected %d blocks, got %d", len(blocks), len(responsePacket))
317310
for i := 0; i < len(responsePacket); i++ {
318311
root1, err := responsePacket[i].HashSSZ()
319-
require.NoError(t, err)
320-
312+
noErr(err)
321313
root2, err := blocks[i].HashSSZ()
322-
require.NoError(t, err)
323-
324-
require.Equal(t, root1, root2)
314+
noErr(err)
315+
assertPanic(root1 == root2, "block %d root mismatch: %x != %x", i, root1, root2)
325316
}
326317
}
327318

328319
func testSentinelStatusRequest(t *testing.T) {
329-
ctx := context.Background()
320+
ctx := t.Context()
330321
db, blocks, _, _, reader := loadChain(t)
331322
ethClock := getEthClock(t)
332323

333324
sent := newTestSentinel(t, ethClock, reader, db, newMockPeerDasStateReader(t))
334325
h := sent.Host()
335326

336327
host1, err := libp2p.New(libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/0"))
337-
require.NoError(t, err)
328+
noErr(err)
338329
defer host1.Close()
339330

340-
err = h.Connect(ctx, peer.AddrInfo{
341-
ID: host1.ID(),
342-
Addrs: host1.Addrs(),
343-
})
344-
require.NoError(t, err)
331+
noErr(h.Connect(ctx, peer.AddrInfo{ID: host1.ID(), Addrs: host1.Addrs()}))
345332

346333
req := &cltypes.Status{
347334
HeadRoot: common.Hash(blocks[0].Block.ParentRoot),
@@ -352,25 +339,23 @@ func testSentinelStatusRequest(t *testing.T) {
352339
sent.SetStatus(req)
353340

354341
stream, err := host1.NewStream(ctx, h.ID(), protocol.ID(communication.StatusProtocolV1))
355-
require.NoError(t, err)
342+
noErr(err)
343+
defer stream.Close()
356344

357-
if err := ssz_snappy.EncodeAndWrite(stream, req); err != nil {
358-
panic(fmt.Sprintf("EncodeAndWrite failed: %v", err))
359-
}
345+
noErr(ssz_snappy.EncodeAndWrite(stream, req))
360346

361347
code := make([]byte, 1)
362348
_, err = stream.Read(code)
363-
require.NoError(t, err)
364-
require.Equal(t, uint8(0), code[0])
349+
noErr(err)
350+
assertPanic(code[0] == uint8(0), "expected code[0]=0, got %d", code[0])
365351

366352
resp := &cltypes.Status{}
367-
err = ssz_snappy.DecodeAndReadNoForkDigest(stream, resp, 0)
368-
require.NoError(t, err)
353+
noErr(ssz_snappy.DecodeAndReadNoForkDigest(stream, resp, 0))
369354

370-
require.Equal(t, req.HeadRoot, resp.HeadRoot)
371-
require.Equal(t, req.HeadSlot, resp.HeadSlot)
372-
require.Equal(t, req.FinalizedRoot, resp.FinalizedRoot)
373-
require.Equal(t, req.FinalizedEpoch, resp.FinalizedEpoch)
355+
assertPanic(req.HeadRoot == resp.HeadRoot, "HeadRoot mismatch: %v != %v", req.HeadRoot, resp.HeadRoot)
356+
assertPanic(req.HeadSlot == resp.HeadSlot, "HeadSlot mismatch: %v != %v", req.HeadSlot, resp.HeadSlot)
357+
assertPanic(req.FinalizedRoot == resp.FinalizedRoot, "FinalizedRoot mismatch: %v != %v", req.FinalizedRoot, resp.FinalizedRoot)
358+
assertPanic(req.FinalizedEpoch == resp.FinalizedEpoch, "FinalizedEpoch mismatch: %v != %v", req.FinalizedEpoch, resp.FinalizedEpoch)
374359
}
375360

376361
func TestSentinelBlocksByRange(t *testing.T) {

0 commit comments

Comments
 (0)