Skip to content

*: fix test commands racing #4513

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KaloyanTanev merged 5 commits into from
May 8, 2026
+291 −260
Merged

*: fix test commands racing #4513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2daf730
Fix MEV test indeterminism when stars align :)
KaloyanTanev May 7, 2026
e5b9356
Refactor to simplify
KaloyanTanev May 7, 2026
ab2eff4
Respect context cancellation
KaloyanTanev May 7, 2026
8a7a256
Minor fixes, mainly readability
KaloyanTanev May 7, 2026
8b29989
Apply PR suggestions
KaloyanTanev May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 22 additions & 35 deletions cmd/testbeacon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,50 +319,37 @@ func testAllBeacons(ctx context.Context, queuedTestCases []testCaseName, allTest
}

func testSingleBeacon(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseBeacon, cfg testBeaconConfig, target string, resCh chan map[string][]testResult) error {
singleTestResCh := make(chan testResult)
allTestRes := []testResult{}
singleTestResCh := make(chan testResult, len(queuedTestCases))

// run all beacon tests for a beacon node, pushing each completed test to the channel until all are complete or timeout occurs
go runBeaconTest(ctx, queuedTestCases, allTestCases, cfg, target, singleTestResCh)

testCounter := 0

finished := false
for !finished {
var testName string

select {
case <-ctx.Done():
testName = queuedTestCases[testCounter].name
allTestRes = append(allTestRes, testResult{Name: testName, Verdict: testVerdictFail, Error: errTimeoutInterrupted})
finished = true
case result, ok := <-singleTestResCh:
if !ok {
finished = true
break
}

testCounter++

allTestRes = append(allTestRes, result)
}
var allTestRes []testResult
for result := range singleTestResCh {
allTestRes = append(allTestRes, result)
Comment thread
KaloyanTanev marked this conversation as resolved.
}

resCh <- map[string][]testResult{target: allTestRes}

return nil
}

// runBeaconTest is the sole producer of results on ch; test case functions must respect ctx cancellation.
func runBeaconTest(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseBeacon, cfg testBeaconConfig, target string, ch chan testResult) {
defer close(ch)

for _, t := range queuedTestCases {
select {
case <-ctx.Done():
for i, t := range queuedTestCases {
result := allTestCases[t](ctx, &cfg, target)
if ctx.Err() != nil {
ch <- failedTestResult(testResult{Name: t.name}, errTimeoutInterrupted)

for _, remaining := range queuedTestCases[i+1:] {
ch <- failedTestResult(testResult{Name: remaining.name}, errTimeoutInterrupted)
}

return
default:
ch <- allTestCases[t](ctx, &cfg, target)
}

ch <- result
}
Comment thread
KaloyanTanev marked this conversation as resolved.
}

Expand Down Expand Up @@ -1545,7 +1532,7 @@ func attestationDuty(ctx context.Context, target string, simulationDuration time
pingCtx, cancel := context.WithTimeout(ctx, simulationDuration)
defer cancel()

time.Sleep(randomizeStart(tickTime))
sleepWithContext(ctx, randomizeStart(tickTime))

ticker := time.NewTicker(tickTime)
defer ticker.Stop()
Expand Down Expand Up @@ -1594,7 +1581,7 @@ func aggregationDuty(ctx context.Context, target string, simulationDuration time
slot = 1
}

time.Sleep(randomizeStart(tickTime))
sleepWithContext(ctx, randomizeStart(tickTime))

ticker := time.NewTicker(tickTime)
defer ticker.Stop()
Expand Down Expand Up @@ -1629,7 +1616,7 @@ func proposalDuty(ctx context.Context, target string, simulationDuration time.Du
pingCtx, cancel := context.WithTimeout(ctx, simulationDuration)
defer cancel()

time.Sleep(randomizeStart(tickTime))
sleepWithContext(ctx, randomizeStart(tickTime))

ticker := time.NewTicker(tickTime)
defer ticker.Stop()
Expand Down Expand Up @@ -1677,7 +1664,7 @@ func syncCommitteeDuties(
pingCtx, cancel := context.WithTimeout(ctx, simulationDuration)
defer cancel()

time.Sleep(randomizeStart(tickTimeSubscribe))
sleepWithContext(ctx, randomizeStart(tickTimeSubscribe))

ticker := time.NewTicker(tickTimeSubscribe)
defer ticker.Stop()
Expand All @@ -1704,7 +1691,7 @@ func syncCommitteeContributionDuty(ctx context.Context, target string, simulatio
pingCtx, cancel := context.WithTimeout(ctx, simulationDuration)
defer cancel()

time.Sleep(randomizeStart(tickTime))
sleepWithContext(ctx, randomizeStart(tickTime))

ticker := time.NewTicker(tickTime)
defer ticker.Stop()
Expand Down Expand Up @@ -1745,7 +1732,7 @@ func syncCommitteeMessageDuty(ctx context.Context, target string, simulationDura
pingCtx, cancel := context.WithTimeout(ctx, simulationDuration)
defer cancel()

time.Sleep(randomizeStart(tickTime))
sleepWithContext(ctx, randomizeStart(tickTime))

ticker := time.NewTicker(tickTime)
defer ticker.Stop()
Expand Down
57 changes: 49 additions & 8 deletions cmd/testbeacon_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -99,9 +99,31 @@ func TestBeaconTest(t *testing.T) {
Targets: map[string][]testResult{
endpoint1: {
{Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Version", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Synced", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PeerCount", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate1", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate10", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate100", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate500", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate1000", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "SimulateCustom", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
},
endpoint2: {
{Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Version", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Synced", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PeerCount", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate1", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate10", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate100", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate500", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "Simulate1000", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "SimulateCustom", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
},
},
},
Expand Down Expand Up @@ -164,7 +186,7 @@ func TestBeaconTest(t *testing.T) {
name: "write to file",
config: testBeaconConfig{
testConfig: testConfig{
OutputJSON: "./write-to-file-test.json.tmp",
OutputJSON: filepath.Join(t.TempDir(), "write-to-file-test.json.tmp"),
Quiet: false,
TestCases: nil,
Timeout: time.Minute,
Expand All @@ -177,12 +199,6 @@ func TestBeaconTest(t *testing.T) {
CategoryName: beaconTestCategory,
},
expectedErr: "",
cleanup: func(t *testing.T, p string) {
t.Helper()

err := os.Remove(p)
require.NoError(t, err)
},
},
}
for _, test := range tests {
Expand Down Expand Up @@ -251,6 +267,31 @@ func defaultFailingBNTests(_ *testing.T, endpoint1 string, endpoint2 string, por
}
}

func TestBeaconTestTimeoutAlwaysProducesResults(t *testing.T) {
var endpoints []string
for range 5 {
endpoints = append(endpoints, fmt.Sprintf("http://localhost:%v", testutil.GetFreePort(t)))
}

for range 100 {
var buf bytes.Buffer

res, err := runTestBeacon(context.Background(), &buf, testBeaconConfig{
testConfig: testConfig{
Timeout: time.Nanosecond,
},
Endpoints: endpoints,
})
require.NoError(t, err)

for _, endpoint := range endpoints {
results, ok := res.Targets[endpoint]
require.True(t, ok, "missing results for %s", endpoint)
require.NotEmpty(t, results, "empty results for %s", endpoint)
}
}
}

func startHealthyMockedBeaconNode(t *testing.T) *httptest.Server {
t.Helper()

Expand Down
46 changes: 17 additions & 29 deletions cmd/testinfra.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,47 +213,35 @@ func runTestInfra(ctx context.Context, w io.Writer, cfg testInfraConfig) (res te
func testSingleInfra(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]func(context.Context, *testInfraConfig) testResult, cfg testInfraConfig, resCh chan map[string][]testResult) {
defer close(resCh)

singleTestResCh := make(chan testResult)
allTestRes := []testResult{}
// run all infra tests for a client, pushing each completed test to the channel until all are complete or timeout occurs
go testInfra(ctx, queuedTestCases, allTestCases, cfg, singleTestResCh)

testCounter := 0

finished := false
for !finished {
var testName string

select {
case <-ctx.Done():
testName = queuedTestCases[testCounter].name
allTestRes = append(allTestRes, testResult{Name: testName, Verdict: testVerdictFail, Error: errTimeoutInterrupted})
finished = true
case result, ok := <-singleTestResCh:
if !ok {
finished = true
break
}
singleTestResCh := make(chan testResult, len(queuedTestCases))

testCounter++
go testInfra(ctx, queuedTestCases, allTestCases, cfg, singleTestResCh)

allTestRes = append(allTestRes, result)
}
var allTestRes []testResult
for result := range singleTestResCh {
allTestRes = append(allTestRes, result)
}

resCh <- map[string][]testResult{"local": allTestRes}
}

// testInfra is the sole producer of results on ch; test case functions must respect ctx cancellation.
func testInfra(ctx context.Context, queuedTests []testCaseName, allTests map[testCaseName]func(context.Context, *testInfraConfig) testResult, cfg testInfraConfig, ch chan testResult) {
defer close(ch)

for _, t := range queuedTests {
select {
case <-ctx.Done():
for i, t := range queuedTests {
result := allTests[t](ctx, &cfg)
if ctx.Err() != nil {
ch <- failedTestResult(testResult{Name: t.name}, errTimeoutInterrupted)

for _, remaining := range queuedTests[i+1:] {
ch <- failedTestResult(testResult{Name: remaining.name}, errTimeoutInterrupted)
}

return
default:
ch <- allTests[t](ctx, &cfg)
}

ch <- result
}
}

Expand Down
36 changes: 28 additions & 8 deletions cmd/testinfra_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"fmt"
"io"
"net"
"os"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -94,6 +94,14 @@ func TestInfraTest(t *testing.T) {
Targets: map[string][]testResult{
"local": {
{Name: "DiskWriteSpeed", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "DiskWriteIOPS", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "DiskReadSpeed", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "DiskReadIOPS", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "AvailableMemory", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "TotalMemory", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "InternetLatency", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "InternetDownloadSpeed", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
{Name: "InternetUploadSpeed", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted},
},
},
Score: categoryScoreC,
Expand Down Expand Up @@ -175,7 +183,7 @@ func TestInfraTest(t *testing.T) {
name: "write to file",
config: testInfraConfig{
testConfig: testConfig{
OutputJSON: "./write-to-file-test.json.tmp",
OutputJSON: filepath.Join(t.TempDir(), "write-to-file-test.json.tmp"),
Quiet: false,
TestCases: []string{"AvailableMemory", "TotalMemory", "InternetLatency", "DiskWriteSpeed", "DiskWriteIOPS", "DiskReadSpeed", "DiskReadIOPS"},
Timeout: time.Minute,
Expand All @@ -199,12 +207,6 @@ func TestInfraTest(t *testing.T) {
CategoryName: infraTestCategory,
},
expectedErr: "",
cleanup: func(t *testing.T, p string) {
t.Helper()

err := os.Remove(p)
require.NoError(t, err)
},
},
}
for _, test := range tests {
Expand Down Expand Up @@ -240,6 +242,24 @@ func TestInfraTest(t *testing.T) {
}
}

func TestInfraTestTimeoutAlwaysProducesResults(t *testing.T) {
for range 100 {
var buf bytes.Buffer

res, err := runTestInfra(context.Background(), &buf, testInfraConfig{
testConfig: testConfig{
Timeout: time.Nanosecond,
},
DiskTestTool: DiskTestToolMock{},
})
require.NoError(t, err)

results, ok := res.Targets["local"]
require.True(t, ok, "missing results for local")
require.NotEmpty(t, results, "empty results for local")
}
}

func StartHealthyInfraClient(t *testing.T, port int, ready chan bool) error {
t.Helper()

Expand Down
Loading
Loading