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.

Sign up for GitHub

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

fix: ftltest config: use ConfigPaths with envar fallback #1543

Merged
merged 3 commits into from
May 21, 2024
Merged

fix: ftltest config: use ConfigPaths with envar fallback #1543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e31b468
fix: ftltest config: use ConfigPaths with envar fallback
safeer May 20, 2024
27b49ca
chore: pin codemirror npm module (#1550)
deniseli May 21, 2024
a8d0b8f
chore: update comment to reflect WithProjectFiles behavior
safeer May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions go-runtime/ftl/ftltest/ftltest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/TBD54566975/ftl/backend/schema"
cf "github.com/TBD54566975/ftl/common/configuration"
pc "github.com/TBD54566975/ftl/common/projectconfig"
"github.com/TBD54566975/ftl/go-runtime/ftl"
"github.com/TBD54566975/ftl/go-runtime/ftl/reflection"
"github.com/TBD54566975/ftl/internal/log"
Expand Down Expand Up @@ -59,7 +60,8 @@ func Context(options ...Option) context.Context {
// WithProjectFiles loads config and secrets from a project file
//
// Takes a list of paths to project files. If multiple paths are provided, they are loaded in order, with later files taking precedence.
// If no paths are provided, the list is inferred from the FTL_CONFIG environment variable. If that is not found, an error is returned.
// If no paths are provided, the list is inferred from the FTL_CONFIG environment variable. If that is not found, the ftl-project.toml
// file in the git root is used. If ftl-project.toml is not found, no project files are loaded.
//
// To be used when setting up a context for a test:
// ctx := ftltest.Context(
Expand All @@ -73,10 +75,11 @@ func WithProjectFiles(paths ...string) Option {
var preprocessingErr error
if len(paths) == 0 {
envValue, ok := os.LookupEnv("FTL_CONFIG")
if !ok {
preprocessingErr = fmt.Errorf("loading project files: no path provided and FTL_CONFIG environment variable not set")
if ok {
paths = strings.Split(envValue, ",")
} else {
paths = pc.ConfigPaths(paths)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we confirm that paths isn't empty and return the same preprocessingErr error from before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intention here is to have the same behavior as main.go, which allows LoadConfig to return an empty config after attempting to fall back on the envar, then the gitroot. I think having that parity makes sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. I was wondering because I can't think of a reason why someone would use WithProjectFiles without any paths. From that perspective, I'd find it useful to get the error message during development so that I know the way I'm using WithProjectFiles isn't actually doing anything. main.go wouldn't want to error in that case because the project toml isn't required, whereas if someone is explicitly invoking WithProjectFiles, I'd assume they have some intent to actually use a project toml.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's clear that if you're not passing explicit config files to WithProjectFiles() that whatever its described behaviour is is in effect. The ftl-project.toml file is designed to be the "default" for a project, and having it consistent across the CLI and tests is fine.

}
paths = strings.Split(envValue, ",")
}
paths = slices.Map(paths, func(p string) string {
path, err := filepath.Abs(p)
Expand Down
12 changes: 12 additions & 0 deletions integration/testdata/go/wrapped/ftl-project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
ftl-min-version = ""

[global]
[global.configuration]
config = "inline://ImJhemJheiI"
[global.secrets]
secret = "inline://ImJhemJheiI"

[executables]
ftl = ""

[commands]
51 changes: 46 additions & 5 deletions integration/testdata/go/wrapped/wrapped_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,54 @@ import (
"github.com/alecthomas/assert/v2"
)

func TestWrapped(t *testing.T) {
func TestWrappedWithConfigEnvar(t *testing.T) {
absProjectPath1, err := filepath.Abs("ftl-project-test-1.toml")
assert.NoError(t, err)
absProjectPath2, err := filepath.Abs("ftl-project-test-2.toml")
assert.NoError(t, err)
t.Setenv("FTL_CONFIG", fmt.Sprintf("%s,%s", absProjectPath1, absProjectPath2))

for _, tt := range []struct {
name string
options []ftltest.Option
configValue string
secretValue string
expectedError ftl.Option[string]
}{
{
name: "WithProjectTomlFromEnvar",
options: []ftltest.Option{
ftltest.WithProjectFiles(),
ftltest.WithCallsAllowedWithinModule(),
ftltest.WhenVerb(time.Time, func(ctx context.Context, req time.TimeRequest) (time.TimeResponse, error) {
return time.TimeResponse{Time: stdtime.Date(2024, 1, 1, 0, 0, 0, 0, stdtime.UTC)}, nil
}),
},
configValue: "foobar",
secretValue: "foobar",
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx := ftltest.Context(
tt.options...,
)
myConfig.Get(ctx)
resp, err := Outer(ctx)

if expected, ok := tt.expectedError.Get(); ok {
assert.EqualError(t, err, expected)
return
} else {
assert.NoError(t, err)
}
assert.Equal(t, "2024-01-01 00:00:00 +0000 UTC", resp.Output)
assert.Equal(t, tt.configValue, resp.Config)
assert.Equal(t, tt.secretValue, resp.Secret)
})
}
}

func TestWrapped(t *testing.T) {
for _, tt := range []struct {
name string
options []ftltest.Option
Expand Down Expand Up @@ -62,7 +103,7 @@ func TestWrapped(t *testing.T) {
secretValue: "shhhhh",
},
{
name: "WithProjectToml",
name: "WithProjectTomlSpecified",
options: []ftltest.Option{
ftltest.WithProjectFiles("ftl-project-test-1.toml"),
ftltest.WithCallsAllowedWithinModule(),
Expand All @@ -73,16 +114,16 @@ func TestWrapped(t *testing.T) {
configValue: "bar",
secretValue: "bar",
}, {
name: "WithProjectTomlFromEnvar",
name: "WithProjectTomlFromRoot",
options: []ftltest.Option{
ftltest.WithProjectFiles(),
ftltest.WithCallsAllowedWithinModule(),
ftltest.WhenVerb(time.Time, func(ctx context.Context, req time.TimeRequest) (time.TimeResponse, error) {
return time.TimeResponse{Time: stdtime.Date(2024, 1, 1, 0, 0, 0, 0, stdtime.UTC)}, nil
}),
},
configValue: "foobar",
secretValue: "foobar",
configValue: "bazbaz",
secretValue: "bazbaz",
},
} {
t.Run(tt.name, func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion renovate.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"enabled": false
},
{
"matchPackageNames": ["eslint"],
"matchPackageNames": ["eslint", "codemirror"],
"enabled": false,
"paths": ["frontend/**", "extensions/**"]
},
Expand Down
Loading