simulators/ethereum/engine: Fix time drift auth tests, break tests into separate suites (#612)

* simulators/ethereum/engine: Retry auth time drift tests

* simulators/ethereum/engine: Break tests into separate suites

Author: marioevz

simulators/ethereum/engine/authtests.go

@@ -12,6 +12,7 @@ type AuthTestSpec struct {
 	TimeDriftSeconds      int64
 	CustomAuthSecretBytes []byte
 	AuthOk                bool
+	RetryAttempts         int64
 }
 
 var authTestSpecs = []AuthTestSpec{
@@ -38,24 +39,28 @@ var authTestSpecs = []AuthTestSpec{
 		TimeDriftSeconds:      -1 - maxTimeDriftSeconds,
 		CustomAuthSecretBytes: nil,
 		AuthOk:                false,
+		RetryAttempts:         5,
 	},
 	{
 		Name:                  "JWT Authentication: Negative time drift, within limit, correct secret",
 		TimeDriftSeconds:      1 - maxTimeDriftSeconds,
 		CustomAuthSecretBytes: nil,
 		AuthOk:                true,
+		RetryAttempts:         5,
 	},
 	{
 		Name:                  "JWT Authentication: Positive time drift, exceeding limit, correct secret",
 		TimeDriftSeconds:      maxTimeDriftSeconds + 1,
 		CustomAuthSecretBytes: nil,
 		AuthOk:                false,
+		RetryAttempts:         5,
 	},
 	{
 		Name:                  "JWT Authentication: Positive time drift, within limit, correct secret",
 		TimeDriftSeconds:      maxTimeDriftSeconds - 1,
 		CustomAuthSecretBytes: nil,
 		AuthOk:                true,
+		RetryAttempts:         5,
 	},
 }
 
@@ -78,26 +83,38 @@ func GenerateAuthTestSpec(authTestSpec AuthTestSpec) TestSpec {
 				TerminalBlockNumber:     0,
 			}
 			testSecret = authTestSpec.CustomAuthSecretBytes
-			testTime   = time.Now()
+			// Time drift test cases are reattempted in order to mitigate false negatives
+			retryAttemptsLeft = authTestSpec.RetryAttempts
 		)
-		if testSecret == nil {
-			testSecret = defaultJwtTokenSecretBytes
-		}
-		if authTestSpec.TimeDriftSeconds != 0 {
-			testTime = testTime.Add(time.Second * time.Duration(authTestSpec.TimeDriftSeconds))
-		}
-		if err := t.Engine.PrepareAuthCallToken(testSecret, testTime); err != nil {
-			t.Fatalf("FAIL (%s): Unable to prepare the auth token: %v", t.TestName, err)
-		}
-		err := t.Engine.c.CallContext(t.Engine.Ctx(), &tConf, "engine_exchangeTransitionConfigurationV1", tConf)
-		if authTestSpec.AuthOk {
-			if err != nil {
-				t.Fatalf("FAIL (%s): Authentication was supposed to pass authentication but failed: %v", t.TestName, err)
+
+		for {
+			var testTime = time.Now()
+			if testSecret == nil {
+				testSecret = defaultJwtTokenSecretBytes
+			}
+			if authTestSpec.TimeDriftSeconds != 0 {
+				testTime = testTime.Add(time.Second * time.Duration(authTestSpec.TimeDriftSeconds))
+			}
+			if err := t.Engine.PrepareAuthCallToken(testSecret, testTime); err != nil {
+				t.Fatalf("FAIL (%s): Unable to prepare the auth token: %v", t.TestName, err)
+			}
+			err := t.Engine.c.CallContext(t.Engine.Ctx(), &tConf, "engine_exchangeTransitionConfigurationV1", tConf)
+			if (authTestSpec.AuthOk && err == nil) || (!authTestSpec.AuthOk && err != nil) {
+				// Test passed
+				return
 			}
-		} else {
-			if err == nil {
-				t.Fatalf("FAIL (%s): Authentication was supposed to fail authentication but passed", t.TestName)
+			if retryAttemptsLeft == 0 {
+				if err != nil {
+					// Test failed because unexpected error
+					t.Fatalf("FAIL (%s): Authentication was supposed to pass authentication but failed: %v", t.TestName, err)
+				} else {
+					// Test failed because unexpected success
+					t.Fatalf("FAIL (%s): Authentication was supposed to fail authentication but passed", t.TestName)
+				}
 			}
+			retryAttemptsLeft--
+			// Wait at least a second before trying again
+			time.Sleep(time.Second)
 		}
 	}
 	return TestSpec{

simulators/ethereum/engine/main.go

@@ -96,22 +96,40 @@ type TestSpec struct {
 	ChainFile string
 }
 
-var allTests = append(
-	engineTests,
-	append(
-		mergeTests,
-		authTests...,
-	)...,
-)
-
 func main() {
-	suite := hivesim.Suite{
+	engine := hivesim.Suite{
 		Name: "engine",
 		Description: `
 Test Engine API tests using CL mocker to inject commands into clients after they 
 have reached the Terminal Total Difficulty.`[1:],
 	}
-	for _, currentTest := range allTests {
+	addTestsToSuite(&engine, engineTests)
+
+	transition := hivesim.Suite{
+		Name: "transition",
+		Description: `
+Test Engine API tests using CL mocker to inject commands into clients and drive 
+them through the merge.`[1:],
+	}
+	addTestsToSuite(&transition, mergeTests)
+
+	auth := hivesim.Suite{
+		Name: "auth",
+		Description: `
+Test Engine API authentication features.`[1:],
+	}
+	addTestsToSuite(&auth, authTests)
+
+	// Mark suites for execution
+	simulator := hivesim.New()
+	hivesim.MustRunSuite(simulator, engine)
+	hivesim.MustRunSuite(simulator, transition)
+	hivesim.MustRunSuite(simulator, auth)
+}
+
+// Add test cases to a given test suite
+func addTestsToSuite(suite *hivesim.Suite, tests []TestSpec) {
+	for _, currentTest := range tests {
 		currentTest := currentTest
 		genesisPath := "./init/genesis.json"
 		// If the TestSpec specified a custom genesis file, use that instead.
@@ -137,9 +155,9 @@ have reached the Terminal Total Difficulty.`[1:],
 			Parameters:  newParams,
 			Files:       testFiles,
 			Run: func(t *hivesim.T, c *hivesim.Client) {
-				t.Logf("Start test: %s", currentTest.Name)
+				t.Logf("Start test (%s): %s", c.Type, currentTest.Name)
 				defer func() {
-					t.Logf("End test: %s", currentTest.Name)
+					t.Logf("End test (%s): %s", c.Type, currentTest.Name)
 				}()
 				timeout := DefaultTestCaseTimeout
 				// If a TestSpec specifies a timeout, use that instead
@@ -151,7 +169,6 @@ have reached the Terminal Total Difficulty.`[1:],
 			},
 		})
 	}
-	hivesim.MustRunSuite(hivesim.New(), suite)
 }
 
 // TTD is the value specified in the TestSpec + Genesis.Difficulty