Switch to gcp from gce #2488
Switch to gcp from gce #2488
Conversation
| ) | ||
|
|
||
| func TestNormalizeGcp(t *testing.T) { | ||
| expectedCfgMap, err := confmaptest.LoadConf("testdata/normalize_gcp/upstream_agent_config_post_migration.yaml") |
There was a problem hiding this comment.
imo it greatly improves readability and lessens maintenance requirements to use confmap.NewFromStringMap() directly instead of having separate files.
There was a problem hiding this comment.
I like the file approach to make it easier to copy a troublesome or relevant customer/upstream config as-is into the test folders, and prefer to have the "logic" separated from the test data to reduce visual scope, but I don't have a strong opinion and could be swayed if you're invested enough to make a counter argument on this.
As-is I'd say "if someone else on the team agrees with your take I'll treat that as consensus and make the change, otherwise plan to keep the same".
There was a problem hiding this comment.
One argument that would convince me would be "You can reduce the copy-pasta in your configs if you generate it in code", since that would reduce the size of our test suite.
| @@ -0,0 +1,35 @@ | |||
| receivers: | |||
There was a problem hiding this comment.
Not anymore... thank you 😔
There was a problem hiding this comment.
fwiw I think its more expansive checks could be helpful in testing
Co-authored-by: Antoine Toulme <[email protected]>
as specified in breaking changes, need to convert these
I'm sure there's some cleaner ways to organize the code and am receptive to any suggestions if you have some.