Repository: cucumber/godog
Pull request #476 (2022-05-13 08:54:43)

add new option for created features with parsing from byte slices
Hello!  In my work we are trying to build in a pre-processor for our feature files so we can have some dynamic feature generation when running our tests.  I made this for us to use but though maybe it could be interesting for more people than just us.  I've added another option to define custom features to be used in the suite, and then added an interface to pass in a map[string][]byte representing the path/read contents from the file (in our case, the contents after doing preprocessing).  I think this implementation is maybe not fully working since it seems to be missing the line filtering, but other than that I think I've got all the other inner workings there.  Just wanted to submit it if it could be interesting for anyone!

Leave me a comment if you think this is a good idea and then I can try adding some testing/additional documentation

<!---
Thanks for helping to make Cucumber better! 💖

You can feel free to open a "Draft" status pull request if you're not ready for feedback yet.

Don't worry about getting everything perfect! We're here to help and will coach you through
to getting your pull request ready to merge.

The prompts below are for guidance to help you describe your change in a way that is most 
likely to make sense to other people when they are reviewing it. Still, it's just a guide, 
so feel free to delete anything that doesn't feel appropriate, and add anything additional 
that seems like it would probide useful context. 👏🏻
-->

### 🤔 What's changed?

<!-- Describe your changes in detail -->

### ⚡️ What's your motivation? 

<!-- 
What motivated you to propose this change? Does it fix a bug? Add a new feature?
If it fixes an open issue, you can link to the issue here, e.g. "Fixes #99"
-->

### 🏷️ What kind of change is this?

<!--- Delete any options that are not relevant -->

- :bank: Refactoring/debt/DX (improvement to code design, tooling, documentation etc. without changing behaviour)
- :bug: Bug fix (non-breaking change which fixes a defect)
- :zap: New feature (non-breaking change which adds new behaviour)
- :boom: Breaking change (incompatible changes to the API)

### ♻️ Anything particular you want feedback on?

<!-- 
Is there anything in this change you're unsure about, or would 
particularly like reviewers to give you feedback on?
-->

### 📋 Checklist:

<!--- 
This is to help you remember all the little things we often forget to do!

Feel free to delete any tasks that are not relevant, or add new ones.
-->

