Improve e2e logging

When a test fails, we the go test framework logs the failure in an
unhelpful way. This works for unit tests, since they are simple and do
not log anything, but our tests are complex and log a lot. The failure
logs from go test are hidden and hard to find.

This change add wrappers for t.Fatal, t.FailNow, t.Skip, and t.Skipf so
they use our logger instead of the go test framework logger. This
ensures that every failure will have a proper ERROR log which is very
easy to find. Errors logs include a traceback making it easier to
understand the failure.

When handling errors in parent tests we don't have any context and we
cannot log any useful error. The actual error that failed the sub test
already logged the error, so we just mark the test as failed with
util.FailNow().

Part-of: RamenDR#1595
Signed-off-by: Nir Soffer <[email protected]>

e2e/actions_test.go

@@ -8,70 +8,71 @@ import (
 
 	"github.com/ramendr/ramen/e2e/dractions"
 	"github.com/ramendr/ramen/e2e/testcontext"
+	"github.com/ramendr/ramen/e2e/util"
 )
 
 func DeployAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if err := testCtx.Deployer.Deploy(testCtx.Workload); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Deploy failed", err)
 	}
 }
 
 func EnableAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if err := dractions.EnableProtection(testCtx.Workload, testCtx.Deployer); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Enable failed", err)
 	}
 }
 
 func FailoverAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if err := dractions.Failover(testCtx.Workload, testCtx.Deployer); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failover failed", err)
 	}
 }
 
 func RelocateAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if err := dractions.Relocate(testCtx.Workload, testCtx.Deployer); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Relocate failed", err)
 	}
 }
 
 func DisableAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if err := dractions.DisableProtection(testCtx.Workload, testCtx.Deployer); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Disable failed", err)
 	}
 }
 
 func UndeployAction(t *testing.T) {
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Error(err)
+		util.Fatal(t, "Disable failed", err)
 	}
 
 	if err := testCtx.Deployer.Undeploy(testCtx.Workload); err != nil {
-		t.Error(err)
+		util.Fatal(t, "Undeploy failed", err)
 	}
 }

e2e/exhaustive_suite_test.go

@@ -67,12 +67,12 @@ func Exhaustive(t *testing.T) {
 	t.Parallel()
 
 	if err := util.EnsureChannel(); err != nil {
-		t.Fatalf("failed to ensure channel: %v", err)
+		util.Fatal(t, "Failed to ensure channel", err)
 	}
 
 	t.Cleanup(func() {
 		if err := util.EnsureChannelDeleted(); err != nil {
-			t.Fatalf("failed to ensure channel deleted: %v", err)
+			util.Fatal(t, "Failed to ensure channel deleted", err)
 		}
 	})
 
@@ -103,34 +103,34 @@ func runTestFlow(t *testing.T) {
 
 	testCtx, err := testcontext.GetTestContext(t.Name())
 	if err != nil {
-		t.Fatal(err)
+		util.Fatal(t, "Failed to get test context", err)
 	}
 
 	if !testCtx.Deployer.IsWorkloadSupported(testCtx.Workload) {
-		t.Skipf("Workload %s not supported by deployer %s, skip test", testCtx.Workload.GetName(), testCtx.Deployer.GetName())
+		util.Skipf(t, "Workload %s not supported by deployer %s", testCtx.Workload.GetName(), testCtx.Deployer.GetName())
 	}
 
 	if !t.Run("Deploy", DeployAction) {
-		t.Fatal("Deploy failed")
+		util.FailNow(t)
 	}
 
 	if !t.Run("Enable", EnableAction) {
-		t.Fatal("Enable failed")
+		util.FailNow(t)
 	}
 
 	if !t.Run("Failover", FailoverAction) {
-		t.Fatal("Failover failed")
+		util.FailNow(t)
 	}
 
 	if !t.Run("Relocate", RelocateAction) {
-		t.Fatal("Relocate failed")
+		util.FailNow(t)
 	}
 
 	if !t.Run("Disable", DisableAction) {
-		t.Fatal("Disable failed")
+		util.FailNow(t)
 	}
 
 	if !t.Run("Undeploy", UndeployAction) {
-		t.Fatal("Undeploy failed")
+		util.FailNow(t)
 	}
 }

e2e/util/testing.go

@@ -0,0 +1,35 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
+package util
+
+import (
+	"fmt"
+	"testing"
+)
+
+// Fatal logs an error and fails the test.
+func Fatal(t *testing.T, msg string, err error) {
+	t.Helper()
+	Ctx.Log.Error(err, msg)
+	t.FailNow()
+}
+
+// FailNow fails the tests silently. Use for parent tests.
+func FailNow(t *testing.T) {
+	t.Helper()
+	t.FailNow()
+}
+
+// Skipf logs formatted message the skips the test.
+func Skipf(t *testing.T, format string, args ...any) {
+	t.Helper()
+	Skip(t, fmt.Sprintf(format, args...))
+}
+
+// Skip log msg and skips the test.
+func Skip(t *testing.T, msg string) {
+	t.Helper()
+	Ctx.Log.Info(msg)
+	t.SkipNow()
+}

e2e/validation_suite_test.go

@@ -14,19 +14,19 @@ func Validate(t *testing.T) {
 	t.Run("hub", func(t *testing.T) {
 		err := util.ValidateRamenHubOperator(util.Ctx.Hub.K8sClientSet)
 		if err != nil {
-			t.Fatal(err)
+			util.Fatal(t, "Failed to validate hub", err)
 		}
 	})
 	t.Run("c1", func(t *testing.T) {
 		err := util.ValidateRamenDRClusterOperator(util.Ctx.C1.K8sClientSet, "c1")
 		if err != nil {
-			t.Fatal(err)
+			util.Fatal(t, "Failed to validate cluster c1", err)
 		}
 	})
 	t.Run("c2", func(t *testing.T) {
 		err := util.ValidateRamenDRClusterOperator(util.Ctx.C2.K8sClientSet, "c2")
 		if err != nil {
-			t.Fatal(err)
+			util.Fatal(t, "Failed to validate cluster c2", err)
 		}
 	})
 }