Merge pull request #118 from harshanarayana/feature/git-112/enable-fr…

…amework-fail-fast-mode

GIT-112: enable framework specific fail-fast mode

docs/design/fail-fast-mode.md

@@ -0,0 +1,52 @@
+# Fail Fast Mode
+
+When writing test feature using `e2e-framework` it is possible that you can write test assessments grouped under the same feature that are dependent on each other.
+This is possible as the framework ensures that the assessments are processed in sequence of how they are registered. This key behavior bring up a need to be able to 
+optionally terminate the Feature(s) under test when a specific assessment fails. 
+
+## Why not use `test.failfast`?
+
+`go test` provides a handy way to terminate test execution of first sign of failure via the `-failfast` argument.
+However, this terminates the entire test suite in question.
+
+Such termination of the suite is not desirable for the framework as the rest of the Tests can still be processed
+in case if an assessment in one test fails. This bring in the need to introduce a framework specific `fail-fast`
+mode that can perform the following.
+
+1. It should Terminate the feature(s) under test and mark the test as failure
+2. Skip the Teardown workflow of the feature(s) under test to enable easy debug
+
+## Framework specific `--fail-fast` Mode
+
+`e2e-framework` introduces a new CLI argument flag that can be invoked while triggering the test to achieve the
+fail-fast behavior built into the framework.
+
+There are certain caveats to how this feature works. 
+
+1. The `fail-fast` mode doesn't work in conjunction with the `parallel` test mode
+2. Test developers have to explicitly invoke the `t.Fail()` or `t.FailNow()` handlers in the assessment to inform
+   the framework that the fail-fast mode needs to be triggered
+
+## Example Assessment
+Below section shows a simple example of how the feature can be leveraged in the assessment. This should be combined with `--fail-fast` argument while invoking the test to leverage the full feature.
+
+```go
+func TestFeatureOne(t *testing.T) {
+    featureOne := features.New("feature-one").
+        Assess("this fails", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			if 1 != 2 {
+				t.Log("1 != 2")
+				t.FailNow() // mark test case as failed here, don't continue execution
+			} else {
+				t.Log("1 == 2")
+			}
+			return ctx
+		}).
+        Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+			t.Log("This teardown should not be invoked")
+			return ctx
+		}).
+        Feature()
+    testenv.Test(t, failFeature, nextFeature)
+}
+```

examples/fail_fast/README.md

@@ -0,0 +1,134 @@
+# Fail Fast Mode
+
+There are times in your test infra that you want the rest of your feature assessments to fail in
+case if one of the current executing assessments fail.
+This can aid in getting a better turn around time especially in cases where your assessments are
+inter-related and a failed assessment in step 1 can mean that running the rest of the assessment
+is guaranteed to fail. This can be done with the help of a `--fail-fast` flag provided at the
+framework level.
+
+This works similar to how the `-failfast` mode of the `go test` works but provides the same
+feature at the context of the `e2e-framework`.
+
+# How to Use this feature ?
+
+1. Invoke the tests using `--fail-fast` argument
+2. Test developers should make sure they invoke either `t.Fail()` or `t.FailNow()` from the assessment to make sure the 
+additional handlers kick in to stop the test execution of the feature in question where the assessment has failed
+
+
+When the framework specific `--fail-fast` mode is used, this works as follows:
+
+1. It stops the rest of the assessments from getting executed for the feature under test
+2. This stops the next feature from getting executed in case if the feature under test fails as per step 1.
+3. Marks the feature and test associated with it as Failure.
+4. Skips the teardown sequence to make sure it is easier to debug the test failure
+
+> Current limitation is that this can't be combined with the `--parallel` switch
+
+Since this can lead to a test failure, we have just documented an example of this. Thanks to @divmadan for the example.
+
+```go
+// main_test.go
+package example
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"sigs.k8s.io/e2e-framework/pkg/env"
+)
+
+var testenv env.Environment
+
+func TestMain(m *testing.M) {
+	cfg, _ := envconf.NewFromFlags()
+	testenv = env.NewWithConfig(cfg)
+
+	os.Exit(testenv.Run(m))
+}
+```
+
+```go
+// example_test.go
+package example
+
+import (
+	"context"
+	"testing"
+
+	"sigs.k8s.io/e2e-framework/pkg/envconf"
+	"sigs.k8s.io/e2e-framework/pkg/features"
+)
+
+func TestExample(t *testing.T) {
+	failFeature := features.New("fail-feature").
+		Assess("1==2", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			if 1 != 2 {
+				t.Log("1 != 2")
+				t.FailNow() // mark test case as failed here, don't continue execution
+			} else {
+				t.Log("1 == 2")
+			}
+			return ctx
+		}).
+		Assess("print", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			t.Log("THIS LINE SHOULDN'T BE PRINTED")
+			return ctx
+		}).
+		Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+			t.Log("This teardown should not be invoked")
+			return ctx
+		}).
+ 		Feature()
+
+	nextFeature := features.New("next-feature").
+		Assess("print", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			t.Log("THIS LINE ALSO SHOULDN'T BE PRINTED")
+			return ctx
+		}).
+		Feature()
+
+	testenv.Test(t, failFeature, nextFeature)
+}
+
+// even if the previous testcase fails, execute this testcase
+func TestNext(t *testing.T) {
+	nextFeature := features.New("next-test-feature").
+		Assess("print", func(ctx context.Context, t *testing.T, cfg *envconf.Config) context.Context {
+			t.Log("THIS LINE SHOULD BE PRINTED")
+			return ctx
+		}).
+		Teardown(func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+			t.Log("This teardown should be invoked")
+			return ctx
+		}).
+		Feature()
+
+	testenv.Test(t, nextFeature)
+}
+```
+
+When run this using `--fail-fast` you get the following behavior.
+
+```bash
+❯ go test . -test.v -args --fail-fast
+
+=== RUN   TestExample
+=== RUN   TestExample/fail-feature
+=== RUN   TestExample/fail-feature/1==2
+    example_test.go:15: 1 != 2
+--- FAIL: TestExample (0.00s)
+    --- FAIL: TestExample/fail-feature (0.00s)
+        --- FAIL: TestExample/fail-feature/1==2 (0.00s)
+=== RUN   TestNext
+=== RUN   TestNext/next-test-feature
+=== RUN   TestNext/next-test-feature/print
+    example_test.go:42: THIS LINE SHOULD BE PRINTED
+--- PASS: TestNext (0.00s)
+    --- PASS: TestNext/next-test-feature (0.00s)
+        --- PASS: TestNext/next-test-feature/print (0.00s)
+FAIL
+```

