Skip to content

Commit

Permalink
Fix config struct processing, add (much needed) tests
Browse files Browse the repository at this point in the history
Added:

- Tests (partial implementation/coverage)
  - validate the most common code-paths (much more TODO)
    - config handling
    - logging setup
  - run tests as part of GitHub Actions Workflow
    - recursively
    - after getting deps, but before linting steps
      in order to allow early failure
  - update Makefile to run tests recursively, verbosely
- Fix configuration precedence handling
- Add string slice equality check from StackOverflow

Changed:

- Fix configuration precedence handling
  - Config file loses to everything except default config settings
- `config.logBuffer` moved to `logging.LogBuffer`
  - internal detail exposed for general use
    - may change this later
- Updated code (where apparent) to be more test friendly
  - e.g., use `io.Reader` instead of filename so that we can use
    an in-memory TOML config file for testing config parsing
    and precedence validation
- Split config package into smaller files in an effort to make
  related code easier to manage
- Fail if requested config file not found
  - previously an error was logged, but execution was allowed
    to continue.
- Move default values to Getter methods
  - use those methods instead of directly dereferencing config
    struct fields in the majority of the code
  - use Getter methods to guard against nil dereferences
- Partial work to de-couple reliance on `Config{}` (see #170)

Deprecated:

Both of these functions from the `config` package do not appear
to be needed any longer, but are being kept for a cycle in case
I change my mind:

- `Config.SetDefaultConfig()`
- `Config.GetStructTag()`

Fixed:

- Fix configuration precedence handling
  - Config settings are merged properly, so that even default
    settings are allowed to override lower precedence config
    sources
- NumFilesToKeep default (didn't match v0.5.1 change)
- Add missing `default` switch statements along with error
  return codes for `Set` functions with specific valid option values
- README updates
  - cover config file flag, environment variables
  - config precedence corrections
- Multiple linting errors (with more that needs to be evaluated)
- Add missing exit if IgnoreErrors is false
- Handle unintended nil assignment
- Update GoDoc doc file
  - reflect config file support, including updated Help text

TODO:

- Update multiple tests to use "tables" instead of hard-coding
  specific checks (which ends up being very lengthy)
  - e.g., `TestValidate()`

refs #161, #156, #170
  • Loading branch information
atc0005 committed Dec 12, 2019
1 parent 1c24a42 commit e5fa0b6
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 0 deletions.
114 changes: 114 additions & 0 deletions COMMITS.LOG
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
785850d Remove scatch notes used to setup config tests
3b53749 Doc updates that attempt to fix precedence details to match current implementation
a4f1dc6 Stub out TestEnableSyslogLogging func, add missing default switch statement
cf5c728 Replace actual phrasing with got to be more idomatic
b904432 WIP: AppMetadata fields are no longer user configurable
c83db6b Finished testing complete and incomplete config struct merges
43d30f5 WIP: Partial work towards testing complete and incomplete config struct merges
6819d94 WIP: Partial work towards testing complete and incomplete config struct merges
ef6847f WIP: Update MergeConfig() to bring over ConfigFile values if set in source config
3ec8b99 WIP: First stab at implementing flags config testing. The ConfigFile bits are still problematic
71d4561 Update example FileExtensions entries to reflect env vars spacing requirements
d295d13 Reactivate TestMergeConfig
cee725b Fix unintentional space in extensions slice, update formatting of test output to quote slice entries to make mistakes like this easier to spot
d9fcd83 Defer env var unset via anonymous func
cd883cc Unset env vars after we are done using them for testing
a2a0200 Fix MergeConfigTest() call, args
4bc6283 Fix log level setting
2bc6b45 Stub in MergeConfigTest() call
79a6d01 WIP: Disable TestMergeConfig(), begin stubbing out standard function to temporarily replace it
4a86a17 WIP: Attempt to stub in env var config test content
cadd5d1 README: Add coverage for conf file env var, flag
8930ac7 Update TODO items for remaining TestMergeConfig work
b52cd27 Add NOTICE.txt entry for stackoverflow example code
f021ad3 WIP: Add CompareConfig, testStringSliceEqual funcs, use them
b8bddc5 WIP: Prepping to build fileConfig to match scratch notes
b8e0017 Fix doc comment to indicate correct field and note why it needs to be initialized
1bb393f Finish first draft of merge test planning
fa4253c Specify FileExtensions, note why we will do so. Update Test function name
1c5eeee Add more scratch notes likely to be folded into the main README later
c4f5e8a Add scratch notes to be pruned/merged later
2ff1f17 Add TODO items for follow-up
c589b2f Add MergeConfig() test, update existing tests
a88c95c Add TODO notes for later work
407aef1 Clarify doc comment
19407b6 Update tests to use local copy of test values
be7332a Move default in-memory config to global test var
7fba570 Fix config file load/validation tests
e56453d Fix tests by switching logic on whether file exists
8a37ac5 Add tests for loading config file
0013b81 Add config/deprecated.go that was meant to go in a few commits back
6af8d29 Update config.LoadConfigFile to use io.Reader instead of filename
565586f Add FIXME for breaking dependency on config package
d10ac50 Finish moving code to deprecated.go
3cd84a5 Formatting tweaks for potential nil err
d71f5a5 Update Makefile to run go tests recursively, verbosely
23a6b94 Doc comment tweaks
f079327 Add explicit note that we're proceeding with evaluation of argsConfig after fileConfig validation failure
08d2dc2 Allow fileConfig post-merge validation failure
f8d1b30 Nevermind, going to try just making fileConfig validation a non-fatal status
5cc575a Fix tests to match last code change
4a424f0 Testing lighter Validate() enforcement
531bcad Fix goconst linting error
3a2ae1d WIP: Add TODO re calling Validate() against fileConfig post-merge
e198739 Move Validate() tests to dedicated file
dd774fb WIP: Finished Validate() tests?
ce8d608 WIP: Crafting Validate() tests
bb22d5e WIP: Crafting Validate() tests, fix Validate()
b0e40ef WIP: Crafting Validate() tests | KeepOldest, Remove, IgnoreErrors
4d6b4bc WIP: Crafting Validate() tests | FileAge
897e184 WIP: Crafting Validate() tests
172cc1c WIP: Crafting Validate() tests, fix Validate()
390a16f WIP: Crafting Validate() tests
2236140 WIP: Crafting Validate() tests
5c62376 Update config.Validate() to only return err/nil instead of bool + err/nil
358715f Add TODO item for later
8fb1bc1 Fix invalid assignment in MergeConfig()
d25b717 TESTS | Add coverage for LogBuffer.Flush()
b25f1d1 Prune old scratch doc comments
29fdc9d Update several logging methods to be test friendly
0e98dd2 TESTS | Add tests for SetLoggerConsoleOutput()
ae33e15 TESTING | Add tests for SetLoggerLevel
c5882da GitHub Actions Workflow: Run go tests
f297916 TESTS: Experiment with adding tests
5c939f6 Add stubbed out print statements proving os.Args does not contain quoting
88bf604 Log os.Args as first item in NewConfig()
b5ba232 Prune unused test code
df4c37d Split config.go into multiple sibling files
514083d Move package global logBuffer from config to logging package
c8ab59e Update doc comments, fail if requested config file not found
06c06be Miscellaneous cleanup, replace direct derefs
c91b1f5 Add missing exit if IgnoreErrors is false
3f0cd9b Fix prototype test file to accomodate NewConfig() changes
b40abf2 Update log level for merge failure messages, create help text writer func
3d9d2e2 WIP: Saving progress passing error handling to main()
e1aa4fe Prototyping panic behavior after post-merge config validation failure
597eee9 Tweak panic message for logger level default case
43ad767 SUCCESS! Found unintended nil assignment due to unhandled code path
9952ccc WIP: Still broken, saving status
71db887 WIP: Add untested handling of failed config validation
132d0c0 TODO: Fix config validation error handling
544469a WIP: Further troubleshooting
67b4435 WIP: Created SetDefaultConfig() method, minor tweaks, update String() method to use Getter methods
d20382e Saving progress
3c128c4 Move default values to Getter methods, use infamous new() func to initialize field pointers
1d5e35a Create defaultXYZ config struct fields; experimenting
0049cf8 Move AppMetadata values with other defaults (missed test file)
025a4a5 Move AppMetadata values with other defaults
94bdd1b Restore NumFilesToKeep default to match v0.5.1 change
d212e8a Add TODO re default values
9e26da2 Add back AppVersion, extend Validate() method
92fda99 Use GetUseSyslog() getter to restore flag value logging
3afd3ae Use non-pointer string slice fields for Paths, FileExtensions
7a364a4 Direct nil check on c.Paths instead of using Getter
8e34159 Fix several nil pointer derefs
46e85c2 WIP: Untested adjustments guided by Twitter feedback
189d628 Fix method call ref
6d18365 WIP: More fixes related to Getter methods
b70a385 WIP: More fixes to make use of Getter methods
0e75133 WIP: Set fileConfig and argsConfig to uninit structs
f8405e5 WIP: Finish Getter methods, much left to do
1a03244 Ignore local 'scratch' directory
e8fff5c WIP: Still broken, still rerranging the pieces
2c6227b WIP: Working on Getter methods
291bfe9 WIP: Need to build Getter methods next, replace direct access
68 changes: 68 additions & 0 deletions COMMITS_PR_Message.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
Fix config struct processing, add (much needed) tests

Added:

- Tests (partial implementation/coverage)
- validate the most common code-paths (much more TODO)
- config handling
- logging setup
- run tests as part of GitHub Actions Workflow
- update Makefile to run tests recursively, verbosely
- Fix configuration precedence handling
- Add string slice equality check from StackOverflow


Changed:

- Fix configuration precedence handling
- Config file loses to everything except default config settings
- `config.logBuffer` moved to `logging.LogBuffer`
- internal detail exposed for general use
- may change this later
- Updated code (where apparent) to be more test friendly
- e.g., use `io.Reader` instead of filename so that we can use
an in-memory TOML config file for testing config parsing
and precedence validation
- Split config package into smaller files in an effort to make related
code easier to manage
- Fail if requested config file not found
- previously an error was logged, but execution was allowed
to continue.
- Move default values to Getter methods
- use those methods instead of directly dereferencing config
struct fields in the majority of the code
- use Getter methods to guard against nil dereferences


Deprecated:

Both of these functions from the `config` package do not appear
to be needed any longer, but are being kept for a cycle in case
I change my mind:

- `Config.SetDefaultConfig()`
- `Config.GetStructTag()`


Fixed:

- Fix configuration precedence handling
- Config settings are merged properly, so that even default
settings are allowed to override lower precedence config sources
- NumFilesToKeep default (didn't match v0.5.1 change)
- Add missing `default` switch statements along with error
return codes for `Set` functions with specific valid option values
- README updates
- cover config file flag, environment variables
- config precedence corrections
- Multiple linting errors (with more that needs to be evaluated)
- Add missing exit if IgnoreErrors is false
- Handle unintended nil assignment
- Update GoDoc doc file
- reflect config file support, including updated Help text

TODO:

- Update multiple tests to use "tables" instead of hard-coding
specific checks (which ends up being very lengthy)
- e.g., `TestValidate()`

0 comments on commit e5fa0b6

Please sign in to comment.