Repository: go-bdd/gobdd (go)

Issue:
Catching invalid step functions
**Is your feature request related to a problem? Please describe.**
Currently `suite.AddStep` returns an error which I could check if I want to handle invalid step definitions. This is a bit counterintuitive, because I certainly don't want to check them one by one.

**Describe the solution you'd like**
Ideally, the framework should probably check all step functions at once when calling `suite.Run`. I would like to see all invalid functions.

If it already does, then I would remove the returned error from `suite.AddStep` to have a cleaner API.

**Describe alternatives you've considered**
Add a `suite.MustAddStep` that terminates the test (`*testing.T` is already available at this point), but I think it would be better to see all invalid step functions.


Pull request description:
Catching invalid step functions
Fixes #48 
FYI @sagikazarmark 

Patch:
diff --git a/README.md b/README.md
index 830dbc0..961d505 100644
--- a/README.md
+++ b/README.md
@@ -14,3 +14,5 @@ If you didn't find the answer to your question in the documentation, feel free t
 
 Please join us on the `#go-bdd-library` channel on the [Gophers slack](https://gophers.slack.com/): You can get [an invite here](https://gophersinvite.herokuapp.com/).
 You can find updates about the progress on Twiter: [GoBdd](https://twitter.com/Go_BDD).
+
+You can support my work using [issuehunt](https://issuehunt.io/r/go-bdd) or by [buying me a coffee](https://www.buymeacoffee.com/bklimczak).
\ No newline at end of file
diff --git a/gobdd.go b/gobdd.go
index 022a830..47fd4ee 100644
--- a/gobdd.go
+++ b/gobdd.go
@@ -21,9 +21,10 @@ import (
 
 // Holds all the information about the suite (options, steps to execute etc)
 type Suite struct {
-	t       *testing.T
-	steps   []stepDef
-	options SuiteOptions
+	t           TestingT
+	steps       []stepDef
+	options     SuiteOptions
+	stepsErrors []error
 }
 
 // Holds all the information about how the suite or features/steps should be configured
@@ -91,8 +92,17 @@ type stepDef struct {
 	f    interface{}
 }
 
+type TestingT interface {
+	Log(...interface{})
+	Fatal(...interface{})
+	Fatalf(string, ...interface{})
+	Parallel()
+	Fail()
+	Run(name string, f func(t *testing.T)) bool
+}
+
 // Creates a new suites with given configuration and empty steps defined
-func NewSuite(t *testing.T, options SuiteOptions) *Suite {
+func NewSuite(t TestingT, options SuiteOptions) *Suite {
 	return &Suite{
 		t:       t,
 		steps:   []stepDef{},
@@ -112,18 +122,23 @@ func NewSuite(t *testing.T, options SuiteOptions) *Suite {
 // 	func myStepFunction(ctx context.Context, first int, second int) (context.Context, error) {
 // 		return ctx, nil
 // 	}
-func (s *Suite) AddStep(expr string, step interface{}) error {
+func (s *Suite) AddStep(expr string, step interface{}) {
 	err := validateStepFunc(step)
 	if err != nil {
-		return err
+		s.stepsErrors = append(s.stepsErrors, err)
+		return
+	}
+
+	compiled, err := regexp.Compile(expr)
+	if err != nil {
+		s.stepsErrors = append(s.stepsErrors, err)
+		return
 	}
 
 	s.steps = append(s.steps, stepDef{
-		expr: regexp.MustCompile(expr),
+		expr: compiled,
 		f:    step,
 	})
-
-	return nil
 }
 
 // AddRegexStep registers a step in the suite.
@@ -138,22 +153,29 @@ func (s *Suite) AddStep(expr string, step interface{}) error {
 // 	func myStepFunction(ctx context.Context, first int, second int) (context.Context, error) {
 // 		return ctx, nil
 // 	}
-func (s *Suite) AddRegexStep(expr *regexp.Regexp, step interface{}) error {
+func (s *Suite) AddRegexStep(expr *regexp.Regexp, step interface{}) {
 	err := validateStepFunc(step)
 	if err != nil {
-		return err
+		s.stepsErrors = append(s.stepsErrors, err)
+		return
 	}
 
 	s.steps = append(s.steps, stepDef{
 		expr: expr,
 		f:    step,
 	})
-
-	return nil
 }
 
 // Executes the suite with given options and defined steps
 func (s *Suite) Run() {
+	if len(s.stepsErrors) > 0 {
+		for _, err := range s.stepsErrors {
+			s.t.Log(err)
+		}
+		s.t.Fatal("the test contains invalid step definitions")
+		return
+	}
+
 	files, err := filepath.Glob(s.options.featuresPaths)
 	if err != nil {
 		s.t.Fatalf("cannot find features/ directory")
@@ -260,11 +282,7 @@ func (s *Suite) stepsFromExample(sourceStep *messages.GherkinDocument_Feature_St
 			}
 
 			expr := strings.Replace(def.expr.String(), ph, t, -1)
-			err = s.AddStep(expr, def.f)
-
-			if err != nil {
-				continue
-			}
+			s.AddStep(expr, def.f)
 		}
 	}
 


Test patch:
diff --git a/gobdd_test.go b/gobdd_test.go
index 78f1281..b57665e 100644
--- a/gobdd_test.go
+++ b/gobdd_test.go
@@ -4,49 +4,30 @@ import (
 	"context"
 	"errors"
 	"testing"
+
+	"github.com/go-bdd/assert"
 )
 
 func TestScenarios(t *testing.T) {
 	suite := NewSuite(t, NewSuiteOptions().WithFeaturesPath("features/example.feature"))
-	err := suite.AddStep(`I add (\d+) and (\d+)`, add)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = suite.AddStep(`the result should equal (\d+)`, check)
-	if err != nil {
-		t.Fatal(err)
-	}
+	suite.AddStep(`I add (\d+) and (\d+)`, add)
+	suite.AddStep(`the result should equal (\d+)`, check)
 
 	suite.Run()
 }
 
 func TestDifferentFuncTypes(t *testing.T) {
 	suite := NewSuite(t, NewSuiteOptions().WithFeaturesPath("features/func_types.feature"))
-	err := suite.AddStep(`I add ([+-]?[0-9]*[.]?[0-9]+) and ([+-]?[0-9]*[.]?[0-9]+)`, addf)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = suite.AddStep(`the result should equal ([+-]?[0-9]*[.]?[0-9]+)`, checkf)
-	if err != nil {
-		t.Fatal(err)
-	}
+	suite.AddStep(`I add ([+-]?[0-9]*[.]?[0-9]+) and ([+-]?[0-9]*[.]?[0-9]+)`, addf)
+	suite.AddStep(`the result should equal ([+-]?[0-9]*[.]?[0-9]+)`, checkf)
 
 	suite.Run()
 }
 
 func TestScenarioOutline(t *testing.T) {
 	suite := NewSuite(t, NewSuiteOptions().WithFeaturesPath("features/outline.feature"))
-	err := suite.AddStep(`I add <digit1> and <digit2>`, add)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = suite.AddStep(`the result should equal <result>`, check)
-	if err != nil {
-		t.Fatal(err)
-	}
+	suite.AddStep(`I add (\d+) and (\d+)`, add)
+	suite.AddStep(`the result should equal <result>`, check)
 
 	suite.Run()
 }
@@ -54,15 +35,8 @@ func TestScenarioOutline(t *testing.T) {
 func TestBackground(t *testing.T) {
 	options := NewSuiteOptions().WithFeaturesPath("features/background.feature")
 	suite := NewSuite(t, options)
-	err := suite.AddStep(`I add (\d+) and (\d+)`, add)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = suite.AddStep(`the result should equal (\d+)`, check)
-	if err != nil {
-		t.Fatal(err)
-	}
+	suite.AddStep(`I add (\d+) and (\d+)`, add)
+	suite.AddStep(`the result should equal (\d+)`, check)
 
 	suite.Run()
 }
@@ -70,15 +44,8 @@ func TestBackground(t *testing.T) {
 func TestTags(t *testing.T) {
 	options := NewSuiteOptions().WithFeaturesPath("features/tags.feature").WithTags([]string{"@tag"})
 	suite := NewSuite(t, options)
-	err := suite.AddStep(`fail the test`, fail)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = suite.AddStep(`the test should pass`, pass)
-	if err != nil {
-		t.Fatal(err)
-	}
+	suite.AddStep(`fail the test`, fail)
+	suite.AddStep(`the test should pass`, pass)
 
 	suite.Run()
 }
@@ -87,11 +54,7 @@ func TestIgnoredTags(t *testing.T) {
 	options := NewSuiteOptions().WithFeaturesPath("features/ignored_tags.feature")
 	options = options.WithIgnoredTags([]string{"@ignore"})
 	suite := NewSuite(t, options)
-	err := suite.AddStep(`fail the test`, fail)
-	if err != nil {
-		t.Fatal(err)
-	}
-
+	suite.AddStep(`fail the test`, fail)
 	suite.Run()
 }
 
@@ -108,10 +71,13 @@ func TestInvalidFunctionSignature(t *testing.T) {
 
 	for name, testCase := range testCases {
 		t.Run(name, func(t *testing.T) {
-			suite := NewSuite(t, NewSuiteOptions())
-			err := suite.AddStep("", testCase.f)
-			if err == nil {
-				t.Errorf("the function has invalid signature; the test should fail")
+			tester := &mockTester{}
+			suite := NewSuite(tester, NewSuiteOptions())
+			suite.AddStep("", testCase.f)
+			suite.Run()
+			err := assert.Equals(1, tester.fatalCalled)
+			if err != nil {
+				t.Fatal(err)
 			}
 		})
 	}
@@ -156,3 +122,27 @@ func fail(ctx context.Context) (context.Context, error) {
 func pass(ctx context.Context) (context.Context, error) {
 	return ctx, nil
 }
+
+type mockTester struct {
+	fatalCalled int
+}
+
+func (m mockTester) Log(...interface{}) {
+}
+
+func (m *mockTester) Fatal(...interface{}) {
+	m.fatalCalled++
+}
+
+func (m mockTester) Fatalf(string, ...interface{}) {
+}
+
+func (m mockTester) Parallel() {
+}
+
+func (m mockTester) Fail() {
+}
+
+func (m mockTester) Run(name string, f func(t *testing.T)) bool {
+	return true
+}
diff --git a/testhttp/setup.go b/testhttp/setup.go
index d4284c3..0d05905 100644
--- a/testhttp/setup.go
+++ b/testhttp/setup.go
@@ -6,13 +6,14 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/go-bdd/assert"
 	"io/ioutil"
 	"net/http"
+
+	"github.com/go-bdd/assert"
 )
 
 type addStepper interface {
-	AddStep(definition string, step interface{}) error
+	AddStep(definition string, step interface{})
 }
 
 type testHTTPMethods struct {
@@ -29,16 +30,16 @@ func Build(addStep addStepper, h httpHandler) TestHTTP {
 
 	testHTTP := testHTTPMethods{tHTTP: thhtp}
 
-	_ = addStep.AddStep(`^I make a (GET|POST|PUT|DELETE|OPTIONS) request to "([^"]*)"$`, testHTTP.makeRequest)
-	_ = addStep.AddStep(`^the response code equals (\d+)$`, testHTTP.statusCodeEquals)
-	_ = addStep.AddStep(`^the response contains a valid JSON$`, testHTTP.validJSON)
-	_ = addStep.AddStep(`^the response is "(.*)"$`, testHTTP.theResponseIs)
-	_ = addStep.AddStep(`^the response header "(.*)" equals "(.*)"$`, testHTTP.responseHeaderEquals)
-	_ = addStep.AddStep(`^I have a (GET|POST|PUT|DELETE|OPTIONS) request "(.*)"$`, testHTTP.iHaveARequest)
-	_ = addStep.AddStep(`^I set request header "(.*)" to "(.*)"$`, testHTTP.iSetRequestSetTo)
-	_ = addStep.AddStep(`^I set request body to "([^"]*)"$`, testHTTP.iSetRequestBodyTo)
-	_ = addStep.AddStep(`^the request has body "(.*)"$`, testHTTP.theRequestHasBody)
-	_ = addStep.AddStep(`^I make the request$`, testHTTP.iMakeRequest)
+	addStep.AddStep(`^I make a (GET|POST|PUT|DELETE|OPTIONS) request to "([^"]*)"$`, testHTTP.makeRequest)
+	addStep.AddStep(`^the response code equals (\d+)$`, testHTTP.statusCodeEquals)
+	addStep.AddStep(`^the response contains a valid JSON$`, testHTTP.validJSON)
+	addStep.AddStep(`^the response is "(.*)"$`, testHTTP.theResponseIs)
+	addStep.AddStep(`^the response header "(.*)" equals "(.*)"$`, testHTTP.responseHeaderEquals)
+	addStep.AddStep(`^I have a (GET|POST|PUT|DELETE|OPTIONS) request "(.*)"$`, testHTTP.iHaveARequest)
+	addStep.AddStep(`^I set request header "(.*)" to "(.*)"$`, testHTTP.iSetRequestSetTo)
+	addStep.AddStep(`^I set request body to "([^"]*)"$`, testHTTP.iSetRequestBodyTo)
+	addStep.AddStep(`^the request has body "(.*)"$`, testHTTP.theRequestHasBody)
+	addStep.AddStep(`^I make the request$`, testHTTP.iMakeRequest)
 
 	return thhtp
 }
diff --git a/testhttp_test.go b/testhttp_test.go
index 70eebc9..16f2401 100644
--- a/testhttp_test.go
+++ b/testhttp_test.go
@@ -2,10 +2,11 @@ package gobdd
 
 import (
 	"context"
-	"github.com/go-bdd/assert"
 	"net/http"
 	"testing"
 
+	"github.com/go-bdd/assert"
+
 	"github.com/go-bdd/gobdd/testhttp"
 )
 
@@ -30,11 +31,11 @@ func TestHTTP(t *testing.T) {
 		w.WriteHeader(http.StatusOK)
 	})
 
-	_ = s.AddStep(`^the request has method set to (GET|POST|PUT|DELETE|OPTIONS)$`, requestHasMethodSetTo)
-	_ = s.AddStep(`^the url is set to "([^"]*)"$`, urlIsSetTo)
-	_ = s.AddStep(`^the request body is nil$`, requestBodyIsNil)
-	_ = s.AddStep(`^I set the header "([^"]*)" to "([^"]*)"$`, ISetHeaderTo)
-	_ = s.AddStep(`^the request has header "([^"]*)" set to "([^"]*)"$`, requestHasHeaderSetTo)
+	s.AddStep(`^the request has method set to (GET|POST|PUT|DELETE|OPTIONS)$`, requestHasMethodSetTo)
+	s.AddStep(`^the url is set to "([^"]*)"$`, urlIsSetTo)
+	s.AddStep(`^the request body is nil$`, requestBodyIsNil)
+	s.AddStep(`^I set the header "([^"]*)" to "([^"]*)"$`, ISetHeaderTo)
+	s.AddStep(`^the request has header "([^"]*)" set to "([^"]*)"$`, requestHasHeaderSetTo)
 	testhttp.Build(s, router)
 
 	s.Run()
@@ -62,7 +63,7 @@ func ISetHeaderTo(ctx context.Context, headerName, value string) (context.Contex
 	return ctx, nil
 }
 
-func requestHasHeaderSetTo(ctx context.Context, headerName,expected  string) (context.Context, error) {
+func requestHasHeaderSetTo(ctx context.Context, headerName, expected string) (context.Context, error) {
 	r := ctx.Value(testhttp.RequestKey{}).(*http.Request)
 	given := r.Header.Get(headerName)
 	return ctx, assert.Equals(expected, given)