From fbed999ad81a4aeffb265156d002490c5e98f9f1 Mon Sep 17 00:00:00 2001 From: tfreville Date: Thu, 10 Dec 2020 12:35:47 +0100 Subject: [PATCH] feat(step_definition): Allows to define step function without return. Issue: It is not possible to use function without return when matching steps, resulting in a lot of Nil only error returns. Fix: Allows to provide empty result function by correctly matching reflect Calls on step Handler. When nothing is returned by the Handler, it will return nil as if errors was nil. --- features/tags.feature | 1 + internal/models/stepdef.go | 6 +++++- internal/models/stepdef_test.go | 21 +++++++++++++++++++++ run_test.go | 3 ++- suite_context_test.go | 4 ++-- test_context.go | 30 ++++++++++++++++-------------- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/features/tags.feature b/features/tags.feature index 06e3f20..bf182fd 100644 --- a/features/tags.feature +++ b/features/tags.feature @@ -10,6 +10,7 @@ Feature: tag filters Background: Given passing step + And passing step without return Scenario Outline: parse a scenario Given a feature path "" diff --git a/internal/models/stepdef.go b/internal/models/stepdef.go index 1b24013..d2a2419 100644 --- a/internal/models/stepdef.go +++ b/internal/models/stepdef.go @@ -156,7 +156,11 @@ func (sd *StepDefinition) Run() interface{} { } } - return sd.HandlerValue.Call(values)[0].Interface() + res := sd.HandlerValue.Call(values) + if len(res) == 0 { + return nil + } + return res[0].Interface() } func (sd *StepDefinition) shouldBeString(idx int) (string, error) { diff --git a/internal/models/stepdef_test.go b/internal/models/stepdef_test.go index 315a0fb..2e42f02 100644 --- a/internal/models/stepdef_test.go +++ b/internal/models/stepdef_test.go @@ -12,6 +12,27 @@ import ( "github.com/cucumber/godog/internal/models" ) +func TestShouldSupportEmptyHandlerReturn(t *testing.T) { + fn := func(a int64, b int32, c int16, d int8) {} + + def := &models.StepDefinition{ + StepDefinition: formatters.StepDefinition{ + Handler: fn, + }, + HandlerValue: reflect.ValueOf(fn), + } + + def.Args = []interface{}{"1", "1", "1", "1"} + if err := def.Run(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + def.Args = []interface{}{"1", "1", "1", strings.Repeat("1", 9)} + if err := def.Run(); err == nil { + t.Fatalf("expected convertion fail for int8, but got none") + } +} + func TestShouldSupportIntTypes(t *testing.T) { fn := func(a int64, b int32, c int16, d int8) error { return nil } diff --git a/run_test.go b/run_test.go index b94018b..01fe4cf 100644 --- a/run_test.go +++ b/run_test.go @@ -568,7 +568,8 @@ type progressOutput struct { bottomRows []string } -func passingStepDef() error { return nil } +func passingStepDef() error { return nil } +func passingStepDefWithoutReturn() {} func oddEvenStepDef(odd, even int) error { return oddOrEven(odd, even) } diff --git a/suite_context_test.go b/suite_context_test.go index 12f20f0..8997b4f 100644 --- a/suite_context_test.go +++ b/suite_context_test.go @@ -108,6 +108,7 @@ func InitializeScenario(ctx *ScenarioContext) { return nil }) + ctx.Step(`^(?:a )?passing step without return$`, func() {}) ctx.BeforeStep(tc.inject) } @@ -424,9 +425,8 @@ func (tc *godogFeaturesScenario) aFeatureFile(path string, body *DocString) erro return err } -func (tc *godogFeaturesScenario) featurePath(path string) error { +func (tc *godogFeaturesScenario) featurePath(path string) { tc.paths = append(tc.paths, path) - return nil } func (tc *godogFeaturesScenario) parseFeatures() error { diff --git a/test_context.go b/test_context.go index c2eafa0..7ad539f 100644 --- a/test_context.go +++ b/test_context.go @@ -176,8 +176,8 @@ func (ctx *ScenarioContext) Step(expr, stepFunc interface{}) { panic(fmt.Sprintf("expected handler to be func, but got: %T", stepFunc)) } - if typ.NumOut() != 1 { - panic(fmt.Sprintf("expected handler to return only one value, but it has: %d", typ.NumOut())) + if typ.NumOut() > 1 { + panic(fmt.Sprintf("expected handler to return either zero or one value, but it has: %d", typ.NumOut())) } def := &models.StepDefinition{ @@ -188,19 +188,21 @@ func (ctx *ScenarioContext) Step(expr, stepFunc interface{}) { HandlerValue: v, } - typ = typ.Out(0) - switch typ.Kind() { - case reflect.Interface: - if !typ.Implements(errorInterface) { - panic(fmt.Sprintf("expected handler to return an error, but got: %s", typ.Kind())) + if typ.NumOut() == 1 { + typ = typ.Out(0) + switch typ.Kind() { + case reflect.Interface: + if !typ.Implements(errorInterface) { + panic(fmt.Sprintf("expected handler to return an error, but got: %s", typ.Kind())) + } + case reflect.Slice: + if typ.Elem().Kind() != reflect.String { + panic(fmt.Sprintf("expected handler to return []string for multistep, but got: []%s", typ.Kind())) + } + def.Nested = true + default: + panic(fmt.Sprintf("expected handler to return an error or []string, but got: %s", typ.Kind())) } - case reflect.Slice: - if typ.Elem().Kind() != reflect.String { - panic(fmt.Sprintf("expected handler to return []string for multistep, but got: []%s", typ.Kind())) - } - def.Nested = true - default: - panic(fmt.Sprintf("expected handler to return an error or []string, but got: %s", typ.Kind())) } ctx.suite.steps = append(ctx.suite.steps, def)