Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add duplicate steps example and "Motivation" README #642

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ Gopkg.toml
_artifacts

vendor
godog.iml
47 changes: 47 additions & 0 deletions _examples/dupsteps/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Duplicate Steps

## Problem Statement

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

A ["solution" was proposed](solution/README.md) of using a simple "report combiner" in conjunction
with establishment of separate, standard `go` tests.

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 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).

38 changes: 38 additions & 0 deletions _examples/dupsteps/cmd/stepchecker/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# 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 -- demo/*.go features/*.feature
Found 5 feature step(s):
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: 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 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/10 20:18:57 1 issue(s) found
exit status 1
```
3 changes: 3 additions & 0 deletions _examples/dupsteps/cmd/stepchecker/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module github.com/cucumber/godog/_examples/dupsteps/cmd/stepchecker

go 1.23.1
295 changes: 295 additions & 0 deletions _examples/dupsteps/cmd/stepchecker/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,295 @@
package main

import (
"bufio"
"fmt"
"go/ast"
"go/parser"
"go/token"
"log"
"os"
"regexp"
"strconv"
"strings"
)

//
// See accompanying README file(s);
// also https://github.com/cucumber/godog/pull/642
//

func main() {
if len(os.Args) < 3 {
log.Printf("Usage: main.go [go-file(s)] [feature-file(s)]\n")

os.Exit(RC_USER)
}

// Structures into which to collect step patterns found in Go and feature files
godogSteps := make(map[string]*StepMatch)
featureSteps := make(map[string]*StepMatch)

// 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)

os.Exit(RC_ISSUES)
}
}

if strings.HasSuffix(filePath, ".feature") {
if err := collectFeatureSteps(filePath, featureSteps); err != nil {
fmt.Printf("error collecting `feature` steps: %s\n", err)

os.Exit(RC_ISSUES)
}
}
}

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(goFilePath string, steps map[string]*StepMatch) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know if this static parser will spot the cases where the ScenarioContext has been wrapped.
I've seen (and I use) a scheme where folk register steps via a proxy wrapper of the ScenarioContext, that adds generic behaviours to the steps being registered.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! 👍

If you provide an example, I'll work up a test case to fix.

Do you have a fix strategy in mind?

fset := token.NewFileSet()

node, errParse := parser.ParseFile(fset, goFilePath, nil, parser.ParseComments)
if errParse != nil {
return fmt.Errorf("error parsing Go file %s: %w", goFilePath, 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", goFilePath, errInspect)
}

return nil
}

func collectFeatureSteps(featureFilePath string, steps map[string]*StepMatch) error {
file, errOpen := os.Open(featureFilePath)
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", featureFilePath, 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", featureFilePath, 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
Loading