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

add new option for created features with parsing from byte slices #476

Merged
5 changes: 5 additions & 0 deletions internal/flags/options.go
Expand Up @@ -64,4 +64,9 @@ 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 map[string][]byte
Copy link
Member

@vearutop vearutop Jul 24, 2022

Choose a reason for hiding this comment

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

Let's make this a slice of structs to improve clarity of data items.
Something like:

	// Features allow passing in additional features manually.
	Features []Feature
}

// Feature contains path and contents of a gherkin file.
type Feature struct {
	Path    string
	Content []byte
}

}
49 changes: 49 additions & 0 deletions internal/parser/parser.go
Expand Up @@ -53,6 +53,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 {
Expand Down Expand Up @@ -162,6 +178,39 @@ func ParseFeatures(filter string, paths []string) ([]*models.Feature, error) {
return features, nil
}

func ParseFromBytes(filter string, featuresInputs map[string][]byte) ([]*models.Feature, error) {
var order int

featureIdxs := make(map[string]int)
uniqueFeatureURI := make(map[string]*models.Feature)
newIDFunc := (&messages.Incrementing{}).NewId
for path, feature := range featuresInputs {
ft, err := parseBytes(path, feature, 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)
Expand Down
37 changes: 37 additions & 0 deletions internal/parser/parser_test.go
Expand Up @@ -37,6 +37,43 @@ func Test_FeatureFilePathParser(t *testing.T) {
}
}

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 := map[string][]byte{
filepath.Join(baseDir, featureFileName): []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
Expand Down
7 changes: 6 additions & 1 deletion run.go
Expand Up @@ -227,7 +227,12 @@ 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 {
if len(opt.FeatureContents) > 0 {
runner.features, err = parser.ParseFromBytes(opt.Tags, opt.FeatureContents)
} else {
runner.features, err = parser.ParseFeatures(opt.Tags, opt.Paths)
}
Copy link
Member

Choose a reason for hiding this comment

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

Let's change logic here so that both Paths and bytes can work together. We can check for non-empty opt.Paths and append runner.features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, I'm not so sure how simple this would be. From what I can see, if someone wants to run through FeatureContents only, and they leave the Paths option blank, the current behavior will force them to use a features directory by default. Either they'll have their features already in a features directory, or they won't, in which case there could be a failure due to non-existent directory, no? I think it's maybe just simpler to say, either you can use feature contents, or features, but not both. If someone is already parsing files and passing them in, I can't imagine it's much more challenging to also include from all features as opposed to from only a few. what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Hm, this is interesting point.

If someone is already parsing files and passing them in, I can't imagine it's much more challenging to also include from all features as opposed to from only a few.

Another case when this feature could be useful would be sourcing gherkin files from a remote storage (e.g. via HTTP) or generating/templating them by code. In such situation implementing additional file preloader might be a burden.

If we disable that default in case of feature contents presence it may be perceived as unexpected behavior.

	if len(opt.Paths) == 0 && len(opt.Features) == 0 {
		inf, err := os.Stat("features")
		if err == nil && inf.IsDir() {
			opt.Paths = []string{"features"}
		}
	}

If we ignore Paths (make feature contents and paths mutually exclusive) this can also be perceived as unexpected behavior (that needs to be documented or maybe even have a check and explicit failure to not mislead user).
And implementing preloader can be a minor annoyance for a user in case they want both types of features.

From these options I'm leaning to disabling the default as it seems less disruptive overall.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good!

if err != nil {
fmt.Fprintln(os.Stderr, err)
return exitOptionError
}
Expand Down
40 changes: 40 additions & 0 deletions run_test.go
Expand Up @@ -263,6 +263,46 @@ 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)

featureContents := make(map[string][]byte)
for _, item := range items {
if !item.IsDir() && strings.Contains(item.Name(), ".feature"){
contents, err := os.ReadFile("./features/"+item.Name())
require.NoError(t, err)
featureContents[item.Name()] = 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 bufErrorPipe(t *testing.T) (io.ReadCloser, func()) {
stderr := os.Stderr
r, w, err := os.Pipe()
Expand Down