add mockability, make tests work #111
Conversation
|
@obs-gh-alexlew I made a few changes so the tests pass, let me know what you think! If it looks good, I can rebase all of your changes off of main and add this to your original PR |
| paths := make([]string, 0) | ||
| for _, field := range c.ConfigFields { | ||
| val := rawConnConfig.GetBool(field.configYAMLPath) | ||
| if val && field.colConfigFilePath != "" { | ||
| configPath, _ := c.RenderConfigTemplate(ctx, tmpDir, field.colConfigFilePath, confValues) |
There was a problem hiding this comment.
if we fail to render a fragment, should we still start the agent?
There was a problem hiding this comment.
Good question; I think it's better to exit. I'll change this
| } | ||
|
|
||
| func (c *ConnectionType) GetConfigFilePaths(ctx context.Context, tmpDir string) []string { | ||
| var rawConnConfig = viper.Sub(c.Name) | ||
| var rawConnConfig = c.getConfig() |
There was a problem hiding this comment.
why do we pass the whole config into each fragment?
There was a problem hiding this comment.
This is still just the sub config by default, but having a dynamic function to retrieve it allows for mocks in the test.
There was a problem hiding this comment.
ah i see, you've moved the function onto the struct
cmd/commands/start/start.go
Outdated
| } | ||
| configFilePaths, overridePath, err := config.GetAllOtelConfigFilePaths(ctx, tmpDir) | ||
| if err != nil { | ||
| os.RemoveAll(tmpDir) |
There was a problem hiding this comment.
why don't we just call cleanup here? we've already created the override file right?
| return c | ||
| } | ||
|
|
||
| func WithConfigFolderPath(configFolderPath string) ConnectionTypeOption { |
There was a problem hiding this comment.
are these just test helper functions?
There was a problem hiding this comment.
Yes, this was recommended to me as a way to allow tests to override the defaults.
| suite.Run(t, new(ConnectionsTestSuite)) | ||
| } | ||
|
|
||
| func (suite *ConnectionsTestSuite) TestConnections_RenderConfigTemplate() { |
There was a problem hiding this comment.
would be good to check that variable types beyond string get rendered correctly, i.e. arrays or even nested structs
|
looks great! nice job |
### Description OB-37310 Process all config fragments as golang templates Replaces #100 and #111 ### Checklist - [x] Created tests which fail without the change (if possible) - [ ] Extended the README / documentation, if necessary Co-authored-by: Alex Lew <[email protected]>
No description provided.