Add support for loading features from fs.FS #146
Conversation
sagikazarmark
force-pushed
the
features-fs
branch
from
656c830 to
560a4fd
Compare
gobdd_go1_16.go
Outdated
|
|
||
| // WithFeaturesFS configures a filesystem and a pattern (regexp) where features can be found. | ||
| // An empty path defaults to "*.feature" | ||
| func WithFeaturesFS(fs fs.FS, pattern string) func(*SuiteOptions) { |
There was a problem hiding this comment.
Should we add a separate function that doesn't require a pattern? Might be slightly better than having to pass an empty string as a second parameter.
| "github.com/go-bdd/gobdd/features" | ||
| ) | ||
|
|
||
| func TestWithFeaturesFS(t *testing.T) { |
There was a problem hiding this comment.
Any ideas for better tests?
There was a problem hiding this comment.
I think the test is OK. I have some thoughts about the API (see commend below).
| "github.com/go-bdd/gobdd/features" | ||
| ) | ||
|
|
||
| func TestWithFeaturesFS(t *testing.T) { |
There was a problem hiding this comment.
I think the test is OK. I have some thoughts about the API (see commend below).
gobdd_go1_16_test.go
Outdated
| ) | ||
|
|
||
| func TestWithFeaturesFS(t *testing.T) { | ||
| suite := NewSuite(t, WithFeaturesFS(features.Features(), "example.feature")) |
There was a problem hiding this comment.
thought: if the pattern is optional, we can make it truly optional. I can see a few options.
- use the
WithXYZpattern
suite := NewSuite(t, WithFeaturesFS(features.Features(), WithPattern("example.feature")))- Make the pattern to be a slice and allow to put multiple (or zero) patterns
suite := NewSuite(t, WithFeaturesFS(features.Features(), "example1.feature", "example2.feature", "example3.feature"))- Allow providing any
io.Reader
suite := NewSuite(t, WithFeaturesFS(reader))If you have any other ideas, pls let me know. For now, I'd prefer the second or third option.
There was a problem hiding this comment.
I don't think option three is the same concept. (For that we could make the featureSource interface public at some point)
Pattern is not optional, but it (currently) falls back to *.feature which means all feature files.
Maybe it's easier to just make it a mandatory parameter?
suite := NewSuite(t, WithFeaturesFS(features.Features(), "*.feature"))
… sources Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
sagikazarmark
force-pushed
the
features-fs
branch
from
a317bc0 to
f5bf4f5
Compare
|
I thought about the PR and I think I don't have a better idea. I have one more request for you - could you update the docs as well? I'd be useful for others to find out about the new feature :) |
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
This reverts commit fb14f48.
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
|
After some thinking, I decided to make the pattern/path parameter mandatory without fallback. Implicit behavior is always harder to understand for the reader. Having to spell out I also added some docs as requested. |
Signed-off-by: Mark Sagi-Kazar <mark.sagikazar@gmail.com>
This PR adds backward compatible support for loading features from alternative sources (like fs.FS filesystems).
Fixes #143
TODO