From ff2b8879f6572b711e5c24df4d2cfd8037a8e219 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Fri, 27 Oct 2023 16:35:21 +1000 Subject: [PATCH 1/4] op-e2e: Split e2e tests into two executors --- .circleci/config.yml | 6 +++++ op-e2e/faultproof_test.go | 32 ++++++++++++----------- op-e2e/helper.go | 55 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index beaffef8fb97..57cc16eb6c0c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -779,6 +779,12 @@ jobs: docker: - image: us-docker.pkg.dev/oplabs-tools-artifacts/images/ci-builder:latest resource_class: xlarge + # Note: Tests are split between runs manually. + # Tests default to run on the first executor but can be moved to the second with: + # InitParallel(t, UseExecutor(1)) + # Any tests assigned to an executor greater than the number available automatically use the last executor. + # Executor indexes start from 0 + parallelism: 2 steps: - checkout - check-changed: diff --git a/op-e2e/faultproof_test.go b/op-e2e/faultproof_test.go index e6b2c755aeba..1998cf6c7516 100644 --- a/op-e2e/faultproof_test.go +++ b/op-e2e/faultproof_test.go @@ -18,7 +18,7 @@ import ( ) func TestMultipleCannonGames(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -78,7 +78,7 @@ func TestMultipleCannonGames(t *testing.T) { } func TestMultipleGameTypes(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -113,7 +113,7 @@ func TestMultipleGameTypes(t *testing.T) { } func TestChallengerCompleteDisputeGame(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) tests := []struct { name string @@ -182,7 +182,7 @@ func TestChallengerCompleteDisputeGame(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -219,7 +219,7 @@ func TestChallengerCompleteDisputeGame(t *testing.T) { } func TestChallengerCompleteExhaustiveDisputeGame(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) testCase := func(t *testing.T, isRootCorrect bool) { ctx := context.Background() @@ -267,17 +267,17 @@ func TestChallengerCompleteExhaustiveDisputeGame(t *testing.T) { } t.Run("RootCorrect", func(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) testCase(t, true) }) t.Run("RootIncorrect", func(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) testCase(t, false) }) } func TestCannonDisputeGame(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) tests := []struct { name string @@ -290,7 +290,7 @@ func TestCannonDisputeGame(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -328,7 +328,7 @@ func TestCannonDisputeGame(t *testing.T) { } func TestCannonDefendStep(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -370,7 +370,7 @@ func TestCannonDefendStep(t *testing.T) { } func TestCannonProposedOutputRootInvalid(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) // honestStepsFail attempts to perform both an attack and defend step using the correct trace. honestStepsFail := func(ctx context.Context, game *disputegame.CannonGameHelper, correctTrace *disputegame.HonestHelper, parentClaimIdx int64) { // Attack step should fail @@ -421,7 +421,7 @@ func TestCannonProposedOutputRootInvalid(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - InitParallel(t) + InitParallel(t, UseExecutor(1)) ctx := context.Background() sys, l1Client, game, correctTrace := setupDisputeGameForInvalidOutputRoot(t, test.outputRoot) @@ -448,7 +448,7 @@ func TestCannonProposedOutputRootInvalid(t *testing.T) { } func TestCannonPoisonedPostState(t *testing.T) { - InitParallel(t, UsesCannon) + InitParallel(t, UsesCannon, UseExecutor(1)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -558,8 +558,10 @@ func setupDisputeGameForInvalidOutputRoot(t *testing.T, outputRoot common.Hash) } func TestCannonChallengeWithCorrectRoot(t *testing.T) { - InitParallel(t, UsesCannon) - + InitParallel(t, UsesCannon, UseExecutor(1)) + if true { + return + } ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) t.Cleanup(sys.Close) diff --git a/op-e2e/helper.go b/op-e2e/helper.go index 0b36458d4780..af0954a7b143 100644 --- a/op-e2e/helper.go +++ b/op-e2e/helper.go @@ -2,23 +2,70 @@ package op_e2e import ( "os" + "strconv" "testing" ) var enableParallelTesting bool = os.Getenv("OP_E2E_DISABLE_PARALLEL") != "true" -func InitParallel(t *testing.T, opts ...func(t *testing.T)) { +type testopts struct { + executor int +} + +func InitParallel(t *testing.T, args ...func(t *testing.T, opts *testopts)) { t.Helper() if enableParallelTesting { t.Parallel() } - for _, opt := range opts { - opt(t) + + opts := &testopts{} + for _, arg := range args { + arg(t, opts) } + checkExecutor(t, opts.executor) } -func UsesCannon(t *testing.T) { +func UsesCannon(t *testing.T, opts *testopts) { if os.Getenv("OP_E2E_CANNON_ENABLED") == "false" { t.Skip("Skipping cannon test") } } + +// UseExecutor allows manually splitting tests between circleci executors +// +// Tests default to run on the first executor but can be moved to the second with: +// InitParallel(t, UseExecutor(1)) +// Any tests assigned to an executor greater than the number available automatically use the last executor. +// Executor indexes start from 0 +func UseExecutor(assignedIdx int) func(t *testing.T, opts *testopts) { + return func(t *testing.T, opts *testopts) { + opts.executor = assignedIdx + } +} + +func checkExecutor(t *testing.T, assignedIdx int) { + envTotal := os.Getenv("CIRCLE_NODE_TOTAL") + envIdx := os.Getenv("CIRCLE_NODE_INDEX") + if envTotal == "" || envIdx == "" { + // Not using test splitting, so ignore assigned executor + t.Logf("Running test. Test splitting not in use.") + return + } + total, err := strconv.Atoi(envTotal) + if err != nil { + t.Fatalf("Could not parse CIRCLE_NODE_TOTAL env var %v: %v", envTotal, err) + } + idx, err := strconv.Atoi(envIdx) + if err != nil { + t.Fatalf("Could not parse CIRCLE_NODE_INDEX env var %v: %v", envIdx, err) + } + if assignedIdx >= total && idx == total-1 { + t.Logf("Running test. Current executor (%v) is the last executor and assigned executor (%v) >= total executors (%v).", idx, assignedIdx, total) + return + } + if idx == assignedIdx { + t.Logf("Running test. Assigned executor (%v) matches current executor (%v) of total (%v)", assignedIdx, idx, total) + return + } + t.Skipf("Skipping test. Assigned executor %v, current executor %v of total %v", assignedIdx, idx, total) +} From 7d51ff39c7d3edfe5a2f9616e1e732c133be39e1 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Fri, 27 Oct 2023 16:57:11 +1000 Subject: [PATCH 2/4] op-e2e: Move some tests back to executor 0. --- op-e2e/faultproof_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/op-e2e/faultproof_test.go b/op-e2e/faultproof_test.go index 1998cf6c7516..0cfdbbe97dc8 100644 --- a/op-e2e/faultproof_test.go +++ b/op-e2e/faultproof_test.go @@ -18,7 +18,7 @@ import ( ) func TestMultipleCannonGames(t *testing.T) { - InitParallel(t, UsesCannon, UseExecutor(1)) + InitParallel(t, UsesCannon, UseExecutor(0)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -78,7 +78,7 @@ func TestMultipleCannonGames(t *testing.T) { } func TestMultipleGameTypes(t *testing.T) { - InitParallel(t, UsesCannon, UseExecutor(1)) + InitParallel(t, UsesCannon, UseExecutor(0)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -370,7 +370,7 @@ func TestCannonDefendStep(t *testing.T) { } func TestCannonProposedOutputRootInvalid(t *testing.T) { - InitParallel(t, UsesCannon, UseExecutor(1)) + InitParallel(t, UsesCannon, UseExecutor(0)) // honestStepsFail attempts to perform both an attack and defend step using the correct trace. honestStepsFail := func(ctx context.Context, game *disputegame.CannonGameHelper, correctTrace *disputegame.HonestHelper, parentClaimIdx int64) { // Attack step should fail @@ -421,7 +421,7 @@ func TestCannonProposedOutputRootInvalid(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - InitParallel(t, UseExecutor(1)) + InitParallel(t, UseExecutor(0)) ctx := context.Background() sys, l1Client, game, correctTrace := setupDisputeGameForInvalidOutputRoot(t, test.outputRoot) @@ -448,7 +448,7 @@ func TestCannonProposedOutputRootInvalid(t *testing.T) { } func TestCannonPoisonedPostState(t *testing.T) { - InitParallel(t, UsesCannon, UseExecutor(1)) + InitParallel(t, UsesCannon, UseExecutor(0)) ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) @@ -558,7 +558,7 @@ func setupDisputeGameForInvalidOutputRoot(t *testing.T, outputRoot common.Hash) } func TestCannonChallengeWithCorrectRoot(t *testing.T) { - InitParallel(t, UsesCannon, UseExecutor(1)) + InitParallel(t, UsesCannon, UseExecutor(0)) if true { return } From 04b4611de9b1f7d1a868e2bdb5dcabae6a20432d Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Fri, 27 Oct 2023 17:27:14 +1000 Subject: [PATCH 3/4] op-e2e: Use unsigned ints to avoid negatives. --- op-e2e/helper.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/op-e2e/helper.go b/op-e2e/helper.go index af0954a7b143..3c8d411b1292 100644 --- a/op-e2e/helper.go +++ b/op-e2e/helper.go @@ -9,7 +9,7 @@ import ( var enableParallelTesting bool = os.Getenv("OP_E2E_DISABLE_PARALLEL") != "true" type testopts struct { - executor int + executor uint64 } func InitParallel(t *testing.T, args ...func(t *testing.T, opts *testopts)) { @@ -37,13 +37,13 @@ func UsesCannon(t *testing.T, opts *testopts) { // InitParallel(t, UseExecutor(1)) // Any tests assigned to an executor greater than the number available automatically use the last executor. // Executor indexes start from 0 -func UseExecutor(assignedIdx int) func(t *testing.T, opts *testopts) { +func UseExecutor(assignedIdx uint64) func(t *testing.T, opts *testopts) { return func(t *testing.T, opts *testopts) { opts.executor = assignedIdx } } -func checkExecutor(t *testing.T, assignedIdx int) { +func checkExecutor(t *testing.T, assignedIdx uint64) { envTotal := os.Getenv("CIRCLE_NODE_TOTAL") envIdx := os.Getenv("CIRCLE_NODE_INDEX") if envTotal == "" || envIdx == "" { @@ -51,11 +51,11 @@ func checkExecutor(t *testing.T, assignedIdx int) { t.Logf("Running test. Test splitting not in use.") return } - total, err := strconv.Atoi(envTotal) + total, err := strconv.ParseUint(envTotal, 10, 0) if err != nil { t.Fatalf("Could not parse CIRCLE_NODE_TOTAL env var %v: %v", envTotal, err) } - idx, err := strconv.Atoi(envIdx) + idx, err := strconv.ParseUint(envIdx, 10, 0) if err != nil { t.Fatalf("Could not parse CIRCLE_NODE_INDEX env var %v: %v", envIdx, err) } From db016e6e0aa73cced5176f41b8f52c28362af3d6 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Mon, 30 Oct 2023 17:06:31 +1000 Subject: [PATCH 4/4] op-e2e: Remove manual test code. --- op-e2e/faultproof_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/op-e2e/faultproof_test.go b/op-e2e/faultproof_test.go index 0cfdbbe97dc8..7857fbbee0f7 100644 --- a/op-e2e/faultproof_test.go +++ b/op-e2e/faultproof_test.go @@ -559,9 +559,6 @@ func setupDisputeGameForInvalidOutputRoot(t *testing.T, outputRoot common.Hash) func TestCannonChallengeWithCorrectRoot(t *testing.T) { InitParallel(t, UsesCannon, UseExecutor(0)) - if true { - return - } ctx := context.Background() sys, l1Client := startFaultDisputeSystem(t) t.Cleanup(sys.Close)