Repository: cucumber/godog
Pull request #366 (2020-12-15 16:09:36)

make pickleStepIDs unique accross multiple paths
Ran into trouble with pickleStepID not being unique when having the feature files on multiple paths.

In the failing scenario we have a bunch of features distributed along multiple paths.
I experienced a nil pointer dereference in the pretty printer when executing a scenario outline pickle.
The pickleStep received from `pickleStep = f.storage.MustGetPickleStep(result.PickleStepID)` was originating from a different feature from a different path

codecov[bot]: # [Codecov](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=h1) Report
> Merging [#366](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=desc) (fa51281) into [master](https://codecov.io/gh/cucumber/godog/commit/adfc936dd7e280a063023d5cb1786b74d2c46e28?el=desc) (adfc936) will **not change** coverage.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/cucumber/godog/pull/366/graphs/tree.svg?width=650&height=150&src=pr&token=qCtLO7dhcu)](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #366   +/-   ##
=======================================
  Coverage   79.12%   79.12%           
=======================================
  Files          25       25           
  Lines        1993     1993           
=======================================
  Hits         1577     1577           
  Misses        320      320           
  Partials       96       96           
```


| [Impacted Files](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/parser/parser.go](https://codecov.io/gh/cucumber/godog/pull/366/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcGFyc2VyL3BhcnNlci5nbw==) | `9.45% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=footer). Last update [adfc936...fa51281](https://codecov.io/gh/cucumber/godog/pull/366?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/internal/parser/parser.go b/internal/parser/parser.go
index 40c2895..8d9fc9e 100644
--- a/internal/parser/parser.go
+++ b/internal/parser/parser.go
@@ -78,7 +78,7 @@ func parseFeatureDir(dir string, newIDFunc func() string) ([]*models.Feature, er
 	})
 }
 
-func parsePath(path string) ([]*models.Feature, error) {
+func parsePath(path string, newIDFunc func() string) ([]*models.Feature, error) {
 	var features []*models.Feature
 
 	path, line := ExtractFeaturePathLine(path)
@@ -88,8 +88,6 @@ func parsePath(path string) ([]*models.Feature, error) {
 		return features, err
 	}
 
-	newIDFunc := (&messages.Incrementing{}).NewId
-
 	if fi.IsDir() {
 		return parseFeatureDir(path, newIDFunc)
 	}
@@ -119,8 +117,9 @@ func ParseFeatures(filter string, paths []string) ([]*models.Feature, error) {
 
 	featureIdxs := make(map[string]int)
 	uniqueFeatureURI := make(map[string]*models.Feature)
+	newIDFunc := (&messages.Incrementing{}).NewId
 	for _, path := range paths {
-		feats, err := parsePath(path)
+		feats, err := parsePath(path, newIDFunc)
 
 		switch {
 		case os.IsNotExist(err):


Test patch:
diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go
index b7f2f7c..19fbf85 100644
--- a/internal/parser/parser_test.go
+++ b/internal/parser/parser_test.go
@@ -1,9 +1,13 @@
 package parser_test
 
 import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/cucumber/godog/internal/parser"
 )
@@ -32,3 +36,44 @@ func Test_FeatureFilePathParser(t *testing.T) {
 		assert.Equal(t, ln, c.line)
 	}
 }
+
+func Test_ParseFeatures_FromMultiplePaths(t *testing.T) {
+	const featureFileName = "godogs.feature"
+	const featureFileContents = `Feature: eat godogs
+  In order to be happy
+  As a hungry gopher
+  I need to be able to eat godogs
+
+  Scenario: Eat 5 out of 12
+    Given there are 12 godogs
+    When I eat 5
+		Then there should be 7 remaining`
+
+	baseDir := filepath.Join(os.TempDir(), t.Name(), "godogs")
+	errA := os.MkdirAll(baseDir+"/a", 0755)
+	errB := os.MkdirAll(baseDir+"/b", 0755)
+	defer os.RemoveAll(baseDir)
+
+	require.Nil(t, errA)
+	require.Nil(t, errB)
+
+	err := ioutil.WriteFile(filepath.Join(baseDir+"/a", featureFileName), []byte(featureFileContents), 0644)
+	require.Nil(t, err)
+	err = ioutil.WriteFile(filepath.Join(baseDir+"/b", featureFileName), []byte(featureFileContents), 0644)
+	require.Nil(t, err)
+
+	features, err := parser.ParseFeatures("", []string{baseDir + "/a", baseDir + "/b"})
+	assert.Nil(t, err)
+	assert.Len(t, features, 2)
+
+	pickleIDs := map[string]bool{}
+	for _, f := range features {
+		for _, p := range f.Pickles {
+			if pickleIDs[p.Id] {
+				assert.Failf(t, "found duplicate pickle ID", "Pickle ID %s was already used", p.Id)
+			}
+
+			pickleIDs[p.Id] = true
+		}
+	}
+}