-
Notifications
You must be signed in to change notification settings - Fork 617
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
Refactor config loader tests #199
Labels
Milestone
Comments
magiconair
added a commit
that referenced
this issue
Nov 30, 2016
The config loader test was not testing the full roundtrip of the command line arguments, environment variables and config files from files and URLs. Various smaller tests were only testing individual parts of the config loader. This made updating and refactoring the config loading code more vulnerable to regressions. This refactor replaces all smaller tests with a single test suite that tests all aspects of the config loading in the same way as it is used by the main() function. New tests can be added easily and consistently. At the same time, variables which store temporary values for the config loader have been removed from the main config structure as they are redundant and confusing since no part of the application needs access to the raw argument values.
magiconair
added a commit
that referenced
this issue
Dec 6, 2016
The config loader test was not testing the full roundtrip of the command line arguments, environment variables and config files from files and URLs. Various smaller tests were only testing individual parts of the config loader. This made updating and refactoring the config loading code more vulnerable to regressions. This refactor replaces all smaller tests with a single test suite that tests all aspects of the config loading in the same way as it is used by the main() function. New tests can be added easily and consistently. At the same time, variables which store temporary values for the config loader have been removed from the main config structure as they are redundant and confusing since no part of the application needs access to the raw argument values.
magiconair
added a commit
that referenced
this issue
Dec 6, 2016
Moving the circonus config variables into a struct to avoid confusing them when calling helper functions.
Merged to master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In an effort to rewrite/refactor/consolidate more tests as full-roundtrip integration like tests the config loading tests need to be merged in a way that tests the way the arguments are parsed from the
main()
function. The current tests are too small and do not test the full round-trip which makes extending and refactoring the config code brittle and unnecessarily vulnerable for regressions.The text was updated successfully, but these errors were encountered: