Repository: go-bdd/gobdd (go)

Issue:
Do not require returning the context from a step
**Is your feature request related to a problem? Please describe.**
Returning the context seems unnecessary (since it's internal state is mutated) and it clutters the code.

**Describe the solution you'd like**
Accept the context in the step function parameter list, but don't require returning it.

**Describe alternatives you've considered**
This would make sense if the context was immutable, but it's not.

**Additional context**
Based on our conversation on slack with @bkielbasa I felt like there is a misunderstanding:

> the context is the holder of the state. It has to be kept somewhere so to avoid global variables I decided to do this way. Do you have any better idea?

Accepting the context as a parameter is OK, having to return it as well seems unnecessary.


Pull request description:
Remove context from step return values
This PR removes the context from the return parameters for step functions.

As explained in #98, the underlying map in `context.Context` is already accessed by reference, so there is no need to return the context from step functions.

This patch is backwards compatible in the sense that step functions returning the context will continue to work (proven by tests), but the current implementation also allows overwriting the context entirely that is no longer possible with this patch, which technically makes it a BC break. However, I don't think overwriting the context is the desired behavior in this case.

The current set of tests proves that there is no need to return the context, subsequent steps can access values from the context set by earlier steps.

Fixes #98

Patch:
diff --git a/docs/creating-steps.markdown b/docs/creating-steps.markdown
index e86b5e7..193c9fb 100644
--- a/docs/creating-steps.markdown
+++ b/docs/creating-steps.markdown
@@ -8,7 +8,7 @@ title: Creating steps
 Every step function should accept the `StepTest` as the first 2 parameters and returns the `Context`. Here's an example:
 
 ```go
-type StepFunc func(gobdd.StepTest, ctx context.Context, var1 int, var2 string) context.Context
+type StepFunc func(gobdd.StepTest, ctx context.Context, var1 int, var2 string)
 ```
 
 What's important to stress - the context is a [custom struct](https://github.com/go-bdd/gobdd/tree/master/context), not the built-in interface.
diff --git a/docs/quick-start.markdown b/docs/quick-start.markdown
index fb46c79..bb932c2 100644
--- a/docs/quick-start.markdown
+++ b/docs/quick-start.markdown
@@ -12,24 +12,22 @@ go get github.com/go-bdd/gobdd
 Add a new test `main_test.go`:
 
 ```go
-func add(t gobdd.StepTest, ctx context.Context, var1, var2 int) context.Context{
+func add(t gobdd.StepTest, ctx context.Context, var1, var2 int) {
 	res := var1 + var2
 	ctx.Set("sumRes", res)
-	return ctx
 }
 
-func check(t gobdd.StepTest, ctx context.Context, sum int) context.Context{
+func check(t gobdd.StepTest, ctx context.Context, sum int) {
 	received, err := ctx.GetInt("sumRes")
     if err != nil {
         t.Error(err)
-        return ctx
+
+        return
     }
 
 	if sum != received {
         t.Error(errors.New("the math does not work for you"))
 	}
-
-	return ctx
 }
 
 func TestScenarios(t *testing.T) {
diff --git a/gobdd.go b/gobdd.go
index 520812c..d9044d8 100644
--- a/gobdd.go
+++ b/gobdd.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/cucumber/gherkin-go/v9"
 	msgs "github.com/cucumber/messages-go/v9"
+
 	"github.com/go-bdd/gobdd/context"
 )
 
@@ -135,11 +136,9 @@ func NewSuite(t TestingT, optionClosures ...func(*SuiteOptions)) *Suite {
 // when a step definition matches the provided regular expression.
 //
 // A step function can have any number of parameters (even zero),
-// but it MUST accept a gobdd.StepTest and context.Context as the first parameters (if there is any)
-// and MUST return the context.Context back:
+// but it MUST accept a gobdd.StepTest and context.Context as the first parameters (if there is any):
 //
-// 	func myStepFunction(t gobdd.StepTest, ctx context.Context, first int, second int) context.Context {
-// 		return ctx
+// 	func myStepFunction(t gobdd.StepTest, ctx context.Context, first int, second int) {
 // 	}
 func (s *Suite) AddStep(expr string, step interface{}) {
 	err := validateStepFunc(step)
@@ -170,11 +169,9 @@ func (s *Suite) AddStep(expr string, step interface{}) {
 // when a step definition matches the provided regular expression.
 //
 // A step function can have any number of parameters (even zero),
-// but it MUST accept a gobdd.StepTest and context.Context as the first parameters (if there is any)
-// and MUST return the context.Context back:
+// but it MUST accept a gobdd.StepTest and context.Context as the first parameters (if there is any):
 //
-// 	func myStepFunction(t gobdd.StepTest, ctx context.Context, first int, second int) context.Context {
-// 		return ctx
+// 	func myStepFunction(t gobdd.StepTest, ctx context.Context, first int, second int) {
 // 	}
 func (s *Suite) AddRegexStep(expr *regexp.Regexp, step interface{}) {
 	err := validateStepFunc(step)
@@ -349,7 +346,7 @@ func (s *Suite) runScenario(ctx context.Context, scenario *msgs.GherkinDocument_
 	t.Run(scenario.Name, func(t *testing.T) {
 		if bkg != nil {
 			steps := s.getBackgroundSteps(bkg)
-			ctx = s.runSteps(ctx, t, steps)
+			s.runSteps(ctx, t, steps)
 		}
 		steps := scenario.Steps
 		if examples := scenario.GetExamples(); len(examples) > 0 {
@@ -359,16 +356,13 @@ func (s *Suite) runScenario(ctx context.Context, scenario *msgs.GherkinDocument_
 	})
 }
 
-func (s *Suite) runSteps(ctx context.Context, t *testing.T,
-	steps []*msgs.GherkinDocument_Feature_Step) context.Context {
+func (s *Suite) runSteps(ctx context.Context, t *testing.T, steps []*msgs.GherkinDocument_Feature_Step) {
 	for _, step := range steps {
-		ctx = s.runStep(ctx, t, step)
+		s.runStep(ctx, t, step)
 	}
-
-	return ctx
 }
 
-func (s *Suite) runStep(ctx context.Context, t *testing.T, step *msgs.GherkinDocument_Feature_Step) context.Context {
+func (s *Suite) runStep(ctx context.Context, t *testing.T, step *msgs.GherkinDocument_Feature_Step) {
 	defer func() {
 		if r := recover(); r != nil {
 			t.Error(r)
@@ -378,18 +372,17 @@ func (s *Suite) runStep(ctx context.Context, t *testing.T, step *msgs.GherkinDoc
 	def, err := s.findStepDef(step.Text)
 	if err != nil {
 		t.Errorf("cannot find step definition for step: %s%s", step.Keyword, step.Text)
-		return ctx
+
+		return
 	}
 
 	params := def.expr.FindSubmatch([]byte(step.Text))[1:]
 	t.Run(step.Text, func(t *testing.T) {
-		ctx = def.run(ctx, t, params)
+		def.run(ctx, t, params)
 	})
-
-	return ctx
 }
 
-func (def *stepDef) run(ctx context.Context, t TestingT, params [][]byte) context.Context {
+func (def *stepDef) run(ctx context.Context, t TestingT, params [][]byte) {
 	defer func() {
 		if r := recover(); r != nil {
 			t.Errorf("%+v", r)
@@ -399,7 +392,8 @@ func (def *stepDef) run(ctx context.Context, t TestingT, params [][]byte) contex
 	d := reflect.ValueOf(def.f)
 	if len(params)+2 != d.Type().NumIn() {
 		t.Errorf("the step function %s accepts %d arguments but %d received", d.String(), d.Type().NumIn(), len(params)+2)
-		return ctx
+
+		return
 	}
 
 	in := []reflect.Value{reflect.ValueOf(t), reflect.ValueOf(ctx)}
@@ -414,9 +408,7 @@ func (def *stepDef) run(ctx context.Context, t TestingT, params [][]byte) contex
 		in = append(in, paramType)
 	}
 
-	retValues := d.Call(in)
-
-	return retValues[0].Interface().(context.Context)
+	d.Call(in)
 }
 
 func paramType(param []byte, inType reflect.Type) reflect.Value {
diff --git a/steps.go b/steps.go
index bb8e178..044fae2 100644
--- a/steps.go
+++ b/steps.go
@@ -13,22 +13,11 @@ func validateStepFunc(f interface{}) error {
 		return errors.New("the parameter should be a function")
 	}
 
-	if value.Type().NumOut() != 1 {
-		return errors.New("the function should return the context.Context")
-	}
-
-	val := value.Type().Out(0)
-
-	n := val.ConvertibleTo(reflect.TypeOf((*context.Context)(nil)).Elem())
-	if !n {
-		return errors.New("the returned value should implement the context.Context interface")
-	}
-
 	if value.Type().NumIn() < 2 {
 		return errors.New("the function should have StepTest and Context as the first argument")
 	}
 
-	val = value.Type().In(0)
+	val := value.Type().In(0)
 
 	testingInterface := reflect.TypeOf((*StepTest)(nil)).Elem()
 	if !val.Implements(testingInterface) {
@@ -37,7 +26,7 @@ func validateStepFunc(f interface{}) error {
 
 	val = value.Type().In(1)
 
-	n = val.ConvertibleTo(reflect.TypeOf((*context.Context)(nil)).Elem())
+	n := val.ConvertibleTo(reflect.TypeOf((*context.Context)(nil)).Elem())
 	if !n {
 		return errors.New("the function should have Context as the second argument")
 	}


Test patch:
diff --git a/gobdd_test.go b/gobdd_test.go
index a8645c9..11ee2cf 100644
--- a/gobdd_test.go
+++ b/gobdd_test.go
@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/go-bdd/assert"
+
 	"github.com/go-bdd/gobdd/context"
 )
 
@@ -140,66 +141,57 @@ func TestFailureOutput(t *testing.T) {
 	}
 }
 
-func addf(t StepTest, ctx context.Context, var1, var2 float32) context.Context {
+func addf(_ StepTest, ctx context.Context, var1, var2 float32) {
 	res := var1 + var2
 	ctx.Set("sumRes", res)
-
-	return ctx
 }
 
-func add(t StepTest, ctx context.Context, var1, var2 int) context.Context {
+func add(_ StepTest, ctx context.Context, var1, var2 int) {
 	res := var1 + var2
 	ctx.Set("sumRes", res)
-
-	return ctx
 }
 
-func checkf(t StepTest, ctx context.Context, sum float32) context.Context {
+func checkf(t StepTest, ctx context.Context, sum float32) {
 	received, err := ctx.Get("sumRes")
 	if err != nil {
 		t.Error(err.Error())
-		return ctx
+
+		return
 	}
 
 	if sum != received {
 		t.Error("the sum doesn't match")
 	}
-
-	return ctx
 }
 
-func check(t StepTest, ctx context.Context, sum int) context.Context {
+func check(t StepTest, ctx context.Context, sum int) {
 	received, err := ctx.Get("sumRes")
 	if err != nil {
 		t.Error(err.Error())
-		return ctx
+
+		return
 	}
 
 	if sum != received {
 		t.Error("the math does not work for you")
-		return ctx
-	}
 
-	return ctx
+		return
+	}
 }
 
-func fail(t StepTest, ctx context.Context) context.Context {
+func fail(t StepTest, _ context.Context) {
 	t.Error("the step should never be executed")
-	return ctx
 }
 
-func failure(t StepTest, ctx context.Context) context.Context {
+func failure(t StepTest, _ context.Context) {
 	t.Error("the step failed")
-	return ctx
 }
 
-func panics(t StepTest, _ context.Context) context.Context {
+func panics(_ StepTest, _ context.Context) {
 	panic(errors.New("the step panicked"))
 }
 
-func pass(t StepTest, ctx context.Context) context.Context {
-	return ctx
-}
+func pass(_ StepTest, _ context.Context) {}
 
 type mockTester struct {
 	fatalCalled int
diff --git a/steps_test.go b/steps_test.go
index 40ccd18..809ec5e 100644
--- a/steps_test.go
+++ b/steps_test.go
@@ -8,11 +8,9 @@ import (
 
 func TestValidateStepFunc(t *testing.T) {
 	testCases := map[string]interface{}{
-		"function without arguments":                 func() context.Context { return context.Context{} },
-		"function with 1 argument":                   func(StepTest) context.Context { return context.Context{} },
-		"function with invalid first argument":       func(int, context.Context) context.Context { return context.Context{} },
-		"function without returned values":           func(StepTest, context.Context) {},
-		"function with invalid first returned value": func(StepTest, context.Context) int { return 0 },
+		"function without arguments":           func() {},
+		"function with 1 argument":             func(StepTest) {},
+		"function with invalid first argument": func(int, context.Context) {},
 	}
 
 	for name, testCase := range testCases {
@@ -25,7 +23,13 @@ func TestValidateStepFunc(t *testing.T) {
 }
 
 func TestValidateStepFunc_ValidFunction(t *testing.T) {
-	if err := validateStepFunc(func(StepTest, context.Context) context.Context { return context.Context{} }); err != nil {
+	if err := validateStepFunc(func(StepTest, context.Context) {}); err != nil {
 		t.Errorf("the test should NOT fail for the function: %s", err)
 	}
 }
+
+func TestValidateStepFunc_ReturnContext(t *testing.T) {
+	if err := validateStepFunc(func(StepTest, context.Context) context.Context { return context.Context{} }); err != nil {
+		t.Errorf("step function returning a context should NOT fail validation: %s", err)
+	}
+}