diff --git a/cmd/testbeacon.go b/cmd/testbeacon.go index 1e1b3d140..017543769 100644 --- a/cmd/testbeacon.go +++ b/cmd/testbeacon.go @@ -319,33 +319,13 @@ 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) } resCh <- map[string][]testResult{target: allTestRes} @@ -353,16 +333,23 @@ func testSingleBeacon(ctx context.Context, queuedTestCases []testCaseName, allTe 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 } } @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() diff --git a/cmd/testbeacon_internal_test.go b/cmd/testbeacon_internal_test.go index 45b1522cd..ed448e8c2 100644 --- a/cmd/testbeacon_internal_test.go +++ b/cmd/testbeacon_internal_test.go @@ -9,7 +9,7 @@ import ( "io" "net/http" "net/http/httptest" - "os" + "path/filepath" "testing" "time" @@ -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}, }, }, }, @@ -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, @@ -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 { @@ -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() diff --git a/cmd/testinfra.go b/cmd/testinfra.go index 5abd4ae83..d9a78a05b 100644 --- a/cmd/testinfra.go +++ b/cmd/testinfra.go @@ -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 } } diff --git a/cmd/testinfra_internal_test.go b/cmd/testinfra_internal_test.go index 00247743c..30a5ca7bf 100644 --- a/cmd/testinfra_internal_test.go +++ b/cmd/testinfra_internal_test.go @@ -8,7 +8,7 @@ import ( "fmt" "io" "net" - "os" + "path/filepath" "testing" "time" @@ -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, @@ -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, @@ -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 { @@ -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() diff --git a/cmd/testmev.go b/cmd/testmev.go index 19195dfb4..32b4f537d 100644 --- a/cmd/testmev.go +++ b/cmd/testmev.go @@ -216,51 +216,37 @@ func testAllMEVs(ctx context.Context, queuedTestCases []testCaseName, allTestCas } func testSingleMEV(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseMEV, cfg testMEVConfig, target string, resCh chan map[string][]testResult) error { - singleTestResCh := make(chan testResult) - allTestRes := []testResult{} + singleTestResCh := make(chan testResult, len(queuedTestCases)) - // run all mev tests for a mev node, pushing each completed test to the channel until all are complete or timeout occurs go runMEVTest(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) } - relayName := formatMEVRelayName(target) - resCh <- map[string][]testResult{relayName: allTestRes} + resCh <- map[string][]testResult{formatMEVRelayName(target): allTestRes} return nil } +// runMEVTest is the sole producer of results on ch; test case functions must respect ctx cancellation. func runMEVTest(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseMEV, cfg testMEVConfig, 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 } } diff --git a/cmd/testmev_internal_test.go b/cmd/testmev_internal_test.go index 028453153..f4576e406 100644 --- a/cmd/testmev_internal_test.go +++ b/cmd/testmev_internal_test.go @@ -9,7 +9,7 @@ import ( "io" "net/http" "net/http/httptest" - "os" + "path/filepath" "testing" "time" @@ -128,9 +128,13 @@ func TestMEVTest(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: "CreateBlock", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, endpoint2: { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "CreateBlock", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, }, }, @@ -204,7 +208,7 @@ func TestMEVTest(t *testing.T) { name: "write to file", config: testMEVConfig{ 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, @@ -228,12 +232,6 @@ func TestMEVTest(t *testing.T) { CategoryName: mevTestCategory, }, expectedErr: "", - cleanup: func(t *testing.T, p string) { - t.Helper() - - err := os.Remove(p) - require.NoError(t, err) - }, }, } for _, test := range tests { @@ -269,6 +267,31 @@ func TestMEVTest(t *testing.T) { } } +func TestMEVTestTimeoutAlwaysProducesResults(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 := runTestMEV(context.Background(), &buf, testMEVConfig{ + 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 StartHealthyMockedMEVNode(t *testing.T) *httptest.Server { t.Helper() diff --git a/cmd/testpeers.go b/cmd/testpeers.go index 1b72892ed..8a12b2e14 100644 --- a/cmd/testpeers.go +++ b/cmd/testpeers.go @@ -304,9 +304,6 @@ func testAllPeers(ctx context.Context, queuedTestCases []testCaseName, allTestCa } func testSinglePeer(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCasePeer, conf testPeersConfig, p2pNode host.Host, target string, allTestResCh chan map[string][]testResult) error { - singleTestResCh := make(chan testResult) - allTestRes := []testResult{} - enrTarget, err := enr.Parse(target) if err != nil { return err @@ -321,37 +318,17 @@ func testSinglePeer(ctx context.Context, queuedTestCases []testCaseName, allTest nameENR := fmt.Sprintf("peer %v %v", peerTarget.Name, formatENR) if len(queuedTestCases) == 0 { - allTestResCh <- map[string][]testResult{nameENR: allTestRes} + allTestResCh <- map[string][]testResult{nameENR: {}} return nil } - // run all peers tests for a peer, pushing each completed test to the channel until all are complete or timeout occurs - go runPeerTest(ctx, queuedTestCases, allTestCases, conf, p2pNode, peerTarget, singleTestResCh) - - testCounter := 0 - - finished := false - for !finished { - var testName string - - select { - case <-ctx.Done(): - if testCounter < len(queuedTestCases) { - testName = queuedTestCases[testCounter].name - allTestRes = append(allTestRes, testResult{Name: testName, Verdict: testVerdictFail, Error: errTimeoutInterrupted}) - } - - finished = true - case result, ok := <-singleTestResCh: - if !ok { - finished = true - continue - } + singleTestResCh := make(chan testResult, len(queuedTestCases)) - testCounter++ + go runPeerTest(ctx, queuedTestCases, allTestCases, conf, p2pNode, peerTarget, singleTestResCh) - allTestRes = append(allTestRes, result) - } + var allTestRes []testResult + for result := range singleTestResCh { + allTestRes = append(allTestRes, result) } allTestResCh <- map[string][]testResult{nameENR: allTestRes} @@ -359,17 +336,23 @@ func testSinglePeer(ctx context.Context, queuedTestCases []testCaseName, allTest return nil } +// runPeerTest is the sole producer of results on testResCh; test case functions must respect ctx cancellation. func runPeerTest(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCasePeer, conf testPeersConfig, p2pNode host.Host, target p2p.Peer, testResCh chan testResult) { defer close(testResCh) - for _, t := range queuedTestCases { - select { - case <-ctx.Done(): + for i, t := range queuedTestCases { + result := allTestCases[t](ctx, &conf, p2pNode, target) + if ctx.Err() != nil { testResCh <- failedTestResult(testResult{Name: t.name}, errTimeoutInterrupted) + + for _, remaining := range queuedTestCases[i+1:] { + testResCh <- failedTestResult(testResult{Name: remaining.name}, errTimeoutInterrupted) + } + return - default: - testResCh <- allTestCases[t](ctx, &conf, p2pNode, target) } + + testResCh <- result } } @@ -476,13 +459,18 @@ func peerDirectConnTest(ctx context.Context, conf *testPeersConfig, p2pNode host z.Any("target", p2pPeer.Name)) var err error + for range int(conf.DirectConnectionTimeout.Seconds()) { + if ctx.Err() != nil { + return failedTestResult(testRes, errTimeoutInterrupted) + } + err = p2pNode.Connect(network.WithForceDirectDial(ctx, "relay_to_direct"), peer.AddrInfo{ID: p2pPeer.ID}) if err == nil { break } - time.Sleep(time.Second) + sleepWithContext(ctx, time.Second) } if err != nil { @@ -504,38 +492,18 @@ func peerDirectConnTest(ctx context.Context, conf *testPeersConfig, p2pNode host // self tests func testSelf(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCasePeerSelf, conf testPeersConfig, allTestResCh chan map[string][]testResult) error { - singleTestResCh := make(chan testResult) - - allTestRes := []testResult{} if len(queuedTestCases) == 0 { - allTestResCh <- map[string][]testResult{"self": allTestRes} + allTestResCh <- map[string][]testResult{"self": {}} return nil } - go runSelfTest(ctx, queuedTestCases, allTestCases, conf, singleTestResCh) + singleTestResCh := make(chan testResult, len(queuedTestCases)) - 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 - continue - } + go runSelfTest(ctx, queuedTestCases, allTestCases, conf, singleTestResCh) - testName = queuedTestCases[testCounter].name - testCounter++ - result.Name = testName - allTestRes = append(allTestRes, result) - } + var allTestRes []testResult + for result := range singleTestResCh { + allTestRes = append(allTestRes, result) } allTestResCh <- map[string][]testResult{"self": allTestRes} @@ -543,16 +511,23 @@ func testSelf(ctx context.Context, queuedTestCases []testCaseName, allTestCases return nil } +// runSelfTest is the sole producer of results on ch; test case functions must respect ctx cancellation. func runSelfTest(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCasePeerSelf, conf testPeersConfig, ch chan testResult) { defer close(ch) - for _, t := range queuedTestCases { - select { - case <-ctx.Done(): + for i, t := range queuedTestCases { + result := allTestCases[t](ctx, &conf) + 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, &conf) } + + ch <- result } } @@ -613,40 +588,19 @@ func testAllRelays(ctx context.Context, queuedTestCases []testCaseName, allTestC } func testSingleRelay(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseRelay, conf testPeersConfig, target string, allTestResCh chan map[string][]testResult) error { - singleTestResCh := make(chan testResult) - allTestRes := []testResult{} - relayName := fmt.Sprintf("relay %v", target) if len(queuedTestCases) == 0 { - allTestResCh <- map[string][]testResult{relayName: allTestRes} + allTestResCh <- map[string][]testResult{relayName: {}} return nil } - // run all relay tests for a relay, pushing each completed test to the channel until all are complete or timeout occurs - go runRelayTest(ctx, queuedTestCases, allTestCases, conf, target, singleTestResCh) - - testCounter := 0 + singleTestResCh := make(chan testResult, len(queuedTestCases)) - 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 - continue - } + go runRelayTest(ctx, queuedTestCases, allTestCases, conf, target, singleTestResCh) - testName = queuedTestCases[testCounter].name - testCounter++ - result.Name = testName - allTestRes = append(allTestRes, result) - } + var allTestRes []testResult + for result := range singleTestResCh { + allTestRes = append(allTestRes, result) } allTestResCh <- map[string][]testResult{relayName: allTestRes} @@ -654,16 +608,23 @@ func testSingleRelay(ctx context.Context, queuedTestCases []testCaseName, allTes return nil } +// runRelayTest is the sole producer of results on testResCh; test case functions must respect ctx cancellation. func runRelayTest(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]testCaseRelay, conf testPeersConfig, target string, testResCh chan testResult) { defer close(testResCh) - for _, t := range queuedTestCases { - select { - case <-ctx.Done(): + for i, t := range queuedTestCases { + result := allTestCases[t](ctx, &conf, target) + if ctx.Err() != nil { + testResCh <- failedTestResult(testResult{Name: t.name}, errTimeoutInterrupted) + + for _, remaining := range queuedTestCases[i+1:] { + testResCh <- failedTestResult(testResult{Name: remaining.name}, errTimeoutInterrupted) + } + return - default: - testResCh <- allTestCases[t](ctx, &conf, target) } + + testResCh <- result } } diff --git a/cmd/testpeers_internal_test.go b/cmd/testpeers_internal_test.go index 136837fb0..b388a6b9e 100644 --- a/cmd/testpeers_internal_test.go +++ b/cmd/testpeers_internal_test.go @@ -13,6 +13,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "slices" "strings" "testing" @@ -148,12 +149,21 @@ func TestPeersTest(t *testing.T) { }, "peer inexpensive-farm enr:-HW4QBHlc...rx6o": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer anxious-pencil enr:-HW4QDwUF...vKDw": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer important-pen enr:-HW4QPSBg...wbr0": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, }, Score: categoryScoreC, @@ -224,7 +234,7 @@ func TestPeersTest(t *testing.T) { name: "write to file", config: testPeersConfig{ testConfig: testConfig{ - OutputJSON: "./write-to-file-test.json.tmp", + OutputJSON: filepath.Join(t.TempDir(), "write-to-file-test.json.tmp"), Quiet: false, Timeout: 3 * time.Second, }, @@ -251,12 +261,21 @@ func TestPeersTest(t *testing.T) { }, "peer inexpensive-farm enr:-HW4QBHlc...rx6o": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer anxious-pencil enr:-HW4QDwUF...vKDw": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer important-pen enr:-HW4QPSBg...wbr0": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, }, Score: categoryScoreC, @@ -294,12 +313,21 @@ func TestPeersTest(t *testing.T) { }, "peer inexpensive-farm enr:-HW4QBHlc...rx6o": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer anxious-pencil enr:-HW4QDwUF...vKDw": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, "peer important-pen enr:-HW4QPSBg...wbr0": { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "DirectConn", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, }, Score: categoryScoreC, @@ -346,11 +374,6 @@ func TestPeersTest(t *testing.T) { if test.config.OutputJSON != "" { testWriteFile(t, test.expected, test.config.OutputJSON) } - - if conf.OutputJSON != "" { - err = os.Remove(conf.OutputJSON) - require.NoError(t, err) - } }) } } diff --git a/cmd/testvalidator.go b/cmd/testvalidator.go index 919ed8d73..723ac7c7a 100644 --- a/cmd/testvalidator.go +++ b/cmd/testvalidator.go @@ -145,47 +145,35 @@ func runTestValidator(ctx context.Context, w io.Writer, cfg testValidatorConfig) func testSingleValidator(ctx context.Context, queuedTestCases []testCaseName, allTestCases map[testCaseName]func(context.Context, *testValidatorConfig) testResult, cfg testValidatorConfig, resCh chan map[string][]testResult) { defer close(resCh) - singleTestResCh := make(chan testResult) - allTestRes := []testResult{} - // run all validator tests for a validator client, pushing each completed test to the channel until all are complete or timeout occurs - go testValidator(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 testValidator(ctx, queuedTestCases, allTestCases, cfg, singleTestResCh) - allTestRes = append(allTestRes, result) - } + var allTestRes []testResult + for result := range singleTestResCh { + allTestRes = append(allTestRes, result) } resCh <- map[string][]testResult{cfg.APIAddress: allTestRes} } +// testValidator is the sole producer of results on ch; test case functions must respect ctx cancellation. func testValidator(ctx context.Context, queuedTests []testCaseName, allTests map[testCaseName]func(context.Context, *testValidatorConfig) testResult, cfg testValidatorConfig, 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 } } diff --git a/cmd/testvalidator_internal_test.go b/cmd/testvalidator_internal_test.go index 78b49bf5f..837721715 100644 --- a/cmd/testvalidator_internal_test.go +++ b/cmd/testvalidator_internal_test.go @@ -8,7 +8,7 @@ import ( "fmt" "io" "net" - "os" + "path/filepath" "testing" "time" @@ -78,6 +78,8 @@ func TestValidatorTest(t *testing.T) { Targets: map[string][]testResult{ validatorAPIAddress: { {Name: "Ping", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingMeasure", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, + {Name: "PingLoad", Verdict: testVerdictFail, Measurement: "", Suggestion: "", Error: errTimeoutInterrupted}, }, }, Score: categoryScoreC, @@ -151,7 +153,7 @@ func TestValidatorTest(t *testing.T) { name: "write to file", config: testValidatorConfig{ testConfig: testConfig{ - OutputJSON: "./write-to-file-test.json.tmp", + OutputJSON: filepath.Join(t.TempDir(), "write-to-file-test.json.tmp"), Quiet: false, Timeout: time.Minute, }, @@ -169,12 +171,6 @@ func TestValidatorTest(t *testing.T) { CategoryName: validatorTestCategory, }, expectedErr: "", - cleanup: func(t *testing.T, p string) { - t.Helper() - - err := os.Remove(p) - require.NoError(t, err) - }, }, } for _, test := range tests { @@ -210,6 +206,24 @@ func TestValidatorTest(t *testing.T) { } } +func TestValidatorTestTimeoutAlwaysProducesResults(t *testing.T) { + for range 100 { + var buf bytes.Buffer + + res, err := runTestValidator(context.Background(), &buf, testValidatorConfig{ + testConfig: testConfig{ + Timeout: time.Nanosecond, + }, + APIAddress: fmt.Sprintf("localhost:%v", testutil.GetFreePort(t)), + }) + require.NoError(t, err) + + for target, results := range res.Targets { + require.NotEmpty(t, results, "empty results for %s", target) + } + } +} + func StartHealthyValidatorClient(t *testing.T, port int, ready chan bool) error { t.Helper()