pkg/env/env.go

@@ -260,6 +260,11 @@ func (e *testEnv) processTests(t *testing.T, enableParallelRun bool, testFeature
 			}(&wg, featName, featureCopy)
 		} else {
 			e.processTestFeature(t, featName, featureCopy)
+			// In case if the feature under test has failed, skip reset of the features
+			// that are part of the same test
+			if e.cfg.FailFast() && t.Failed() {
+				break
+			}
 		}
 	}
 	if runInParallel {
@@ -417,6 +422,7 @@ func (e *testEnv) execFeature(ctx context.Context, t *testing.T, featName string
 		// assessments run as feature/assessment sub level
 		assessments := features.GetStepsByLevel(f.Steps(), types.LevelAssess)
 
+		failed := false
 		for i, assess := range assessments {
 			assessName := assess.Name()
 			if assessName == "" {
@@ -429,6 +435,20 @@ func (e *testEnv) execFeature(ctx context.Context, t *testing.T, featName string
 				}
 				ctx = e.executeSteps(ctx, t, []types.Step{assess})
 			})
+			// Check if the Test assessment under question performed a `t.Fail()` or `t.Failed()` invocation.
+			// We need to track that and stop the next set of assessment in the feature under test from getting
+			// executed
+			if e.cfg.FailFast() && t.Failed() {
+				failed = true
+				break
+			}
+		}
+
+		// Let us fail the test fast and not run the teardown in case if the framework specific fail-fast mode is
+		// invoked to make sure we leave the traces of the failed test behind to enable better debugging for the
+		// test developers
+		if e.cfg.FailFast() && failed {
+			t.FailNow()
 		}
 
 		// teardowns run at feature-level

pkg/envconf/config.go

@@ -42,6 +42,7 @@ type Config struct {
 	skipAssessmentRegex *regexp.Regexp
 	parallelTests       bool
 	dryRun              bool
+	failFast            bool
 }
 
 // New creates and initializes an empty environment configuration
@@ -81,6 +82,7 @@ func NewFromFlags() (*Config, error) {
 	e.skipLabels = envFlags.SkipLabels()
 	e.parallelTests = envFlags.Parallel()
 	e.dryRun = envFlags.DryRun()
+	e.failFast = envFlags.FailFast()
 
 	return e, nil
 }
@@ -220,11 +222,15 @@ func (c *Config) SkipLabels() map[string]string {
 	return c.skipLabels
 }
 
+// WithParallelTestEnabled can be used to enable parallel run of the test
+// features
 func (c *Config) WithParallelTestEnabled() *Config {
 	c.parallelTests = true
 	return c
 }
 
+// ParallelTestEnabled indicates if the test features are being run in
+// parallel or not
 func (c *Config) ParallelTestEnabled() bool {
 	return c.parallelTests
 }
@@ -238,6 +244,21 @@ func (c *Config) DryRunMode() bool {
 	return c.dryRun
 }
 
+// WithFailFast can be used to enable framework specific fail fast mode
+// that controls the test execution of the features and assessments under
+// test
+func (c *Config) WithFailFast() *Config {
+	c.failFast = true
+	return c
+}
+
+// FailFast indicate if the framework is running in fail fast mode. This
+// controls the behavior of how the assessments and features are handled
+// if a test encounters a failure result
+func (c *Config) FailFast() bool {
+	return c.failFast
+}
+
 func randNS() string {
 	return RandomName("testns-", 32)
 }

pkg/envconf/config_test.go

@@ -61,3 +61,15 @@ func TestConfig_New_WithDryRun(t *testing.T) {
 		t.Errorf("expected dryRun mode to be enabled with invoked with --dry-run arguments")
 	}
 }
+
+func TestConfig_New_WithFailFastAndIgnoreFinalize(t *testing.T) {
+	flag.CommandLine = &flag.FlagSet{}
+	os.Args = []string{"test-binary", "-fail-fast"}
+	cfg, err := NewFromFlags()
+	if err != nil {
+		t.Error("failed to parse args", err)
+	}
+	if !cfg.FailFast() {
+		t.Error("expected fail-fast mode to be enabled when -fail-fast argument is passed")
+	}
+}