Merge pull request #162 from mhofstetter/feature/actionrole-logmessage

Fix log output on errors during test lifecycle actions

Author: k8s-ci-robot

pkg/env/action.go

@@ -26,15 +26,36 @@ import (
 	"sigs.k8s.io/e2e-framework/pkg/internal/types"
 )
 
+type actionRole uint8
+
 const (
-	roleSetup = iota
+	roleSetup actionRole = iota
 	roleBeforeTest
 	roleBeforeFeature
 	roleAfterFeature
 	roleAfterTest
 	roleFinish
 )
 
+func (r actionRole) String() string {
+	switch r {
+	case roleSetup:
+		return "Setup"
+	case roleBeforeTest:
+		return "BeforeEachTest"
+	case roleBeforeFeature:
+		return "BeforeEachFeature"
+	case roleAfterFeature:
+		return "AfterEachFeature"
+	case roleAfterTest:
+		return "AfterEachTest"
+	case roleFinish:
+		return "Finish"
+	default:
+		panic("unknown role") // this should never happen
+	}
+}
+
 // action a group env functions
 type action struct {
 	role actionRole

pkg/env/action_test.go

@@ -104,3 +104,59 @@ func TestAction_Run(t *testing.T) {
 		})
 	}
 }
+
+func TestActionRole_String(t *testing.T) {
+	tests := []struct {
+		name string
+		r    actionRole
+		want string
+	}{
+		{
+			name: "RoleSetup",
+			r:    roleSetup,
+			want: "Setup",
+		},
+		{
+			name: "RoleBeforeTest",
+			r:    roleBeforeTest,
+			want: "BeforeEachTest",
+		},
+		{
+			name: "RoleBeforeFeature",
+			r:    roleBeforeFeature,
+			want: "BeforeEachFeature",
+		},
+		{
+			name: "RoleAfterEachFeature",
+			r:    roleAfterFeature,
+			want: "AfterEachFeature",
+		},
+		{
+			name: "RoleAfterTest",
+			r:    roleAfterTest,
+			want: "AfterEachTest",
+		},
+		{
+			name: "RoleFinish",
+			r:    roleFinish,
+			want: "Finish",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := test.r.String(); got != test.want {
+				t.Errorf("String() = %v, want %v", got, test.want)
+			}
+		})
+	}
+}
+
+func TestActionRole_String_Unknown(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Error("Unknown ActionRole should panic")
+		}
+	}()
+
+	_ = actionRole(100).String()
+}

pkg/env/env.go

@@ -38,8 +38,6 @@ type (
 	Environment = types.Environment
 	Func        = types.EnvFunc
 	FeatureFunc = types.FeatureEnvFunc
-
-	actionRole uint8
 )
 
 type testEnv struct {
@@ -187,7 +185,7 @@ func (e *testEnv) processTestActions(t *testing.T, actions []action) {
 	var err error
 	for _, action := range actions {
 		if e.ctx, err = action.runWithT(e.ctx, e.cfg, t); err != nil {
-			t.Fatalf("BeforeEachTest failure: %s", err)
+			t.Fatalf("%s failure: %s", action.role, err)
 		}
 	}
 }
@@ -196,25 +194,23 @@ func (e *testEnv) processTestActions(t *testing.T, actions []action) {
 // workflow of orchestrating the feature execution be running the action configured by BeforeEachFeature /
 // AfterEachFeature.
 func (e *testEnv) processTestFeature(t *testing.T, featureName string, feature types.Feature) {
-	var err error
-
-	// execute each feature
-	beforeFeatureActions := e.getBeforeFeatureActions()
-	afterFeatureActions := e.getAfterFeatureActions()
-
-	for _, action := range beforeFeatureActions {
-		if e.ctx, err = action.runWithFeature(e.ctx, e.cfg, t, deepCopyFeature(feature)); err != nil {
-			t.Fatalf("BeforeEachTest failure: %s", err)
-		}
-	}
+	// execute beforeEachFeature actions
+	e.processFeatureActions(t, feature, e.getBeforeFeatureActions())
 
 	// execute feature test
 	e.ctx = e.execFeature(e.ctx, t, featureName, feature)
 
-	// execute beforeFeature actions
-	for _, action := range afterFeatureActions {
+	// execute afterEachFeature actions
+	e.processFeatureActions(t, feature, e.getAfterFeatureActions())
+}
+
+// processFeatureActions is used to run a series of feature action that were configured as
+// BeforeEachFeature or AfterEachFeature
+func (e *testEnv) processFeatureActions(t *testing.T, feature types.Feature, actions []action) {
+	var err error
+	for _, action := range actions {
 		if e.ctx, err = action.runWithFeature(e.ctx, e.cfg, t, deepCopyFeature(feature)); err != nil {
-			t.Fatalf("BeforeEachTest failure: %s", err)
+			t.Fatalf("%s failure: %s", action.role, err)
 		}
 	}
 }
@@ -327,7 +323,6 @@ func (e *testEnv) Finish(funcs ...Func) types.Environment {
 // package.  This method will all Env.Setup operations prior to
 // starting the tests and run all Env.Finish operations after
 // before completing the suite.
-//
 func (e *testEnv) Run(m *testing.M) int {
 	if e.ctx == nil {
 		panic("context not set") // something is terribly wrong.
@@ -354,15 +349,15 @@ func (e *testEnv) Run(m *testing.M) int {
 		for _, fin := range finishes {
 			// context passed down to each finish step
 			if e.ctx, err = fin.run(e.ctx, e.cfg); err != nil {
-				klog.V(2).ErrorS(err, "Finish action handlers")
+				klog.V(2).ErrorS(err, "Cleanup failed", "action", fin.role)
 			}
 		}
 	}()
 
 	for _, setup := range setups {
 		// context passed down to each setup
 		if e.ctx, err = setup.run(e.ctx, e.cfg); err != nil {
-			klog.Fatal(err)
+			klog.Fatalf("%s failure: %s", setup.role, err)
 		}
 	}