Skip to content
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

Fix config struct processing, add (much needed) tests #174

Merged
merged 1 commit into from
Dec 12, 2019

Conversation

atc0005
Copy link
Owner

@atc0005 atc0005 commented Dec 10, 2019

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 Consider breaking dependency between config package and other subpackages #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()

References

@atc0005 atc0005 added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request config file command-line environment variables config license Related to the license for this project (e.g., LICENSE, NOTICES.txt or README) CI tests labels Dec 10, 2019
@atc0005 atc0005 added this to the v0.7.0 milestone Dec 10, 2019
@atc0005 atc0005 force-pushed the i161-fix-config-struct-processing branch from 6e00bea to 0828f62 Compare December 11, 2019 12:39
@atc0005 atc0005 self-assigned this Dec 11, 2019
@atc0005 atc0005 force-pushed the i161-fix-config-struct-processing branch 6 times, most recently from 2d1e874 to aac431a Compare December 12, 2019 12:20
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()`
- Various goconst linting errors

refs #161, #156, #170, #176
@atc0005 atc0005 force-pushed the i161-fix-config-struct-processing branch from aac431a to 00e0320 Compare December 12, 2019 12:32
@atc0005 atc0005 merged commit 1c24a42 into master Dec 12, 2019
@atc0005 atc0005 deleted the i161-fix-config-struct-processing branch December 12, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI command-line config file config documentation Improvements or additions to documentation enhancement New feature or request environment variables license Related to the license for this project (e.g., LICENSE, NOTICES.txt or README) tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make explicit configuration choices override those set earlier in load order
1 participant