From 9ad4eab27a12755a8c624424e880056bf2aba9d7 Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Sat, 28 Sep 2024 21:39:52 -0700 Subject: [PATCH 1/7] docs: add duplicate steps example --- .gitignore | 1 + _examples/dupsteps/README.md | 34 +++++++++ _examples/dupsteps/dupsteps_test.go | 113 ++++++++++++++++++++++++++++ _examples/dupsteps/go.mod | 21 ++++++ _examples/dupsteps/go.sum | 51 +++++++++++++ 5 files changed, 220 insertions(+) create mode 100644 _examples/dupsteps/README.md create mode 100644 _examples/dupsteps/dupsteps_test.go create mode 100644 _examples/dupsteps/go.mod create mode 100644 _examples/dupsteps/go.sum diff --git a/.gitignore b/.gitignore index bd77fc9f..2ee95b1e 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ Gopkg.toml _artifacts vendor +godog.iml diff --git a/_examples/dupsteps/README.md b/_examples/dupsteps/README.md new file mode 100644 index 00000000..682aa5a5 --- /dev/null +++ b/_examples/dupsteps/README.md @@ -0,0 +1,34 @@ +# Duplicate Steps + +This example reproduces the problem wherein duplicate steps are silently overridden. + +## Motivation + +As a relatively new user of Cucumber & its [Gherkin syntax](https://cucumber.io/docs/gherkin/), and +as an implementer of steps for a scenario using `godog`, I'd like to have the ability to encapsulate +implementation of steps for a scenario without concern that a step function from a different scenario +will be called instead of mine. At least, I'd like to have more control over automatic "re-use" of +step functions; either: (1) choose to limit the assignment of a step function to step text (or regex) +to be matched only within the scope of its enclosing scenario (preferred), (2) have `godog` throw an +error when an ambiguous match to a step function is defined or detected for a step's implementation. + +Though I've begun to understand that re-use of step implementations is somewhat fundamental to the Gherkin design +(e.g., I've read about _[feature coupled step definitions](https://cucumber.io/docs/guides/anti-patterns/?lang=java#feature-coupled-step-definitions)_ +and how Cucumber _"effectively flattens the features/ directory tree"_), it's still annoying that +the `godog` scaffold seems to _require us to conform_ to the Gherkin recommendation to _"organise +your steps by domain concept"_, and not by Feature or even Scenario, as would better suit our project. + +What's ended up happening to several of our developers in our distributed BDD testing initiative is +they end up force-tweaking the text of their steps in order to avoid duplicates, as they don't have agency +over other scenario implementations which they need to run with in our Jenkins pipeline. Later, they're +annoyed when their scenarios suddenly start failing after new scenarios are added having step +implementations with regex which _happens_ to match (thereby overriding) theirs. + +_NOTE:_ due to a limitation in our Jenkins pipeline, all of our features & scenarios _must_ be executed +within the same `godog.Suite`, else (I realize) we could just "solve" this problem by running each scenario +in its own invocation of `godog`. + +## Summary + +In light of the specifics of the "Motivation" above, the stated "problem" here might then be more +effectively re-characterized as a "request" to give more control to the end-user, as suggested above. diff --git a/_examples/dupsteps/dupsteps_test.go b/_examples/dupsteps/dupsteps_test.go new file mode 100644 index 00000000..f231d334 --- /dev/null +++ b/_examples/dupsteps/dupsteps_test.go @@ -0,0 +1,113 @@ +package dupsteps + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/cucumber/godog" +) + +func TestDuplicateSteps(t *testing.T) { + + featureContents := ` +Feature: check this out + + Scenario: Flat Tire + Given I ran over a nail and got a flat tire + Then I fixed it + Then I can continue on my way + + Scenario: Clogged Drain + Given I accidentally poured concrete down my drain and clogged the sewer line + Then I fixed it + Then I can once again use my sink +` + + suite := godog.TestSuite{ + Name: t.Name(), + ScenarioInitializer: func(context *godog.ScenarioContext) { + // NOTE: loading implementations of steps for different scenarios here + (&cloggedDrain{}).addCloggedDrainSteps(context) + (&flatTire{}).addFlatTireSteps(context) + }, + Options: &godog.Options{ + Format: "pretty", + Strict: true, + FeatureContents: []godog.Feature{ + { + Name: fmt.Sprintf("%s contents", t.Name()), + Contents: []byte(featureContents), + }, + }, + }, + } + + rc := suite.Run() + assert.Zero(t, rc) +} + +// Implementation of the steps for the "Clogged Drain" scenario + +type cloggedDrain struct { + drainIsClogged bool +} + +func (cd *cloggedDrain) addCloggedDrainSteps(ctx *godog.ScenarioContext) { + ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, cd.clogSewerLine) + ctx.Step(`^I fixed it$`, cd.iFixedIt) + ctx.Step(`^I can once again use my sink$`, cd.useTheSink) +} + +func (cd *cloggedDrain) clogSewerLine() error { + cd.drainIsClogged = true + + return nil +} + +func (cd *cloggedDrain) iFixedIt() error { + cd.drainIsClogged = false + + return nil +} + +func (cd *cloggedDrain) useTheSink() error { + if cd.drainIsClogged { + return fmt.Errorf("drain is clogged") + } + + return nil +} + +// Implementation of the steps for the "Flat Tire" scenario + +type flatTire struct { + tireIsFlat bool +} + +func (ft *flatTire) addFlatTireSteps(ctx *godog.ScenarioContext) { + ctx.Step(`^I ran over a nail and got a flat tire$`, ft.gotFlatTire) + ctx.Step(`^I fixed it$`, ft.iFixedIt) + ctx.Step(`^I can continue on my way$`, ft.continueOnMyWay) +} + +func (ft *flatTire) gotFlatTire() error { + ft.tireIsFlat = true + + return nil +} + +func (ft *flatTire) iFixedIt() error { + ft.tireIsFlat = false + + return nil +} + +func (ft *flatTire) continueOnMyWay() error { + if ft.tireIsFlat { + return fmt.Errorf("tire was not fixed") + } + + return nil +} diff --git a/_examples/dupsteps/go.mod b/_examples/dupsteps/go.mod new file mode 100644 index 00000000..b97761c2 --- /dev/null +++ b/_examples/dupsteps/go.mod @@ -0,0 +1,21 @@ +module github.com/cucumber/godog/_examples/dupsteps + +go 1.23.1 + +require ( + github.com/cucumber/godog v0.14.1 + github.com/stretchr/testify v1.8.2 +) + +require ( + github.com/cucumber/gherkin/go/v26 v26.2.0 // indirect + github.com/cucumber/messages/go/v21 v21.0.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/gofrs/uuid v4.3.1+incompatible // indirect + github.com/hashicorp/go-immutable-radix v1.3.1 // indirect + github.com/hashicorp/go-memdb v1.3.4 // indirect + github.com/hashicorp/golang-lru v0.5.4 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/spf13/pflag v1.0.5 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/_examples/dupsteps/go.sum b/_examples/dupsteps/go.sum new file mode 100644 index 00000000..12b212b3 --- /dev/null +++ b/_examples/dupsteps/go.sum @@ -0,0 +1,51 @@ +github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cucumber/gherkin/go/v26 v26.2.0 h1:EgIjePLWiPeslwIWmNQ3XHcypPsWAHoMCz/YEBKP4GI= +github.com/cucumber/gherkin/go/v26 v26.2.0/go.mod h1:t2GAPnB8maCT4lkHL99BDCVNzCh1d7dBhCLt150Nr/0= +github.com/cucumber/godog v0.14.1 h1:HGZhcOyyfaKclHjJ+r/q93iaTJZLKYW6Tv3HkmUE6+M= +github.com/cucumber/godog v0.14.1/go.mod h1:FX3rzIDybWABU4kuIXLZ/qtqEe1Ac5RdXmqvACJOces= +github.com/cucumber/messages/go/v21 v21.0.1 h1:wzA0LxwjlWQYZd32VTlAVDTkW6inOFmSM+RuOwHZiMI= +github.com/cucumber/messages/go/v21 v21.0.1/go.mod h1:zheH/2HS9JLVFukdrsPWoPdmUtmYQAQPLk7w5vWsk5s= +github.com/cucumber/messages/go/v22 v22.0.0/go.mod h1:aZipXTKc0JnjCsXrJnuZpWhtay93k7Rn3Dee7iyPJjs= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/gofrs/uuid v4.2.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= +github.com/gofrs/uuid v4.3.1+incompatible h1:0/KbAdpx3UXAx1kEOWHJeOkpbgRFGHVgv+CFIY7dBJI= +github.com/gofrs/uuid v4.3.1+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM= +github.com/hashicorp/go-immutable-radix v1.3.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= +github.com/hashicorp/go-immutable-radix v1.3.1 h1:DKHmCUm2hRBK510BaiZlwvpD40f8bJFeZnpfm2KLowc= +github.com/hashicorp/go-immutable-radix v1.3.1/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= +github.com/hashicorp/go-memdb v1.3.4 h1:XSL3NR682X/cVk2IeV0d70N4DZ9ljI885xAEU8IoK3c= +github.com/hashicorp/go-memdb v1.3.4/go.mod h1:uBTr1oQbtuMgd1SSGoR8YV27eT3sBHbYiNm53bMpgSg= +github.com/hashicorp/go-uuid v1.0.0/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= +github.com/hashicorp/go-uuid v1.0.2 h1:cfejS+Tpcp13yd5nYHWDI6qVCny6wyX2Mt5SGur2IGE= +github.com/hashicorp/go-uuid v1.0.2/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro= +github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= +github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc= +github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= +github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= +github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= +github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 54a569f95c79a3a85575ae2e775245fc1b1f9a28 Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Sun, 29 Sep 2024 21:12:18 -0700 Subject: [PATCH 2/7] docs: update based upon discovery of PR-636! --- _examples/dupsteps/README.md | 32 ++++++++++++++++++++++++++++++-- _examples/dupsteps/go.mod | 3 +++ _examples/dupsteps/go.sum | 2 -- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/_examples/dupsteps/README.md b/_examples/dupsteps/README.md index 682aa5a5..b0d20edc 100644 --- a/_examples/dupsteps/README.md +++ b/_examples/dupsteps/README.md @@ -1,8 +1,36 @@ # Duplicate Steps +## Updated Statement + +After submitting this example / feedback, while looking through the `godog` source +code, I came across [PR-636](https://github.com/cucumber/godog/pull/636) which deals +with the very same issue as described below - i.e., duplicate (ambiguous) steps. + +Indeed, this accepted and merged PR wasn't available in the `godog` library version +we are using (you can be sure I'll put in an upgrade request to the team, though!), +so I was not familiar with it. + +Now that this PR is in place and enabled when using `strict` mode (which we _are_ +using), it will help us - at least our developers will receive a repeatable test +failure _right away_ when duplicate (ambiguous) steps are detected (though the +"error" message is a bit cryptic: i.e., it seems like it just says **"IS STRICT=2"** +in the given example case). However, ultimately as described below we'd like +to be able to encapsulate step implementations completely within a scenario, +and it seems this desired functionality is not (yet?) available. + +Therefore, the focus of this PR is now upon tips / suggestions for how to implement +the desired functionality of limiting the scope of a step definition to either +a scenario and/or perhaps a feature (as opposed to always globally). Or possibly, +why that's a bad idea (I find it suspicious that hasn't been done already ;-0). + +The original (pre-update above) text of this README is preserved for posterity, +below: + +## Original Statement + This example reproduces the problem wherein duplicate steps are silently overridden. -## Motivation +### Motivation As a relatively new user of Cucumber & its [Gherkin syntax](https://cucumber.io/docs/gherkin/), and as an implementer of steps for a scenario using `godog`, I'd like to have the ability to encapsulate @@ -28,7 +56,7 @@ _NOTE:_ due to a limitation in our Jenkins pipeline, all of our features & scena within the same `godog.Suite`, else (I realize) we could just "solve" this problem by running each scenario in its own invocation of `godog`. -## Summary +### Summary In light of the specifics of the "Motivation" above, the stated "problem" here might then be more effectively re-characterized as a "request" to give more control to the end-user, as suggested above. diff --git a/_examples/dupsteps/go.mod b/_examples/dupsteps/go.mod index b97761c2..0944604e 100644 --- a/_examples/dupsteps/go.mod +++ b/_examples/dupsteps/go.mod @@ -19,3 +19,6 @@ require ( github.com/spf13/pflag v1.0.5 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +// See updated comment in the README - using the local source tree to get the "strict" functionality in PR-636 +replace github.com/cucumber/godog v0.14.1 => ../.. diff --git a/_examples/dupsteps/go.sum b/_examples/dupsteps/go.sum index 12b212b3..a5aeb8f4 100644 --- a/_examples/dupsteps/go.sum +++ b/_examples/dupsteps/go.sum @@ -1,8 +1,6 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cucumber/gherkin/go/v26 v26.2.0 h1:EgIjePLWiPeslwIWmNQ3XHcypPsWAHoMCz/YEBKP4GI= github.com/cucumber/gherkin/go/v26 v26.2.0/go.mod h1:t2GAPnB8maCT4lkHL99BDCVNzCh1d7dBhCLt150Nr/0= -github.com/cucumber/godog v0.14.1 h1:HGZhcOyyfaKclHjJ+r/q93iaTJZLKYW6Tv3HkmUE6+M= -github.com/cucumber/godog v0.14.1/go.mod h1:FX3rzIDybWABU4kuIXLZ/qtqEe1Ac5RdXmqvACJOces= github.com/cucumber/messages/go/v21 v21.0.1 h1:wzA0LxwjlWQYZd32VTlAVDTkW6inOFmSM+RuOwHZiMI= github.com/cucumber/messages/go/v21 v21.0.1/go.mod h1:zheH/2HS9JLVFukdrsPWoPdmUtmYQAQPLk7w5vWsk5s= github.com/cucumber/messages/go/v22 v22.0.0/go.mod h1:aZipXTKc0JnjCsXrJnuZpWhtay93k7Rn3Dee7iyPJjs= From 60fae8cf6a76e624c28a4523307c91327c6e4d90 Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Sun, 6 Oct 2024 15:57:48 -0700 Subject: [PATCH 3/7] test: make example test 'pass' (i.e., by demonstrating problem), add 'step checker' tool --- _examples/dupsteps/README.md | 4 + _examples/dupsteps/cmd/stepchecker/README.md | 39 +++ _examples/dupsteps/cmd/stepchecker/go.mod | 3 + _examples/dupsteps/cmd/stepchecker/main.go | 306 ++++++++++++++++++ _examples/dupsteps/dupsteps_test.go | 113 ------- _examples/dupsteps/tests/dupsteps_test.go | 188 +++++++++++ .../dupsteps/tests/features/dupsteps.feature | 14 + _examples/dupsteps/{ => tests}/go.mod | 7 +- _examples/dupsteps/{ => tests}/go.sum | 5 +- 9 files changed, 560 insertions(+), 119 deletions(-) create mode 100644 _examples/dupsteps/cmd/stepchecker/README.md create mode 100644 _examples/dupsteps/cmd/stepchecker/go.mod create mode 100644 _examples/dupsteps/cmd/stepchecker/main.go delete mode 100644 _examples/dupsteps/dupsteps_test.go create mode 100644 _examples/dupsteps/tests/dupsteps_test.go create mode 100644 _examples/dupsteps/tests/features/dupsteps.feature rename _examples/dupsteps/{ => tests}/go.mod (69%) rename _examples/dupsteps/{ => tests}/go.sum (92%) diff --git a/_examples/dupsteps/README.md b/_examples/dupsteps/README.md index b0d20edc..39cc9586 100644 --- a/_examples/dupsteps/README.md +++ b/_examples/dupsteps/README.md @@ -6,6 +6,10 @@ After submitting this example / feedback, while looking through the `godog` sour code, I came across [PR-636](https://github.com/cucumber/godog/pull/636) which deals with the very same issue as described below - i.e., duplicate (ambiguous) steps. +- _NOTE: until the change made in [PR-636](https://github.com/cucumber/godog/pull/636) + is made available (it's not yet been released as of this writing), you can use something + like the [stepchecker](cmd/stepchecker/README.md) to detect such cases._ + Indeed, this accepted and merged PR wasn't available in the `godog` library version we are using (you can be sure I'll put in an upgrade request to the team, though!), so I was not familiar with it. diff --git a/_examples/dupsteps/cmd/stepchecker/README.md b/_examples/dupsteps/cmd/stepchecker/README.md new file mode 100644 index 00000000..46b894c5 --- /dev/null +++ b/_examples/dupsteps/cmd/stepchecker/README.md @@ -0,0 +1,39 @@ +# Step Checker + +You can use this step checker tool to help detect missing, duplicate or ambiguous steps +implemented in a `godog` test suite. Early detection of the presence of these prior to +submitting a PR or running a Jenkins pipeline with new or modified step implementations +can potentially save time & help prevent the appearance of unexpected false positives +or negative test results, as described in this example's Problem Statement (see outer +README file). + +## Invocation + +Example: + +```shell +$ cd ~/repos/godog/_examples/dupsteps +$ go run cmd/stepchecker/main.go tests +Found 5 feature step(s): +1. "I ran over a nail and got a flat tire" +2. "I fixed it" + - 2 matching godog step(s) found: + from: tests/features/dupsteps.feature:7 + from: tests/features/dupsteps.feature:13 + to: tests/dupsteps_test.go:93:11 + to: tests/dupsteps_test.go:125:11 +3. "I can continue on my way" +4. "I accidentally poured concrete down my drain and clogged the sewer line" +5. "I can once again use my sink" + +Found 5 godog step(s): +1. "^I fixed it$" +2. "^I can once again use my sink$" +3. "^I ran over a nail and got a flat tire$" +4. "^I can continue on my way$" +5. "^I accidentally poured concrete down my drain and clogged the sewer line$" + +2024/10/06 15:38:50 1 issue(s) found +exit status 1 +$ _ +``` diff --git a/_examples/dupsteps/cmd/stepchecker/go.mod b/_examples/dupsteps/cmd/stepchecker/go.mod new file mode 100644 index 00000000..afde37ea --- /dev/null +++ b/_examples/dupsteps/cmd/stepchecker/go.mod @@ -0,0 +1,3 @@ +module github.com/cucumber/godog/_examples/dupsteps/cmd/stepchecker + +go 1.23.1 diff --git a/_examples/dupsteps/cmd/stepchecker/main.go b/_examples/dupsteps/cmd/stepchecker/main.go new file mode 100644 index 00000000..b5d047c7 --- /dev/null +++ b/_examples/dupsteps/cmd/stepchecker/main.go @@ -0,0 +1,306 @@ +package main + +import ( + "bufio" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "log" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" +) + +// +// See accompanying README file(s); +// also https://github.com/cucumber/godog/pull/642 +// + +func main() { + if len(os.Args) != 2 { + log.Printf("Usage: main.go \n") + + os.Exit(RC_USER) + } + + rootPath := os.Args[1] + + // Structures into which to collect step patterns found in Go and feature files + godogSteps := make(map[string]*StepMatch) + featureSteps := make(map[string]*StepMatch) + + // Walk through all files in the directory + errWalk := filepath.WalkDir(rootPath, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + if strings.HasSuffix(path, ".go") { + return collectGoSteps(path, godogSteps) + } + + if strings.HasSuffix(path, ".feature") { + return collectFeatureSteps(path, featureSteps) + } + + return nil + }) + + if errWalk != nil { + log.Printf("error walking directory: %v\n", errWalk) + + os.Exit(RC_SYSTEM) + } + + if len(godogSteps) == 0 { + log.Printf("no godog step definition(s) found") + os.Exit(RC_USER) + } + + if len(featureSteps) == 0 { + log.Printf("no feature step invocation(s) found") + os.Exit(RC_USER) + } + + // Match steps between Go and feature files + matchSteps(godogSteps, featureSteps) + + var issuesFound int + + // Report on unexpected (i.e., lack of, duplicate or ambiguous) mapping from feature steps to go steps + fmt.Printf("Found %d feature step(s):\n", len(featureSteps)) + + var fsIdx int + + for text, step := range featureSteps { + fsIdx++ + + fmt.Printf("%d. %q\n", fsIdx, text) + + if len(step.matchedWith) != 1 { + issuesFound++ + + fmt.Printf(" - %d matching godog step(s) found:\n", len(step.matchedWith)) + + for _, match := range step.source { + fmt.Printf(" from: %s\n", match) + } + + for _, match := range step.matchedWith { + fmt.Printf(" to: %s\n", match) + } + } + } + + fmt.Println() + + // Report on lack of mapping from go steps to feature steps + fmt.Printf("Found %d godog step(s):\n", len(godogSteps)) + + var gdsIdx int + + for text, step := range godogSteps { + gdsIdx++ + + fmt.Printf("%d. %q\n", gdsIdx, text) + + if len(step.matchedWith) == 0 { + issuesFound++ + + fmt.Printf(" - No matching feature step(s) found:\n") + + for _, match := range step.source { + fmt.Printf(" from: %s\n", match) + } + } + } + + fmt.Println() + + if issuesFound != 0 { + log.Printf("%d issue(s) found\n", issuesFound) + os.Exit(RC_ISSUES) + } +} + +func collectGoSteps(filePath string, steps map[string]*StepMatch) error { + fset := token.NewFileSet() + + node, errParse := parser.ParseFile(fset, filePath, nil, parser.ParseComments) + if errParse != nil { + return fmt.Errorf("error parsing Go file %s: %w", filePath, errParse) + } + + stepDefPattern := regexp.MustCompile(`^godog.ScenarioContext.(Given|When|Then|And|Step)$`) + + var errInspect error + + ast.Inspect(node, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + + methodID := extractMethodID(call) + if methodID == "" { + return true + } + + if !stepDefPattern.MatchString(methodID) { + return true + } + + if len(call.Args) == 0 { + log.Printf("WARNING: ignoring call to step function with no arguments: %s\n", methodID) + + return true + } + + lit, ok := call.Args[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + log.Printf("WARNING: ignoring unexpected step function invocation at %s\n", fset.Position(call.Pos())) + + return true + } + + pattern, errQ := strconv.Unquote(lit.Value) + + if errQ != nil { + errInspect = errQ + return false + } + + sm, found := steps[pattern] + if !found { + sm = &StepMatch{} + steps[pattern] = sm + } + + sm.source = append(sm.source, sourceRef(fset.Position(lit.ValuePos).String())) + + return true + }) + + if errInspect != nil { + return fmt.Errorf("error encountered while inspecting %q: %w", filePath, errInspect) + } + + return nil +} + +func collectFeatureSteps(filePath string, steps map[string]*StepMatch) error { + file, errOpen := os.Open(filePath) + if errOpen != nil { + return errOpen + } + + defer func() { _ = file.Close() }() + + scanner := bufio.NewScanner(file) + sp := regexp.MustCompile(`^\s*(Given|When|Then|And) (.+)\s*$`) + + for lineNo := 1; scanner.Scan(); lineNo++ { + + line := scanner.Text() + matches := sp.FindStringSubmatch(line) + if matches == nil { + continue + } + + if len(matches) != 3 { + return fmt.Errorf("unexpected number of matches at %s:%d: %d for %q\n", filePath, lineNo, len(matches), line) + } + + stepText := matches[2] + + sm, found := steps[stepText] + if !found { + sm = &StepMatch{} + steps[stepText] = sm + } + + sm.source = append(sm.source, sourceRef(fmt.Sprintf("%s:%d", filePath, lineNo))) + } + + return nil +} + +func matchSteps(godogSteps, featureSteps map[string]*StepMatch) { + // for each step definition found in go + for pattern, godogStep := range godogSteps { + matcher, errComp := regexp.Compile(pattern) + if errComp != nil { + log.Printf("error compiling regex for pattern '%s': %v\n", pattern, errComp) + + continue + } + + // record matches between feature steps and go steps + for featureText, featureStep := range featureSteps { + if matcher.MatchString(featureText) { + featureStep.matchedWith = append(featureStep.matchedWith, godogStep.source...) + godogStep.matchedWith = append(godogStep.matchedWith, featureStep.source...) + } + } + } +} + +func extractMethodID(call *ast.CallExpr) string { + // Ensure the function is a method call + fnSelExpr, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return "" + } + + // Ensure the method is being called on an identifier + lhsID, ok := fnSelExpr.X.(*ast.Ident) + if !ok || lhsID.Obj == nil { + return "" + } + + // Ensure the identifier represents a field declaration + lhsField, ok := lhsID.Obj.Decl.(*ast.Field) + if !ok { + return "" + } + + // Ensure the field type is a pointer to another type + lhsStarExpr, ok := lhsField.Type.(*ast.StarExpr) + if !ok { + return "" + } + + // Ensure the pointer type is a package or struct + lhsSelExpr, ok := lhsStarExpr.X.(*ast.SelectorExpr) + if !ok { + return "" + } + + // Ensure the receiver type is an identifier (e.g., the package or struct name) + lhsLhsID, ok := lhsSelExpr.X.(*ast.Ident) + if !ok { + return "" + } + + // return a method call identifier sufficient to identify those of interest + return fmt.Sprintf("%s.%s.%s", lhsLhsID.Name, lhsSelExpr.Sel.Name, fnSelExpr.Sel.Name) +} + +type sourceRef string + +type StepMatch struct { + source []sourceRef // location of the source(s) for this step + matchedWith []sourceRef // location of match(es) to this step +} + +const RC_ISSUES = 1 +const RC_USER = 2 +const RC_SYSTEM = 3 diff --git a/_examples/dupsteps/dupsteps_test.go b/_examples/dupsteps/dupsteps_test.go deleted file mode 100644 index f231d334..00000000 --- a/_examples/dupsteps/dupsteps_test.go +++ /dev/null @@ -1,113 +0,0 @@ -package dupsteps - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - - "github.com/cucumber/godog" -) - -func TestDuplicateSteps(t *testing.T) { - - featureContents := ` -Feature: check this out - - Scenario: Flat Tire - Given I ran over a nail and got a flat tire - Then I fixed it - Then I can continue on my way - - Scenario: Clogged Drain - Given I accidentally poured concrete down my drain and clogged the sewer line - Then I fixed it - Then I can once again use my sink -` - - suite := godog.TestSuite{ - Name: t.Name(), - ScenarioInitializer: func(context *godog.ScenarioContext) { - // NOTE: loading implementations of steps for different scenarios here - (&cloggedDrain{}).addCloggedDrainSteps(context) - (&flatTire{}).addFlatTireSteps(context) - }, - Options: &godog.Options{ - Format: "pretty", - Strict: true, - FeatureContents: []godog.Feature{ - { - Name: fmt.Sprintf("%s contents", t.Name()), - Contents: []byte(featureContents), - }, - }, - }, - } - - rc := suite.Run() - assert.Zero(t, rc) -} - -// Implementation of the steps for the "Clogged Drain" scenario - -type cloggedDrain struct { - drainIsClogged bool -} - -func (cd *cloggedDrain) addCloggedDrainSteps(ctx *godog.ScenarioContext) { - ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, cd.clogSewerLine) - ctx.Step(`^I fixed it$`, cd.iFixedIt) - ctx.Step(`^I can once again use my sink$`, cd.useTheSink) -} - -func (cd *cloggedDrain) clogSewerLine() error { - cd.drainIsClogged = true - - return nil -} - -func (cd *cloggedDrain) iFixedIt() error { - cd.drainIsClogged = false - - return nil -} - -func (cd *cloggedDrain) useTheSink() error { - if cd.drainIsClogged { - return fmt.Errorf("drain is clogged") - } - - return nil -} - -// Implementation of the steps for the "Flat Tire" scenario - -type flatTire struct { - tireIsFlat bool -} - -func (ft *flatTire) addFlatTireSteps(ctx *godog.ScenarioContext) { - ctx.Step(`^I ran over a nail and got a flat tire$`, ft.gotFlatTire) - ctx.Step(`^I fixed it$`, ft.iFixedIt) - ctx.Step(`^I can continue on my way$`, ft.continueOnMyWay) -} - -func (ft *flatTire) gotFlatTire() error { - ft.tireIsFlat = true - - return nil -} - -func (ft *flatTire) iFixedIt() error { - ft.tireIsFlat = false - - return nil -} - -func (ft *flatTire) continueOnMyWay() error { - if ft.tireIsFlat { - return fmt.Errorf("tire was not fixed") - } - - return nil -} diff --git a/_examples/dupsteps/tests/dupsteps_test.go b/_examples/dupsteps/tests/dupsteps_test.go new file mode 100644 index 00000000..98967b6e --- /dev/null +++ b/_examples/dupsteps/tests/dupsteps_test.go @@ -0,0 +1,188 @@ +package tests + +import ( + "bytes" + "flag" + "fmt" + "io" + "os" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/cucumber/godog" +) + +// +// The tests "pass" by demonstrating the "problem statement" discussed in this `dupsteps` +// example; i.e., they are expected to "fail" when the problem is fixed, or can be fixed +// through a supported `godog` configuration option / enhancement. +// +// What's being demonstrated is how godog's use of a global list of steps defined across +// all configured scenarios allows for indeterminate results, based upon the order in +// which the steps are loaded. The first "matching" step (text) will be used when +// evaluating a step for a given scenario, regardless of which scenario it was defined +// or intended to participate in. +// + +// TestFlatTireFirst demonstrates that loading the 'flatTire' step implementations first +// causes the 'cloggedDrain' test to fail +func TestFlatTireFirst(t *testing.T) { + demonstrateProblemCase(t, + func(ctx *godog.ScenarioContext) { + (&flatTire{}).addFlatTireSteps(ctx) + (&cloggedDrain{}).addCloggedDrainSteps(ctx) + }, + "drain is clogged", + ) +} + +// TestCloggedDrainFirst demonstrates that loading the 'cloggedDrain' step implementations first +// causes the 'flatTire' test to fail +func TestCloggedDrainFirst(t *testing.T) { + demonstrateProblemCase(t, + func(ctx *godog.ScenarioContext) { + (&cloggedDrain{}).addCloggedDrainSteps(ctx) + (&flatTire{}).addFlatTireSteps(ctx) + }, + "tire was not fixed", + ) +} + +// demonstrateProblemCase sets up the test suite using 'caseInitializer' and demonstrates the expected error result. +func demonstrateProblemCase(t *testing.T, caseInitializer func(ctx *godog.ScenarioContext), expectedError string) { + + var sawExpectedError bool + + opts := defaultOpts + opts.Format = "pretty" + opts.Output = &prettyOutputListener{ + wrapped: os.Stdout, + callback: func(s string) { + if strings.Contains(s, expectedError) { + sawExpectedError = true + fmt.Println("====>") + } + }, + } + + suite := godog.TestSuite{ + Name: t.Name(), + ScenarioInitializer: caseInitializer, + Options: &opts, + } + + rc := suite.Run() + + // (demonstration of) expected error + assert.NotZero(t, rc) + + // demonstrate that the expected error message was seen in the godog output + assert.True(t, sawExpectedError) +} + +// Implementation of the steps for the "Clogged Drain" scenario + +type cloggedDrain struct { + drainIsClogged bool +} + +func (cd *cloggedDrain) addCloggedDrainSteps(ctx *godog.ScenarioContext) { + ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, cd.clogSewerLine) + ctx.Step(`^I fixed it$`, cd.iFixedIt) + ctx.Step(`^I can once again use my sink$`, cd.useTheSink) +} + +func (cd *cloggedDrain) clogSewerLine() error { + cd.drainIsClogged = true + + return nil +} + +func (cd *cloggedDrain) iFixedIt() error { + cd.drainIsClogged = false + + return nil +} + +func (cd *cloggedDrain) useTheSink() error { + if cd.drainIsClogged { + return fmt.Errorf("drain is clogged") + } + + return nil +} + +// Implementation of the steps for the "Flat Tire" scenario + +type flatTire struct { + tireIsFlat bool +} + +func (ft *flatTire) addFlatTireSteps(ctx *godog.ScenarioContext) { + ctx.Step(`^I ran over a nail and got a flat tire$`, ft.gotFlatTire) + ctx.Step(`^I fixed it$`, ft.iFixedIt) + ctx.Step(`^I can continue on my way$`, ft.continueOnMyWay) +} + +func (ft *flatTire) gotFlatTire() error { + ft.tireIsFlat = true + + return nil +} + +func (ft *flatTire) iFixedIt() error { + ft.tireIsFlat = false + + return nil +} + +func (ft *flatTire) continueOnMyWay() error { + if ft.tireIsFlat { + return fmt.Errorf("tire was not fixed") + } + + return nil +} + +// standard godog global environment initialization sequence... + +var defaultOpts = godog.Options{ + Strict: true, +} + +func init() { + godog.BindFlags("godog.", flag.CommandLine, &defaultOpts) +} + +// a godog "pretty" output listener used to detect the expected godog error + +type prettyOutputListener struct { + wrapped io.Writer + callback func(string) + buf []byte +} + +func (lw *prettyOutputListener) Write(p []byte) (n int, err error) { + lw.buf = append(lw.buf, p...) + + for { + idx := bytes.IndexByte(lw.buf, '\n') + if idx == -1 { + break + } + + line := string(lw.buf[:idx]) + + lw.callback(line) + + if _, err := lw.wrapped.Write(lw.buf[:idx+1]); err != nil { + return len(p), err + } + + lw.buf = lw.buf[idx+1:] + } + + return len(p), nil +} diff --git a/_examples/dupsteps/tests/features/dupsteps.feature b/_examples/dupsteps/tests/features/dupsteps.feature new file mode 100644 index 00000000..007fe509 --- /dev/null +++ b/_examples/dupsteps/tests/features/dupsteps.feature @@ -0,0 +1,14 @@ +@dupSteps +Feature: Dupsteps example features + + @flatTire + Scenario: Flat Tire + Given I ran over a nail and got a flat tire + Then I fixed it + Then I can continue on my way + + @cloggedDrain + Scenario: Clogged Drain + Given I accidentally poured concrete down my drain and clogged the sewer line + Then I fixed it + Then I can once again use my sink diff --git a/_examples/dupsteps/go.mod b/_examples/dupsteps/tests/go.mod similarity index 69% rename from _examples/dupsteps/go.mod rename to _examples/dupsteps/tests/go.mod index 0944604e..2e5e1a37 100644 --- a/_examples/dupsteps/go.mod +++ b/_examples/dupsteps/tests/go.mod @@ -1,10 +1,10 @@ -module github.com/cucumber/godog/_examples/dupsteps +module github.com/cucumber/godog/_examples/dupsteps/tests go 1.23.1 require ( github.com/cucumber/godog v0.14.1 - github.com/stretchr/testify v1.8.2 + github.com/stretchr/testify v1.9.0 ) require ( @@ -19,6 +19,3 @@ require ( github.com/spf13/pflag v1.0.5 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) - -// See updated comment in the README - using the local source tree to get the "strict" functionality in PR-636 -replace github.com/cucumber/godog v0.14.1 => ../.. diff --git a/_examples/dupsteps/go.sum b/_examples/dupsteps/tests/go.sum similarity index 92% rename from _examples/dupsteps/go.sum rename to _examples/dupsteps/tests/go.sum index a5aeb8f4..74013d4c 100644 --- a/_examples/dupsteps/go.sum +++ b/_examples/dupsteps/tests/go.sum @@ -1,6 +1,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/cucumber/gherkin/go/v26 v26.2.0 h1:EgIjePLWiPeslwIWmNQ3XHcypPsWAHoMCz/YEBKP4GI= github.com/cucumber/gherkin/go/v26 v26.2.0/go.mod h1:t2GAPnB8maCT4lkHL99BDCVNzCh1d7dBhCLt150Nr/0= +github.com/cucumber/godog v0.14.1 h1:HGZhcOyyfaKclHjJ+r/q93iaTJZLKYW6Tv3HkmUE6+M= +github.com/cucumber/godog v0.14.1/go.mod h1:FX3rzIDybWABU4kuIXLZ/qtqEe1Ac5RdXmqvACJOces= github.com/cucumber/messages/go/v21 v21.0.1 h1:wzA0LxwjlWQYZd32VTlAVDTkW6inOFmSM+RuOwHZiMI= github.com/cucumber/messages/go/v21 v21.0.1/go.mod h1:zheH/2HS9JLVFukdrsPWoPdmUtmYQAQPLk7w5vWsk5s= github.com/cucumber/messages/go/v22 v22.0.0/go.mod h1:aZipXTKc0JnjCsXrJnuZpWhtay93k7Rn3Dee7iyPJjs= @@ -39,8 +41,9 @@ github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpE github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= From 4fc41bd587885ade0ec12ed7909a4b005e896e22 Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Thu, 10 Oct 2024 16:51:16 -0700 Subject: [PATCH 4/7] test: add proposal --- _examples/dupsteps/README.md | 98 +++++------- .../dupsteps/{tests => demo}/dupsteps_test.go | 3 +- _examples/dupsteps/{tests => demo}/go.mod | 0 _examples/dupsteps/{tests => demo}/go.sum | 0 .../cloggedDrain.feature} | 6 - _examples/dupsteps/features/flatTire.feature | 8 + _examples/dupsteps/solution/README.md | 14 ++ _examples/dupsteps/solution/proposed_test.go | 142 ++++++++++++++++++ 8 files changed, 204 insertions(+), 67 deletions(-) rename _examples/dupsteps/{tests => demo}/dupsteps_test.go (98%) rename _examples/dupsteps/{tests => demo}/go.mod (100%) rename _examples/dupsteps/{tests => demo}/go.sum (100%) rename _examples/dupsteps/{tests/features/dupsteps.feature => features/cloggedDrain.feature} (62%) create mode 100644 _examples/dupsteps/features/flatTire.feature create mode 100644 _examples/dupsteps/solution/README.md create mode 100644 _examples/dupsteps/solution/proposed_test.go diff --git a/_examples/dupsteps/README.md b/_examples/dupsteps/README.md index 39cc9586..c3b87a2e 100644 --- a/_examples/dupsteps/README.md +++ b/_examples/dupsteps/README.md @@ -1,66 +1,44 @@ # Duplicate Steps -## Updated Statement - -After submitting this example / feedback, while looking through the `godog` source -code, I came across [PR-636](https://github.com/cucumber/godog/pull/636) which deals -with the very same issue as described below - i.e., duplicate (ambiguous) steps. - +## Problem Statement + +In a testing pipeline in which many `godog` scenario test cases for a single feature were +written and maintained by different developers, it quickly became apparent that Cucumber's +approach of considering step definitions as "global" - i.e., the need for a step's text to +identify it uniquely unique across all scenarios being tested - was problematic. + +In the example provided here (see the `demo` and associated `features` folders), +the fact that the step with text `I fixed it` is used in two scenarios, but needs to +be interpreted _differently_ based upon which scenario is executing, implies that +either the development of both scenarios needs to be coordinated (i.e., to come +to a common implementation of that step, like an API), _or_ the step text needs +to be unique between them so that the implementations can be different. + +Running the tests for the two scenarios separately (e.g., using a separate +`godog.TestSuite`) would "solve" the problem, as the step implementations +would be unique within each testing context. However, a hard requirement for +a single testing phase within our build pipeline requires a single "cucumber +report" file to be produced as evidence of the success or failure of each +test scenario. And, `godog` produces a separate report for each invocation +of `godog.TestSuite`, so _something needed to change._ + +## Proposed Solution + +See a proposed "solution" of using a simple "report combiner" in conjunction +with establishment of separate, standard `go` tests, within the nested `solution` +folder. + +The main approach is to feed each `godog.TestSuite` used to partition execution +of the test's scenarios with its own `Output` buffer, and to combine them +into a single "cucumber report" meeting the needs of our build pipeline. + +## Notes + +- See [PR-636](https://github.com/cucumber/godog/pull/636) dealing with a related issue: when `godog` chooses an + incorrect step function when more than one step function matches the text + from a scenario being tested (i.e., an "ambiguous" step, such as illustrated + by the `I fixed it` step in the nested `demo` folder). - _NOTE: until the change made in [PR-636](https://github.com/cucumber/godog/pull/636) is made available (it's not yet been released as of this writing), you can use something like the [stepchecker](cmd/stepchecker/README.md) to detect such cases._ -Indeed, this accepted and merged PR wasn't available in the `godog` library version -we are using (you can be sure I'll put in an upgrade request to the team, though!), -so I was not familiar with it. - -Now that this PR is in place and enabled when using `strict` mode (which we _are_ -using), it will help us - at least our developers will receive a repeatable test -failure _right away_ when duplicate (ambiguous) steps are detected (though the -"error" message is a bit cryptic: i.e., it seems like it just says **"IS STRICT=2"** -in the given example case). However, ultimately as described below we'd like -to be able to encapsulate step implementations completely within a scenario, -and it seems this desired functionality is not (yet?) available. - -Therefore, the focus of this PR is now upon tips / suggestions for how to implement -the desired functionality of limiting the scope of a step definition to either -a scenario and/or perhaps a feature (as opposed to always globally). Or possibly, -why that's a bad idea (I find it suspicious that hasn't been done already ;-0). - -The original (pre-update above) text of this README is preserved for posterity, -below: - -## Original Statement - -This example reproduces the problem wherein duplicate steps are silently overridden. - -### Motivation - -As a relatively new user of Cucumber & its [Gherkin syntax](https://cucumber.io/docs/gherkin/), and -as an implementer of steps for a scenario using `godog`, I'd like to have the ability to encapsulate -implementation of steps for a scenario without concern that a step function from a different scenario -will be called instead of mine. At least, I'd like to have more control over automatic "re-use" of -step functions; either: (1) choose to limit the assignment of a step function to step text (or regex) -to be matched only within the scope of its enclosing scenario (preferred), (2) have `godog` throw an -error when an ambiguous match to a step function is defined or detected for a step's implementation. - -Though I've begun to understand that re-use of step implementations is somewhat fundamental to the Gherkin design -(e.g., I've read about _[feature coupled step definitions](https://cucumber.io/docs/guides/anti-patterns/?lang=java#feature-coupled-step-definitions)_ -and how Cucumber _"effectively flattens the features/ directory tree"_), it's still annoying that -the `godog` scaffold seems to _require us to conform_ to the Gherkin recommendation to _"organise -your steps by domain concept"_, and not by Feature or even Scenario, as would better suit our project. - -What's ended up happening to several of our developers in our distributed BDD testing initiative is -they end up force-tweaking the text of their steps in order to avoid duplicates, as they don't have agency -over other scenario implementations which they need to run with in our Jenkins pipeline. Later, they're -annoyed when their scenarios suddenly start failing after new scenarios are added having step -implementations with regex which _happens_ to match (thereby overriding) theirs. - -_NOTE:_ due to a limitation in our Jenkins pipeline, all of our features & scenarios _must_ be executed -within the same `godog.Suite`, else (I realize) we could just "solve" this problem by running each scenario -in its own invocation of `godog`. - -### Summary - -In light of the specifics of the "Motivation" above, the stated "problem" here might then be more -effectively re-characterized as a "request" to give more control to the end-user, as suggested above. diff --git a/_examples/dupsteps/tests/dupsteps_test.go b/_examples/dupsteps/demo/dupsteps_test.go similarity index 98% rename from _examples/dupsteps/tests/dupsteps_test.go rename to _examples/dupsteps/demo/dupsteps_test.go index 98967b6e..5145dec0 100644 --- a/_examples/dupsteps/tests/dupsteps_test.go +++ b/_examples/dupsteps/demo/dupsteps_test.go @@ -1,4 +1,4 @@ -package tests +package demo import ( "bytes" @@ -150,6 +150,7 @@ func (ft *flatTire) continueOnMyWay() error { var defaultOpts = godog.Options{ Strict: true, + Paths: []string{"../features"}, } func init() { diff --git a/_examples/dupsteps/tests/go.mod b/_examples/dupsteps/demo/go.mod similarity index 100% rename from _examples/dupsteps/tests/go.mod rename to _examples/dupsteps/demo/go.mod diff --git a/_examples/dupsteps/tests/go.sum b/_examples/dupsteps/demo/go.sum similarity index 100% rename from _examples/dupsteps/tests/go.sum rename to _examples/dupsteps/demo/go.sum diff --git a/_examples/dupsteps/tests/features/dupsteps.feature b/_examples/dupsteps/features/cloggedDrain.feature similarity index 62% rename from _examples/dupsteps/tests/features/dupsteps.feature rename to _examples/dupsteps/features/cloggedDrain.feature index 007fe509..507a5c77 100644 --- a/_examples/dupsteps/tests/features/dupsteps.feature +++ b/_examples/dupsteps/features/cloggedDrain.feature @@ -1,12 +1,6 @@ @dupSteps Feature: Dupsteps example features - @flatTire - Scenario: Flat Tire - Given I ran over a nail and got a flat tire - Then I fixed it - Then I can continue on my way - @cloggedDrain Scenario: Clogged Drain Given I accidentally poured concrete down my drain and clogged the sewer line diff --git a/_examples/dupsteps/features/flatTire.feature b/_examples/dupsteps/features/flatTire.feature new file mode 100644 index 00000000..564588b0 --- /dev/null +++ b/_examples/dupsteps/features/flatTire.feature @@ -0,0 +1,8 @@ +@dupSteps +Feature: Dupsteps example features + + @flatTire + Scenario: Flat Tire + Given I ran over a nail and got a flat tire + Then I fixed it + Then I can continue on my way diff --git a/_examples/dupsteps/solution/README.md b/_examples/dupsteps/solution/README.md new file mode 100644 index 00000000..66ee14de --- /dev/null +++ b/_examples/dupsteps/solution/README.md @@ -0,0 +1,14 @@ +# Proposed Solution + +In this folder is demonstrated a proposed solution to the "problem statement" +described in the parent `README` file related to the desire to encapsulate +step implementations within features or scenarios, yet produce a single +report file as a result. + +## Overview + +The proposed solution leverages standard `go` test scaffolding to define and +run multiple `godog` tests (e.g., each using their own `godog.TestSuite`) +for selected features or scenarios, then combine the outputs produced into +a single output file, as required in our case. + diff --git a/_examples/dupsteps/solution/proposed_test.go b/_examples/dupsteps/solution/proposed_test.go new file mode 100644 index 00000000..236aa87b --- /dev/null +++ b/_examples/dupsteps/solution/proposed_test.go @@ -0,0 +1,142 @@ +package solution + +import ( + "bytes" + "encoding/json" + "flag" + "fmt" + "os" + "testing" + + "github.com/cucumber/godog" + "github.com/cucumber/godog/internal/formatters" + "github.com/stretchr/testify/assert" +) + +// +// Demonstration of an approach to produce a single aggregated report from runs of separate test suites +// (e.g., `godog.TestSuite` instances), using standard `go` test cases. See associated README file. +// + +// the single global var needed to "collect" the output(s) produced by the test(s) +var mw = multiWriter{} + +// the main test "scaffold" which runs the test case(s), then finally aggregates the outputs into a single report + +func TestMain(m *testing.M) { + rc := m.Run() // runs the test case(s) + + // then invokes a "combiner" appropriate for the output(s) produced by the test case(s) + // NOTE: the "combiner" is formatter-specific; this one "knows" to combine "cucumber" reports + outputs, err := combineCukeOutputs(mw.getOutputs()) + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "combiner error: %s\n", err) + } else { + // hmm, it'd be nice to have some CLI options to control this destination... + fmt.Println(string(outputs)) + } + + os.Exit(rc) +} + +// one or more test case(s), providing the desired level of step encapsulation + +func TestFlatTire(t *testing.T) { + opts := defaultOpts + + // test runs only selected features/scenarios + opts.Paths = []string{"../features/flatTire.feature"} + opts.Output = mw.newOutput() + + gts := godog.TestSuite{ + Name: t.Name(), + ScenarioInitializer: func(ctx *godog.ScenarioContext) { + ctx.Step(`^I ran over a nail and got a flat tire$`, func() {}) + ctx.Step(`^I fixed it$`, func() {}) + ctx.Step(`^I can continue on my way$`, func() {}) + }, + Options: &opts, + } + + assert.Zero(t, gts.Run()) +} + +func TestCloggedDrain(t *testing.T) { + opts := defaultOpts + + // test runs only selected features/scenarios + opts.Paths = []string{"../features/cloggedDrain.feature"} + opts.Output = mw.newOutput() + + gts := godog.TestSuite{ + Name: t.Name(), + ScenarioInitializer: func(ctx *godog.ScenarioContext) { + ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, func() {}) + ctx.Step(`^I fixed it$`, func() {}) + ctx.Step(`^I can once again use my sink$`, func() {}) + + }, + Options: &opts, + } + + assert.Zero(t, gts.Run()) +} + +// multi writer utility used to collect output generated by the test case(s) + +type multiWriter struct { + bufs []*bytes.Buffer +} + +func (w *multiWriter) newOutput() *bytes.Buffer { + buf := &bytes.Buffer{} + + w.bufs = append(w.bufs, buf) + + return buf +} + +func (w *multiWriter) getOutputs() [][]byte { + outputs := make([][]byte, 0, len(w.bufs)) + + for _, buf := range w.bufs { + outputs = append(outputs, buf.Bytes()) + } + + return outputs +} + +// cucumber combiner - "knows" how to combine multiple "cucumber" reports into one + +func combineCukeOutputs(outputs [][]byte) ([]byte, error) { + var result []formatters.CukeFeatureJSON + + for _, output := range outputs { + var cukeFeatureJSONS []formatters.CukeFeatureJSON + + err := json.Unmarshal(output, &cukeFeatureJSONS) + if err != nil { + return nil, fmt.Errorf("can't unmarshal cuke feature JSON: %w", err) + } + + result = append(result, cukeFeatureJSONS...) + } + + aggregatedResults, err := json.MarshalIndent(result, "", " ") + if err != nil { + return nil, fmt.Errorf("can't marshal combined cuke feature JSON: %w", err) + } + + return aggregatedResults, nil +} + +func init() { + // allow user overrides of preferred godog defaults via command-line flags + godog.BindFlags("godog.", flag.CommandLine, &defaultOpts) +} + +// holds preferred godog defaults to be used by the test case(s) +var defaultOpts = godog.Options{ + Strict: true, + Format: "cucumber", +} From b199afcf30811e8fa1046918889ac9026ac9a18c Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Thu, 10 Oct 2024 20:25:14 -0700 Subject: [PATCH 5/7] test: add table-driven test example --- _examples/dupsteps/cmd/stepchecker/README.md | 33 ++++---- _examples/dupsteps/cmd/stepchecker/main.go | 59 ++++++-------- _examples/dupsteps/demo/dupsteps_test.go | 18 ++--- _examples/dupsteps/solution/cukecombiner.go | 32 ++++++++ _examples/dupsteps/solution/godoginit.go | 18 +++++ _examples/dupsteps/solution/multiwriter.go | 27 +++++++ .../{proposed_test.go => simple_test.go} | 75 ++---------------- _examples/dupsteps/solution/table_test.go | 76 +++++++++++++++++++ 8 files changed, 208 insertions(+), 130 deletions(-) create mode 100644 _examples/dupsteps/solution/cukecombiner.go create mode 100644 _examples/dupsteps/solution/godoginit.go create mode 100644 _examples/dupsteps/solution/multiwriter.go rename _examples/dupsteps/solution/{proposed_test.go => simple_test.go} (50%) create mode 100644 _examples/dupsteps/solution/table_test.go diff --git a/_examples/dupsteps/cmd/stepchecker/README.md b/_examples/dupsteps/cmd/stepchecker/README.md index 46b894c5..c282dde5 100644 --- a/_examples/dupsteps/cmd/stepchecker/README.md +++ b/_examples/dupsteps/cmd/stepchecker/README.md @@ -13,27 +13,26 @@ Example: ```shell $ cd ~/repos/godog/_examples/dupsteps -$ go run cmd/stepchecker/main.go tests +$ go run cmd/stepchecker/main.go -- demo/*.go features/*.feature Found 5 feature step(s): -1. "I ran over a nail and got a flat tire" -2. "I fixed it" +1. "I can continue on my way" +2. "I accidentally poured concrete down my drain and clogged the sewer line" +3. "I fixed it" - 2 matching godog step(s) found: - from: tests/features/dupsteps.feature:7 - from: tests/features/dupsteps.feature:13 - to: tests/dupsteps_test.go:93:11 - to: tests/dupsteps_test.go:125:11 -3. "I can continue on my way" -4. "I accidentally poured concrete down my drain and clogged the sewer line" -5. "I can once again use my sink" + from: features/cloggedDrain.feature:7 + from: features/flatTire.feature:7 + to: demo/dupsteps_test.go:93:11 + to: demo/dupsteps_test.go:125:11 +4. "I can once again use my sink" +5. "I ran over a nail and got a flat tire" Found 5 godog step(s): -1. "^I fixed it$" -2. "^I can once again use my sink$" -3. "^I ran over a nail and got a flat tire$" -4. "^I can continue on my way$" -5. "^I accidentally poured concrete down my drain and clogged the sewer line$" +1. "^I can continue on my way$" +2. "^I accidentally poured concrete down my drain and clogged the sewer line$" +3. "^I fixed it$" +4. "^I can once again use my sink$" +5. "^I ran over a nail and got a flat tire$" -2024/10/06 15:38:50 1 issue(s) found +2024/10/10 20:18:57 1 issue(s) found exit status 1 -$ _ ``` diff --git a/_examples/dupsteps/cmd/stepchecker/main.go b/_examples/dupsteps/cmd/stepchecker/main.go index b5d047c7..5f1744da 100644 --- a/_examples/dupsteps/cmd/stepchecker/main.go +++ b/_examples/dupsteps/cmd/stepchecker/main.go @@ -6,10 +6,8 @@ import ( "go/ast" "go/parser" "go/token" - "io/fs" "log" "os" - "path/filepath" "regexp" "strconv" "strings" @@ -21,52 +19,44 @@ import ( // func main() { - if len(os.Args) != 2 { - log.Printf("Usage: main.go \n") + if len(os.Args) < 3 { + log.Printf("Usage: main.go [go-file(s)] [feature-file(s)]\n") os.Exit(RC_USER) } - rootPath := os.Args[1] - // Structures into which to collect step patterns found in Go and feature files godogSteps := make(map[string]*StepMatch) featureSteps := make(map[string]*StepMatch) - // Walk through all files in the directory - errWalk := filepath.WalkDir(rootPath, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } + // collect input files (must have at least one of each kind (e.g., *.go, *.feature) + for _, filePath := range os.Args[1:] { + if strings.HasSuffix(filePath, ".go") { + if err := collectGoSteps(filePath, godogSteps); err != nil { + fmt.Printf("error collecting `go` steps: %s\n", err) - if d.IsDir() { - return nil + os.Exit(RC_ISSUES) + } } - if strings.HasSuffix(path, ".go") { - return collectGoSteps(path, godogSteps) - } + if strings.HasSuffix(filePath, ".feature") { + if err := collectFeatureSteps(filePath, featureSteps); err != nil { + fmt.Printf("error collecting `feature` steps: %s\n", err) - if strings.HasSuffix(path, ".feature") { - return collectFeatureSteps(path, featureSteps) + os.Exit(RC_ISSUES) + } } - - return nil - }) - - if errWalk != nil { - log.Printf("error walking directory: %v\n", errWalk) - - os.Exit(RC_SYSTEM) } if len(godogSteps) == 0 { log.Printf("no godog step definition(s) found") + os.Exit(RC_USER) } if len(featureSteps) == 0 { log.Printf("no feature step invocation(s) found") + os.Exit(RC_USER) } @@ -131,12 +121,12 @@ func main() { } } -func collectGoSteps(filePath string, steps map[string]*StepMatch) error { +func collectGoSteps(goFilePath string, steps map[string]*StepMatch) error { fset := token.NewFileSet() - node, errParse := parser.ParseFile(fset, filePath, nil, parser.ParseComments) + node, errParse := parser.ParseFile(fset, goFilePath, nil, parser.ParseComments) if errParse != nil { - return fmt.Errorf("error parsing Go file %s: %w", filePath, errParse) + return fmt.Errorf("error parsing Go file %s: %w", goFilePath, errParse) } stepDefPattern := regexp.MustCompile(`^godog.ScenarioContext.(Given|When|Then|And|Step)$`) @@ -190,14 +180,14 @@ func collectGoSteps(filePath string, steps map[string]*StepMatch) error { }) if errInspect != nil { - return fmt.Errorf("error encountered while inspecting %q: %w", filePath, errInspect) + return fmt.Errorf("error encountered while inspecting %q: %w", goFilePath, errInspect) } return nil } -func collectFeatureSteps(filePath string, steps map[string]*StepMatch) error { - file, errOpen := os.Open(filePath) +func collectFeatureSteps(featureFilePath string, steps map[string]*StepMatch) error { + file, errOpen := os.Open(featureFilePath) if errOpen != nil { return errOpen } @@ -216,7 +206,7 @@ func collectFeatureSteps(filePath string, steps map[string]*StepMatch) error { } if len(matches) != 3 { - return fmt.Errorf("unexpected number of matches at %s:%d: %d for %q\n", filePath, lineNo, len(matches), line) + return fmt.Errorf("unexpected number of matches at %s:%d: %d for %q\n", featureFilePath, lineNo, len(matches), line) } stepText := matches[2] @@ -227,7 +217,7 @@ func collectFeatureSteps(filePath string, steps map[string]*StepMatch) error { steps[stepText] = sm } - sm.source = append(sm.source, sourceRef(fmt.Sprintf("%s:%d", filePath, lineNo))) + sm.source = append(sm.source, sourceRef(fmt.Sprintf("%s:%d", featureFilePath, lineNo))) } return nil @@ -303,4 +293,3 @@ type StepMatch struct { const RC_ISSUES = 1 const RC_USER = 2 -const RC_SYSTEM = 3 diff --git a/_examples/dupsteps/demo/dupsteps_test.go b/_examples/dupsteps/demo/dupsteps_test.go index 5145dec0..e9a45e92 100644 --- a/_examples/dupsteps/demo/dupsteps_test.go +++ b/_examples/dupsteps/demo/dupsteps_test.go @@ -17,7 +17,7 @@ import ( // // The tests "pass" by demonstrating the "problem statement" discussed in this `dupsteps` // example; i.e., they are expected to "fail" when the problem is fixed, or can be fixed -// through a supported `godog` configuration option / enhancement. +// by using a `godog` configuration option / enhancement which becomes available. // // What's being demonstrated is how godog's use of a global list of steps defined across // all configured scenarios allows for indeterminate results, based upon the order in @@ -34,7 +34,7 @@ func TestFlatTireFirst(t *testing.T) { (&flatTire{}).addFlatTireSteps(ctx) (&cloggedDrain{}).addCloggedDrainSteps(ctx) }, - "drain is clogged", + "the drain is still clogged", ) } @@ -89,9 +89,9 @@ type cloggedDrain struct { } func (cd *cloggedDrain) addCloggedDrainSteps(ctx *godog.ScenarioContext) { - ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, cd.clogSewerLine) - ctx.Step(`^I fixed it$`, cd.iFixedIt) - ctx.Step(`^I can once again use my sink$`, cd.useTheSink) + ctx.Given(`^I accidentally poured concrete down my drain and clogged the sewer line$`, cd.clogSewerLine) + ctx.Then(`^I fixed it$`, cd.iFixedIt) + ctx.Then(`^I can once again use my sink$`, cd.useTheSink) } func (cd *cloggedDrain) clogSewerLine() error { @@ -108,7 +108,7 @@ func (cd *cloggedDrain) iFixedIt() error { func (cd *cloggedDrain) useTheSink() error { if cd.drainIsClogged { - return fmt.Errorf("drain is clogged") + return fmt.Errorf("the drain is still clogged") } return nil @@ -121,9 +121,9 @@ type flatTire struct { } func (ft *flatTire) addFlatTireSteps(ctx *godog.ScenarioContext) { - ctx.Step(`^I ran over a nail and got a flat tire$`, ft.gotFlatTire) - ctx.Step(`^I fixed it$`, ft.iFixedIt) - ctx.Step(`^I can continue on my way$`, ft.continueOnMyWay) + ctx.Given(`^I ran over a nail and got a flat tire$`, ft.gotFlatTire) + ctx.Then(`^I fixed it$`, ft.iFixedIt) + ctx.Then(`^I can continue on my way$`, ft.continueOnMyWay) } func (ft *flatTire) gotFlatTire() error { diff --git a/_examples/dupsteps/solution/cukecombiner.go b/_examples/dupsteps/solution/cukecombiner.go new file mode 100644 index 00000000..d93f17f8 --- /dev/null +++ b/_examples/dupsteps/solution/cukecombiner.go @@ -0,0 +1,32 @@ +package solution + +import ( + "encoding/json" + "fmt" + + "github.com/cucumber/godog/internal/formatters" +) + +// Cucumber combiner - "knows" how to combine multiple "cucumber" reports into one + +func CombineCukeOutputs(outputs [][]byte) ([]byte, error) { + var result []formatters.CukeFeatureJSON + + for _, output := range outputs { + var cukeFeatureJSONS []formatters.CukeFeatureJSON + + err := json.Unmarshal(output, &cukeFeatureJSONS) + if err != nil { + return nil, fmt.Errorf("can't unmarshal cuke feature JSON: %w", err) + } + + result = append(result, cukeFeatureJSONS...) + } + + aggregatedResults, err := json.MarshalIndent(result, "", " ") + if err != nil { + return nil, fmt.Errorf("can't marshal combined cuke feature JSON: %w", err) + } + + return aggregatedResults, nil +} diff --git a/_examples/dupsteps/solution/godoginit.go b/_examples/dupsteps/solution/godoginit.go new file mode 100644 index 00000000..5642692a --- /dev/null +++ b/_examples/dupsteps/solution/godoginit.go @@ -0,0 +1,18 @@ +package solution + +import ( + "flag" + + "github.com/cucumber/godog" +) + +func init() { + // allow user overrides of preferred godog defaults via command-line flags + godog.BindFlags("godog.", flag.CommandLine, &defaultOpts) +} + +// holds preferred godog defaults to be used by the test case(s) +var defaultOpts = godog.Options{ + Strict: true, + Format: "cucumber", +} diff --git a/_examples/dupsteps/solution/multiwriter.go b/_examples/dupsteps/solution/multiwriter.go new file mode 100644 index 00000000..d8639406 --- /dev/null +++ b/_examples/dupsteps/solution/multiwriter.go @@ -0,0 +1,27 @@ +package solution + +import "bytes" + +// Multi writer utility used to collect output generated by the test case(s) + +type MultiWriter struct { + bufs []*bytes.Buffer +} + +func (w *MultiWriter) NewOutput() *bytes.Buffer { + buf := &bytes.Buffer{} + + w.bufs = append(w.bufs, buf) + + return buf +} + +func (w *MultiWriter) GetOutputs() [][]byte { + outputs := make([][]byte, 0, len(w.bufs)) + + for _, buf := range w.bufs { + outputs = append(outputs, buf.Bytes()) + } + + return outputs +} diff --git a/_examples/dupsteps/solution/proposed_test.go b/_examples/dupsteps/solution/simple_test.go similarity index 50% rename from _examples/dupsteps/solution/proposed_test.go rename to _examples/dupsteps/solution/simple_test.go index 236aa87b..4976472b 100644 --- a/_examples/dupsteps/solution/proposed_test.go +++ b/_examples/dupsteps/solution/simple_test.go @@ -1,25 +1,21 @@ package solution import ( - "bytes" - "encoding/json" - "flag" "fmt" "os" "testing" "github.com/cucumber/godog" - "github.com/cucumber/godog/internal/formatters" "github.com/stretchr/testify/assert" ) // -// Demonstration of an approach to produce a single aggregated report from runs of separate test suites -// (e.g., `godog.TestSuite` instances), using standard `go` test cases. See associated README file. +// Demonstration of a unit testing approach for producing a single aggregated report from runs of separate test suites +// (e.g., `godog.TestSuite` instances), using standard `go` simple test cases. See associated README file. // // the single global var needed to "collect" the output(s) produced by the test(s) -var mw = multiWriter{} +var mw = MultiWriter{} // the main test "scaffold" which runs the test case(s), then finally aggregates the outputs into a single report @@ -28,7 +24,7 @@ func TestMain(m *testing.M) { // then invokes a "combiner" appropriate for the output(s) produced by the test case(s) // NOTE: the "combiner" is formatter-specific; this one "knows" to combine "cucumber" reports - outputs, err := combineCukeOutputs(mw.getOutputs()) + outputs, err := CombineCukeOutputs(mw.GetOutputs()) if err != nil { _, _ = fmt.Fprintf(os.Stderr, "combiner error: %s\n", err) } else { @@ -46,7 +42,7 @@ func TestFlatTire(t *testing.T) { // test runs only selected features/scenarios opts.Paths = []string{"../features/flatTire.feature"} - opts.Output = mw.newOutput() + opts.Output = mw.NewOutput() gts := godog.TestSuite{ Name: t.Name(), @@ -66,7 +62,7 @@ func TestCloggedDrain(t *testing.T) { // test runs only selected features/scenarios opts.Paths = []string{"../features/cloggedDrain.feature"} - opts.Output = mw.newOutput() + opts.Output = mw.NewOutput() gts := godog.TestSuite{ Name: t.Name(), @@ -81,62 +77,3 @@ func TestCloggedDrain(t *testing.T) { assert.Zero(t, gts.Run()) } - -// multi writer utility used to collect output generated by the test case(s) - -type multiWriter struct { - bufs []*bytes.Buffer -} - -func (w *multiWriter) newOutput() *bytes.Buffer { - buf := &bytes.Buffer{} - - w.bufs = append(w.bufs, buf) - - return buf -} - -func (w *multiWriter) getOutputs() [][]byte { - outputs := make([][]byte, 0, len(w.bufs)) - - for _, buf := range w.bufs { - outputs = append(outputs, buf.Bytes()) - } - - return outputs -} - -// cucumber combiner - "knows" how to combine multiple "cucumber" reports into one - -func combineCukeOutputs(outputs [][]byte) ([]byte, error) { - var result []formatters.CukeFeatureJSON - - for _, output := range outputs { - var cukeFeatureJSONS []formatters.CukeFeatureJSON - - err := json.Unmarshal(output, &cukeFeatureJSONS) - if err != nil { - return nil, fmt.Errorf("can't unmarshal cuke feature JSON: %w", err) - } - - result = append(result, cukeFeatureJSONS...) - } - - aggregatedResults, err := json.MarshalIndent(result, "", " ") - if err != nil { - return nil, fmt.Errorf("can't marshal combined cuke feature JSON: %w", err) - } - - return aggregatedResults, nil -} - -func init() { - // allow user overrides of preferred godog defaults via command-line flags - godog.BindFlags("godog.", flag.CommandLine, &defaultOpts) -} - -// holds preferred godog defaults to be used by the test case(s) -var defaultOpts = godog.Options{ - Strict: true, - Format: "cucumber", -} diff --git a/_examples/dupsteps/solution/table_test.go b/_examples/dupsteps/solution/table_test.go new file mode 100644 index 00000000..0002e94b --- /dev/null +++ b/_examples/dupsteps/solution/table_test.go @@ -0,0 +1,76 @@ +package solution + +import ( + "fmt" + "os" + "testing" + + "github.com/cucumber/godog" + "github.com/stretchr/testify/assert" +) + +// +// Demonstration of a unit testing approach for producing a single aggregated report from runs of separate test suites +// (e.g., `godog.TestSuite` instances), using standard `go` table-driven tests. See associated README file. +// + +// the main unit test runs the case(s) defined in the table, then finally aggregates the outputs into a single report + +func TestSeparateScenarios(t *testing.T) { + tests := []struct { + name string + paths []string + steps func(ctx *godog.ScenarioContext) + }{ + { + name: "flat tire", + paths: []string{"../features/flatTire.feature"}, + steps: func(ctx *godog.ScenarioContext) { + ctx.Step(`^I ran over a nail and got a flat tire$`, func() {}) + ctx.Step(`^I fixed it$`, func() {}) + ctx.Step(`^I can continue on my way$`, func() {}) + }, + }, + { + name: "clogged sink", + paths: []string{"../features/cloggedDrain.feature"}, + steps: func(ctx *godog.ScenarioContext) { + ctx.Step(`^I accidentally poured concrete down my drain and clogged the sewer line$`, func() {}) + ctx.Step(`^I fixed it$`, func() {}) + ctx.Step(`^I can once again use my sink$`, func() {}) + }, + }, + } + + outputCollector := MultiWriter{} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + opts := defaultOpts + + // test runs only selected features/scenarios + opts.Paths = test.paths + opts.Output = outputCollector.NewOutput() + + gts := godog.TestSuite{ + Name: t.Name(), + ScenarioInitializer: test.steps, + Options: &opts, + } + + assert.Zero(t, gts.Run()) + }) + } + + // then invokes a "combiner" appropriate for the output(s) produced by the test case(s) + // NOTE: the "combiner" is formatter-specific; this one "knows" to combine "cucumber" reports + outputs, err := CombineCukeOutputs(outputCollector.GetOutputs()) + if err != nil { + _, _ = fmt.Fprintf(os.Stderr, "combiner error: %s\n", err) + return + } + + // route the combined output to where it should go... + // hmm, it'd be nice to have some CLI options to control this destination... + fmt.Println(string(outputs)) +} From 3b4a26cf54a2850a2092d3c7c3c491370ce01b6d Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Thu, 10 Oct 2024 20:28:08 -0700 Subject: [PATCH 6/7] fix: might as well specify format explicitly here --- _examples/dupsteps/solution/table_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/_examples/dupsteps/solution/table_test.go b/_examples/dupsteps/solution/table_test.go index 0002e94b..3a0a9107 100644 --- a/_examples/dupsteps/solution/table_test.go +++ b/_examples/dupsteps/solution/table_test.go @@ -50,6 +50,7 @@ func TestSeparateScenarios(t *testing.T) { // test runs only selected features/scenarios opts.Paths = test.paths + opts.Format = "cucumber" opts.Output = outputCollector.NewOutput() gts := godog.TestSuite{ From b012f43e6de1b22c49f54f1c7e87fd7f37bcbe25 Mon Sep 17 00:00:00 2001 From: Marty Ross Date: Fri, 11 Oct 2024 12:03:32 -0700 Subject: [PATCH 7/7] docs: clarify --- _examples/dupsteps/README.md | 65 +++++++++++---------- _examples/dupsteps/solution/README.md | 6 +- _examples/dupsteps/solution/cukecombiner.go | 15 +++-- _examples/dupsteps/solution/multiwriter.go | 7 ++- _examples/dupsteps/solution/simple_test.go | 11 ++-- _examples/dupsteps/solution/table_test.go | 9 ++- 6 files changed, 57 insertions(+), 56 deletions(-) diff --git a/_examples/dupsteps/README.md b/_examples/dupsteps/README.md index c3b87a2e..0b770322 100644 --- a/_examples/dupsteps/README.md +++ b/_examples/dupsteps/README.md @@ -2,43 +2,46 @@ ## Problem Statement -In a testing pipeline in which many `godog` scenario test cases for a single feature were -written and maintained by different developers, it quickly became apparent that Cucumber's -approach of considering step definitions as "global" - i.e., the need for a step's text to -identify it uniquely unique across all scenarios being tested - was problematic. - -In the example provided here (see the `demo` and associated `features` folders), -the fact that the step with text `I fixed it` is used in two scenarios, but needs to -be interpreted _differently_ based upon which scenario is executing, implies that -either the development of both scenarios needs to be coordinated (i.e., to come -to a common implementation of that step, like an API), _or_ the step text needs -to be unique between them so that the implementations can be different. - -Running the tests for the two scenarios separately (e.g., using a separate -`godog.TestSuite`) would "solve" the problem, as the step implementations -would be unique within each testing context. However, a hard requirement for -a single testing phase within our build pipeline requires a single "cucumber -report" file to be produced as evidence of the success or failure of each -test scenario. And, `godog` produces a separate report for each invocation -of `godog.TestSuite`, so _something needed to change._ +In a testing pipeline in which many `godog` Scenario test cases for a single Feature are run +within a single `godog.TestSuite` (in order to produce a single report of results), it quickly +became apparent that Cucumber's requirement for a global one-to-one pairing between the Pattern +used to match the text of a Step to the Function implementing it is problematic. + +In the illustrative example provided (see the [demo](./demo) and associated [features](./features) folders), +Steps with matching text (e.g., `I fixed it`) appear in two Scenarios; but, each calls +for a _different_ implementation, specific to its Scenario. Cucumber's requirement (as +mentioned above) would force a single implementation of this Step across both Scenarios. +To accommodate this requirement, either the Gherkin (i.e., the _given_ business language) +would have to change, or coding best practices (e.g., Single Responsibility Principle, +Separation of Concerns, Modularity, Encapsulation, Cohesion, etc.) would have to give. + +Running the tests for the two Scenarios _separately_ (e.g., using a separate `godog.TestSuite`) +could "solve" the problem, as matching the common Step text to its scenario-specific Function +would then be unambiguous within the Scenario-specific testing run context. However, a hard requirement +within our build pipeline requires a single "cucumber report" file to be produced as evidence +of the success or failure of _all_ required test Scenarios. Because `godog` produces a +_separate_ report for each invocation of `godog.TestSuite`, _something had to change._ + +## Problem Detection + +A ["step checker" tool](cmd/stepchecker/README.md) was created to facilitate early detection +of the problem situation described above. Using this tool while modifying or adding tests +for new scenarios given by the business was proven useful as part of a "shift left" testing +strategy. ## Proposed Solution -See a proposed "solution" of using a simple "report combiner" in conjunction -with establishment of separate, standard `go` tests, within the nested `solution` -folder. +A ["solution" was proposed](solution/README.md) of using a simple "report combiner" in conjunction +with establishment of separate, standard `go` tests. -The main approach is to feed each `godog.TestSuite` used to partition execution -of the test's scenarios with its own `Output` buffer, and to combine them -into a single "cucumber report" meeting the needs of our build pipeline. +The main idea in the proposed "solution" _is to use separate_ `godog.TestSuite` instances +to partition execution of the test Scenarios, then collect and combine their output into +the single "cucumber report" required by our build pipeline. ## Notes - See [PR-636](https://github.com/cucumber/godog/pull/636) dealing with a related issue: when `godog` chooses an - incorrect step function when more than one step function matches the text - from a scenario being tested (i.e., an "ambiguous" step, such as illustrated - by the `I fixed it` step in the nested `demo` folder). -- _NOTE: until the change made in [PR-636](https://github.com/cucumber/godog/pull/636) - is made available (it's not yet been released as of this writing), you can use something - like the [stepchecker](cmd/stepchecker/README.md) to detect such cases._ + incorrect Step Function when more than one matches the text + from a Scenario being tested (i.e., an "ambiguous" Step, such as illustrated + by the `I fixed it` Step in the nested `demo` folder). diff --git a/_examples/dupsteps/solution/README.md b/_examples/dupsteps/solution/README.md index 66ee14de..ffb99426 100644 --- a/_examples/dupsteps/solution/README.md +++ b/_examples/dupsteps/solution/README.md @@ -2,13 +2,13 @@ In this folder is demonstrated a proposed solution to the "problem statement" described in the parent `README` file related to the desire to encapsulate -step implementations within features or scenarios, yet produce a single +Step implementations within Features or Scenarios, yet produce a single report file as a result. ## Overview The proposed solution leverages standard `go` test scaffolding to define and run multiple `godog` tests (e.g., each using their own `godog.TestSuite`) -for selected features or scenarios, then combine the outputs produced into -a single output file, as required in our case. +for selected Features or Scenarios, then combine the outputs produced into +a single report file, as required in our case. diff --git a/_examples/dupsteps/solution/cukecombiner.go b/_examples/dupsteps/solution/cukecombiner.go index d93f17f8..35054dee 100644 --- a/_examples/dupsteps/solution/cukecombiner.go +++ b/_examples/dupsteps/solution/cukecombiner.go @@ -7,12 +7,11 @@ import ( "github.com/cucumber/godog/internal/formatters" ) -// Cucumber combiner - "knows" how to combine multiple "cucumber" reports into one +// CombineCukeReports "knows" how to combine multiple "cucumber" reports into one +func CombineCukeReports(cukeReportOutputs [][]byte) ([]byte, error) { + var allCukeFeatureJSONs []formatters.CukeFeatureJSON -func CombineCukeOutputs(outputs [][]byte) ([]byte, error) { - var result []formatters.CukeFeatureJSON - - for _, output := range outputs { + for _, output := range cukeReportOutputs { var cukeFeatureJSONS []formatters.CukeFeatureJSON err := json.Unmarshal(output, &cukeFeatureJSONS) @@ -20,13 +19,13 @@ func CombineCukeOutputs(outputs [][]byte) ([]byte, error) { return nil, fmt.Errorf("can't unmarshal cuke feature JSON: %w", err) } - result = append(result, cukeFeatureJSONS...) + allCukeFeatureJSONs = append(allCukeFeatureJSONs, cukeFeatureJSONS...) } - aggregatedResults, err := json.MarshalIndent(result, "", " ") + combinedCukeReport, err := json.MarshalIndent(allCukeFeatureJSONs, "", " ") if err != nil { return nil, fmt.Errorf("can't marshal combined cuke feature JSON: %w", err) } - return aggregatedResults, nil + return combinedCukeReport, nil } diff --git a/_examples/dupsteps/solution/multiwriter.go b/_examples/dupsteps/solution/multiwriter.go index d8639406..e83db8e2 100644 --- a/_examples/dupsteps/solution/multiwriter.go +++ b/_examples/dupsteps/solution/multiwriter.go @@ -2,13 +2,13 @@ package solution import "bytes" -// Multi writer utility used to collect output generated by the test case(s) - +// MultiWriter utility used to collect output generated by the test case(s) type MultiWriter struct { bufs []*bytes.Buffer } -func (w *MultiWriter) NewOutput() *bytes.Buffer { +// NewWriter allocates and returns a new writer used to create output +func (w *MultiWriter) NewWriter() *bytes.Buffer { buf := &bytes.Buffer{} w.bufs = append(w.bufs, buf) @@ -16,6 +16,7 @@ func (w *MultiWriter) NewOutput() *bytes.Buffer { return buf } +// GetOutputs returns the output(s) written by any/all of the writers given out so far... func (w *MultiWriter) GetOutputs() [][]byte { outputs := make([][]byte, 0, len(w.bufs)) diff --git a/_examples/dupsteps/solution/simple_test.go b/_examples/dupsteps/solution/simple_test.go index 4976472b..e6158605 100644 --- a/_examples/dupsteps/solution/simple_test.go +++ b/_examples/dupsteps/solution/simple_test.go @@ -17,19 +17,18 @@ import ( // the single global var needed to "collect" the output(s) produced by the test(s) var mw = MultiWriter{} -// the main test "scaffold" which runs the test case(s), then finally aggregates the outputs into a single report - +// TestMain runs the test case(s), then combines the outputs into a single report func TestMain(m *testing.M) { rc := m.Run() // runs the test case(s) // then invokes a "combiner" appropriate for the output(s) produced by the test case(s) // NOTE: the "combiner" is formatter-specific; this one "knows" to combine "cucumber" reports - outputs, err := CombineCukeOutputs(mw.GetOutputs()) + combinedReport, err := CombineCukeReports(mw.GetOutputs()) if err != nil { _, _ = fmt.Fprintf(os.Stderr, "combiner error: %s\n", err) } else { // hmm, it'd be nice to have some CLI options to control this destination... - fmt.Println(string(outputs)) + fmt.Println(string(combinedReport)) } os.Exit(rc) @@ -42,7 +41,7 @@ func TestFlatTire(t *testing.T) { // test runs only selected features/scenarios opts.Paths = []string{"../features/flatTire.feature"} - opts.Output = mw.NewOutput() + opts.Output = mw.NewWriter() gts := godog.TestSuite{ Name: t.Name(), @@ -62,7 +61,7 @@ func TestCloggedDrain(t *testing.T) { // test runs only selected features/scenarios opts.Paths = []string{"../features/cloggedDrain.feature"} - opts.Output = mw.NewOutput() + opts.Output = mw.NewWriter() gts := godog.TestSuite{ Name: t.Name(), diff --git a/_examples/dupsteps/solution/table_test.go b/_examples/dupsteps/solution/table_test.go index 3a0a9107..839d20b8 100644 --- a/_examples/dupsteps/solution/table_test.go +++ b/_examples/dupsteps/solution/table_test.go @@ -14,8 +14,7 @@ import ( // (e.g., `godog.TestSuite` instances), using standard `go` table-driven tests. See associated README file. // -// the main unit test runs the case(s) defined in the table, then finally aggregates the outputs into a single report - +// TestSeparateScenarios runs the case(s) defined in the table, then combines the outputs into a single report func TestSeparateScenarios(t *testing.T) { tests := []struct { name string @@ -51,7 +50,7 @@ func TestSeparateScenarios(t *testing.T) { // test runs only selected features/scenarios opts.Paths = test.paths opts.Format = "cucumber" - opts.Output = outputCollector.NewOutput() + opts.Output = outputCollector.NewWriter() gts := godog.TestSuite{ Name: t.Name(), @@ -65,7 +64,7 @@ func TestSeparateScenarios(t *testing.T) { // then invokes a "combiner" appropriate for the output(s) produced by the test case(s) // NOTE: the "combiner" is formatter-specific; this one "knows" to combine "cucumber" reports - outputs, err := CombineCukeOutputs(outputCollector.GetOutputs()) + combinedReport, err := CombineCukeReports(outputCollector.GetOutputs()) if err != nil { _, _ = fmt.Fprintf(os.Stderr, "combiner error: %s\n", err) return @@ -73,5 +72,5 @@ func TestSeparateScenarios(t *testing.T) { // route the combined output to where it should go... // hmm, it'd be nice to have some CLI options to control this destination... - fmt.Println(string(outputs)) + fmt.Println(string(combinedReport)) }