- [x] I agree to respect and uphold the [Cucumber Community Code of Conduct](https://cucumber.io/conduct/)
- [ ] I've changed the behaviour of the code
  - [ ] I have added/updated tests to cover my changes.
- [ ] My change requires a change to the documentation.
  - [ ] I have updated the documentation accordingly.
- [ ] Users should know about my change
  - [ ] I have added an entry to the "Unreleased" section of the [**CHANGELOG**](../CHANGELOG.md), linking to this pull request.

----

*This text was originally generated from a [template](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/about-issue-and-pull-request-templates), then edited by hand. [You can modify the template here.](https://github.com/cucumber/.github/edit/main/.github/PULL_REQUEST_TEMPLATE.md)*


vearutop: This is an interesting feature and I think it can be very helpful to improve portability/embedability of tests. Let me check it in more detail to see if the API of this can be improved to be more accessible.
mattwynne: Thanks for sharing! Can you add some tests for this @akaswenwilk?
akaswenwilk: > Thanks for sharing! Can you add some tests for this @akaswenwilk?

Sure thing!
akaswenwilk: hi @mattwynne , so sorry for the delay to get back to this.  I've added some tests for the ParseFromBytes function in the parser, but now I'm trying to address the interface a bit.  Currently I've added a Features option where someone can manually pass in a slice of features, but maybe actually it makes more sense to simple add a different option instead (like FeatureContents) which would be a map[string][]byte of all the features to be run.  Otherwise the user has to generate a slice of Feature models using the RetrieveFeaturesFromBytes method.  Any thoughts on which might be nicer?  At which point I can try adding a test in run_test.go and maybe we can finalize this? Thanks for your help!
codecov[bot]: # [Codecov](https://codecov.io/gh/cucumber/godog/pull/476?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber) Report
> Merging [#476](https://codecov.io/gh/cucumber/godog/pull/476?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber) (bef3d7c) into [main](https://codecov.io/gh/cucumber/godog/commit/5e176da4338ad55ca074f7932b1b5dd38819890f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber) (5e176da) will **decrease** coverage by `0.37%`.
> The diff coverage is `61.53%`.

```diff
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   81.72%   81.34%   -0.38%     
==========================================
  Files          27       27              
  Lines        2232     2268      +36     
==========================================
+ Hits         1824     1845      +21     
- Misses        312      324      +12     
- Partials       96       99       +3     
```


| [Impacted Files](https://codecov.io/gh/cucumber/godog/pull/476?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber) | Coverage Δ | |
|---|---|---|
| [run.go](https://codecov.io/gh/cucumber/godog/pull/476/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber#diff-cnVuLmdv) | `72.39% <30.76%> (-4.08%)` | :arrow_down: |
| [internal/parser/parser.go](https://codecov.io/gh/cucumber/godog/pull/476/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cucumber#diff-aW50ZXJuYWwvcGFyc2VyL3BhcnNlci5nbw==) | `62.50% <76.92%> (+4.80%)` | :arrow_up: |

------

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

akaswenwilk: @vearutop, sorry for the delay, but how about this for improved API?  This way the user can just pass in a map[string][]bytes directly as an option and not have to deal with trying to first parse their bytes into a feature to pass in as an option.  Could be a bit simpler?
vearutop: @akaswenwilk overall looks good to me (apart from 2 minor suggestions that I left). 👍 
wichert: I'm wondering if it makes sense to provide an ability to pass in an `fs.FS` instance, along with a wrapper to expose your map as something that implements FS.
vearutop: I think `fs.FS` does not have enough info (you need to know names in advance).
```go
// An FS provides access to a hierarchical file system.
//
// The FS interface is the minimum implementation required of the file system.
// A file system may implement additional interfaces,
// such as ReadFileFS, to provide additional or optimized functionality.
type FS interface {
	// Open opens the named file.
	//
	// When Open returns an error, it should be of type *PathError
	// with the Op field set to "open", the Path field set to name,
	// and the Err field describing the problem.
	//
	// Open should reject attempts to open names that do not satisfy
	// ValidPath(name), returning a *PathError with Err set to
	// ErrInvalid or ErrNotExist.
	Open(name string) (File, error)
}
```

We could use `fs.ReadDirFS`
```go
// ReadDirFS is the interface implemented by a file system
// that provides an optimized implementation of ReadDir.
type ReadDirFS interface {
	FS

	// ReadDir reads the named directory
	// and returns a list of directory entries sorted by filename.
	ReadDir(name string) ([]DirEntry, error)
}
```
to be able to traverse entries, but I'm not sure it would be more convenient to use instead of a list of named bytes.

Patch:
diff --git a/internal/flags/options.go b/internal/flags/options.go
index 69e39d4..a0eb126 100644
--- a/internal/flags/options.go
+++ b/internal/flags/options.go
@@ -64,4 +64,14 @@ type Options struct {
 
 	// TestingT runs scenarios as subtests.
 	TestingT *testing.T
+
+	// FeatureContents allows passing in each feature manually
+	// where the contents of each feature is stored as a byte slice
+	// in a map entry
+	FeatureContents []Feature
+}
+
+type Feature struct {
+	Name     string
+	Contents []byte
 }
diff --git a/internal/parser/parser.go b/internal/parser/parser.go
index c505cde..e5d945d 100644
--- a/internal/parser/parser.go
+++ b/internal/parser/parser.go
@@ -13,6 +13,7 @@ import (
 	"github.com/cucumber/gherkin-go/v19"
 	"github.com/cucumber/messages-go/v16"
 
+	"github.com/cucumber/godog/internal/flags"
 	"github.com/cucumber/godog/internal/models"
 	"github.com/cucumber/godog/internal/tags"
 )
@@ -53,6 +54,22 @@ func parseFeatureFile(path string, newIDFunc func() string) (*models.Feature, er
 	return &f, nil
 }
 
+func parseBytes(path string, feature []byte, newIDFunc func() string) (*models.Feature, error) {
+	reader := bytes.NewReader(feature)
+
+	var buf bytes.Buffer
+	gherkinDocument, err := gherkin.ParseGherkinDocument(io.TeeReader(reader, &buf), newIDFunc)
+	if err != nil {
+		return nil, fmt.Errorf("%s - %v", path, err)
+	}
+
+	gherkinDocument.Uri = path
+	pickles := gherkin.Pickles(*gherkinDocument, path, newIDFunc)
+
+	f := models.Feature{GherkinDocument: gherkinDocument, Pickles: pickles, Content: buf.Bytes()}
+	return &f, nil
+}
+
 func parseFeatureDir(dir string, newIDFunc func() string) ([]*models.Feature, error) {
 	var features []*models.Feature
 	return features, filepath.Walk(dir, func(p string, f os.FileInfo, err error) error {
@@ -162,6 +179,41 @@ func ParseFeatures(filter string, paths []string) ([]*models.Feature, error) {
 	return features, nil
 }
 
+type FeatureContent = flags.Feature
+
+func ParseFromBytes(filter string, featuresInputs []FeatureContent) ([]*models.Feature, error) {
+	var order int
+
+	featureIdxs := make(map[string]int)
+	uniqueFeatureURI := make(map[string]*models.Feature)
+	newIDFunc := (&messages.Incrementing{}).NewId
+	for _, f := range featuresInputs {
+		ft, err := parseBytes(f.Name, f.Contents, newIDFunc)
+		if err != nil {
+			return nil, err
+		}
+
+		if _, duplicate := uniqueFeatureURI[ft.Uri]; duplicate {
+			continue
+		}
+
+		uniqueFeatureURI[ft.Uri] = ft
+		featureIdxs[ft.Uri] = order
+
+		order++
+	}
+
+	var features = make([]*models.Feature, len(uniqueFeatureURI))
+	for uri, feature := range uniqueFeatureURI {
+		idx := featureIdxs[uri]
+		features[idx] = feature
+	}
+
+	features = filterFeatures(filter, features)
+
+	return features, nil
+}
+
 func filterFeatures(filter string, features []*models.Feature) (result []*models.Feature) {
 	for _, ft := range features {
 		ft.Pickles = tags.ApplyTagFilter(filter, ft.Pickles)
diff --git a/run.go b/run.go
index 5fa6e15..f837b92 100644
--- a/run.go
+++ b/run.go
@@ -213,7 +213,7 @@ func runWithOptions(suiteName string, runner runner, opt Options) int {
 		return exitOptionError
 	}
 
-	if len(opt.Paths) == 0 {
+	if len(opt.Paths) == 0 && len(opt.FeatureContents) == 0 {
 		inf, err := os.Stat("features")
 		if err == nil && inf.IsDir() {
 			opt.Paths = []string{"features"}
@@ -226,10 +226,22 @@ func runWithOptions(suiteName string, runner runner, opt Options) int {
 
 	runner.fmt = multiFmt.FormatterFunc(suiteName, output)
 
-	var err error
-	if runner.features, err = parser.ParseFeatures(opt.Tags, opt.Paths); err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		return exitOptionError
+	if len(opt.FeatureContents) > 0 {
+		features, err := parser.ParseFromBytes(opt.Tags, opt.FeatureContents)
+		if err != nil {
+			fmt.Fprintln(os.Stderr, err)
+			return exitOptionError
+		}
+		runner.features = append(runner.features, features...)
+	}
+
+	if len(opt.Paths) > 0 {
+		features, err := parser.ParseFeatures(opt.Tags, opt.Paths)
+		if err != nil {
+			fmt.Fprintln(os.Stderr, err)
+			return exitOptionError
+		}
+		runner.features = append(runner.features, features...)
 	}
 
 	runner.storage = storage.NewStorage()


Test patch:
diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go
index 19fbf85..f03dfbf 100644
--- a/internal/parser/parser_test.go
+++ b/internal/parser/parser_test.go
@@ -37,6 +37,64 @@ func Test_FeatureFilePathParser(t *testing.T) {
 	}
 }
 
+func Test_ParseFromBytes_FromMultipleFeatures_DuplicateNames(t *testing.T) {
+	eatGodogContents := `
+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`
+	input := []parser.FeatureContent{
+		{Name: "MyCoolDuplicatedFeature", Contents: []byte(eatGodogContents)},
+		{Name: "MyCoolDuplicatedFeature", Contents: []byte(eatGodogContents)},
+	}
+
+	featureFromBytes, err := parser.ParseFromBytes("", input)
+	require.NoError(t, err)
+	require.Len(t, featureFromBytes, 1)
+}
+
+func Test_ParseFromBytes_FromMultipleFeatures(t *testing.T) {
+	featureFileName := "godogs.feature"
+	eatGodogContents := `
+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)
+	defer os.RemoveAll(baseDir)
+
+	require.Nil(t, errA)
+
+	err := ioutil.WriteFile(filepath.Join(baseDir, featureFileName), []byte(eatGodogContents), 0644)
+	require.Nil(t, err)
+
+	featureFromFile, err := parser.ParseFeatures("", []string{baseDir})
+	require.NoError(t, err)
+	require.Len(t, featureFromFile, 1)
+
+	input := []parser.FeatureContent{
+		{Name: filepath.Join(baseDir, featureFileName), Contents: []byte(eatGodogContents)},
+	}
+
+	featureFromBytes, err := parser.ParseFromBytes("", input)
+	require.NoError(t, err)
+	require.Len(t, featureFromBytes, 1)
+
+	assert.Equal(t, featureFromFile, featureFromBytes)
+}
+
 func Test_ParseFeatures_FromMultiplePaths(t *testing.T) {
 	const featureFileName = "godogs.feature"
 	const featureFileContents = `Feature: eat godogs
diff --git a/run_test.go b/run_test.go
index b2f9bd2..9c07a6d 100644
--- a/run_test.go
+++ b/run_test.go
@@ -263,6 +263,92 @@ func Test_ByDefaultRunsFeaturesPath(t *testing.T) {
 	assert.Equal(t, exitSuccess, status)
 }
 
+func Test_RunsWithFeatureContentsOption(t *testing.T) {
+	items, err := ioutil.ReadDir("./features")
+	require.NoError(t, err)
+
+	var featureContents []Feature
+	for _, item := range items {
+		if !item.IsDir() && strings.Contains(item.Name(), ".feature") {
+			contents, err := os.ReadFile("./features/" + item.Name())
+			require.NoError(t, err)
+			featureContents = append(featureContents, Feature{
+				Name:     item.Name(),
+				Contents: contents,
+			})
+		}
+	}
+
+	opts := Options{
+		Format:          "progress",
+		Output:          ioutil.Discard,
+		Strict:          true,
+		FeatureContents: featureContents,
+	}
+
+	status := TestSuite{
+		Name:                "fails",
+		ScenarioInitializer: func(_ *ScenarioContext) {},
+		Options:             &opts,
+	}.Run()
+
+	// should fail in strict mode due to undefined steps
+	assert.Equal(t, exitFailure, status)
+
+	opts.Strict = false
+	status = TestSuite{
+		Name:                "succeeds",
+		ScenarioInitializer: func(_ *ScenarioContext) {},
+		Options:             &opts,
+	}.Run()
+
+	// should succeed in non strict mode due to undefined steps
+	assert.Equal(t, exitSuccess, status)
+}
+
+func Test_RunsWithFeatureContentsAndPathsOptions(t *testing.T) {
+	featureContents := []Feature{
+		{
+			Name: "MySuperCoolFeature",
+			Contents: []byte(`
+Feature: run features from bytes
+  Scenario: should run a normal feature
+    Given a feature "normal.feature" file:
+      """
+      Feature: normal feature
+
+        Scenario: parse a scenario
+          Given a feature path "features/load.feature:6"
+          When I parse features
+          Then I should have 1 scenario registered
+      """
+    When I run feature suite
+    Then the suite should have passed
+    And the following steps should be passed:
+      """
+      a feature path "features/load.feature:6"
+      I parse features
+      I should have 1 scenario registered
+      """`),
+		},
+	}
+
+	opts := Options{
+		Format:          "progress",
+		Output:          ioutil.Discard,
+		Paths:           []string{"./features"},
+		FeatureContents: featureContents,
+	}
+
+	status := TestSuite{
+		Name:                "succeeds",
+		ScenarioInitializer: func(_ *ScenarioContext) {},
+		Options:             &opts,
+	}.Run()
+
+	assert.Equal(t, exitSuccess, status)
+}
+
 func bufErrorPipe(t *testing.T) (io.ReadCloser, func()) {
 	stderr := os.Stderr
 	r, w, err := os.Pipe()
diff --git a/test_context.go b/test_context.go
index 862280e..fbbe981 100644
--- a/test_context.go
+++ b/test_context.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/cucumber/godog/formatters"
 	"github.com/cucumber/godog/internal/builder"
+	"github.com/cucumber/godog/internal/flags"
 	"github.com/cucumber/godog/internal/models"
 	"github.com/cucumber/messages-go/v16"
 )
@@ -316,3 +317,5 @@ func (ctx *ScenarioContext) Step(expr, stepFunc interface{}) {
 func Build(bin string) error {
 	return builder.Build(bin)
 }
+
+type Feature = flags.Feature