Refactor config command to improve testability #2319
Conversation
01247a1 to
e86b07e
Compare
| return fmt.Errorf("invalid key") | ||
| } | ||
|
|
||
| func ValidateValue(key, value string) error { |
There was a problem hiding this comment.
Are the exported Validate* functions to be used anywhere?
There was a problem hiding this comment.
They are used in the config set command. Specifically in the setRun function.
| } | ||
|
|
||
| func (c *fileConfig) Set(hostname, key, value string) error { | ||
| if err := validateConfigEntry(key, value); err != nil { |
There was a problem hiding this comment.
This removes validation in Set and does not replace it with anything. Is this a regression?
There was a problem hiding this comment.
I did change the responsibility of the function here by removing the validation and putting it directly in the config set command. Specifically in the setRun function. This was the only caller of the Set function using user input, that needs validation, that I could find so it felt okay to move the responsibility of checking the validity of the value up to the config set command where the user input is actually coming in.
77e4904 to
58b93e7
Compare
This PR consists of two main changes.
The first is a small cleanup of
internal/config/config_type.goandinternal/config/config_type_test.goto remove duplicate config option variables and also add testing around validating configuration keys.The second is a slightly larger refactor of the
configcommand. It extracts both thegetandsetcommand into separate files and changes their structure to match recently implemented commands, where the setup of the command is separated from the execution of the command, for increased testability. It also updates and adds test coverage for both of the commands.There are no behavior changes in any of the
configcommands.