-
Notifications
You must be signed in to change notification settings - Fork 486
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
Wire up support for converting integrations-next configs to flow. #5973
Changes from 10 commits
5969326
5dde688
e50647f
cc710b6
7570e3c
c5db4d6
d394132
5fca267
1f089bf
2468bb6
228e7ba
b00d236
011904a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package flowmode | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestParseExtraArgs(t *testing.T) { | ||
type testCase struct { | ||
name string | ||
extraArgs string | ||
expected []string | ||
} | ||
|
||
var testCases = []testCase{ | ||
{ | ||
name: "full", | ||
extraArgs: "--key=value", | ||
expected: []string{"--key", "value"}, | ||
}, | ||
{ | ||
name: "shorthand", | ||
extraArgs: "-k=value", | ||
expected: []string{"-k", "value"}, | ||
}, | ||
{ | ||
name: "bool", | ||
extraArgs: "--boolVariable", | ||
expected: []string{"--boolVariable"}, | ||
}, | ||
{ | ||
name: "spaced", | ||
extraArgs: "--key value", | ||
expected: []string{"--key", "value"}, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
res, err := parseExtraArgs(tc.extraArgs) | ||
require.NoError(t, err) | ||
require.Equal(t, tc.expected, res) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ func Convert(in []byte, extraArgs []string) ([]byte, diag.Diagnostics) { | |
var diags diag.Diagnostics | ||
|
||
fs := flag.NewFlagSet("convert", flag.ContinueOnError) | ||
args := []string{"-config.file", "convert", "-config.expand-env"} | ||
args := []string{"-config.file", "convert"} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove this hard coded since we can now pass this in and have more control over the converter. This was the lesser of 2 evils before to automatically expand environment vars. |
||
args = append(args, extraArgs...) | ||
staticConfig, err := config.LoadFromFunc(fs, args, func(_, _ string, expandEnvVars bool, c *config.Config) error { | ||
return config.LoadBytes(in, expandEnvVars, c) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,8 @@ import ( | |
) | ||
|
||
func TestConvert(t *testing.T) { | ||
test_common.TestDirectory(t, "testdata", ".yaml", true, []string{}, staticconvert.Convert) | ||
test_common.TestDirectory(t, "testdata-v2", ".yaml", true, []string{"-enable-features", "integrations-next"}, staticconvert.Convert) | ||
test_common.TestDirectory(t, "testdata", ".yaml", true, []string{"-config.expand-env"}, staticconvert.Convert) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is no longer hard coded to be added so we need to pass it in for the tests. |
||
test_common.TestDirectory(t, "testdata-v2", ".yaml", true, []string{"-enable-features", "integrations-next", "-config.expand-env"}, staticconvert.Convert) | ||
|
||
if runtime.GOOS == "windows" { | ||
test_common.TestDirectory(t, "testdata_windows", ".yaml", true, []string{}, staticconvert.Convert) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
server: | ||
log_level: ${SOME_ENVIRONMENT_VARIABLE:='debug'} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added an integration-next test for env vars to validate it also works. |
||
log_format: json | ||
|
||
metrics: | ||
global: | ||
remote_write: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for addressing this! I'd only add maybe a couple of comments, e.g. the two cases in for loop for
--flag
and-flag
. Some test cases for failed parses or the--key="foo=bar"
may be a good idea too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once I did some deeper testing, I made some more significant improvements to the func. I added comments throughout due to the complexity which will hopefully help future us