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()`
- Various goconst linting errors

refs #161, #156, #170, #176
  • Loading branch information
@atc0005
atc0005 committed Dec 12, 2019
1 parent 599a84d commit ca8980d
Show file tree
Hide file tree
Showing 24 changed files with 3,501 additions and 540 deletions.
13 changes: 9 additions & 4 deletions .github/workflows/lint-and-build-code.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ jobs:
- name: Check out code into the Go module directory
uses: actions/checkout@v1

- name: Get dependencies
run: |
go get -v -t -d ./...
# Force tests to run early as it isn't worth doing much else if the
# tests fail to run properly.
- name: Run all tests
run: go test -v ./...

- name: Install Go linting tools
run: |
# add executables installed with go get to PATH
Expand All @@ -59,10 +68,6 @@ jobs:
export PATH=${PATH}:$(go env GOPATH)/bin
make lintinstall
- name: Get dependencies
run: |
go get -v -t -d ./...
- name: Install Ubuntu packages
if: contains(matrix.os, 'ubuntu')
run: sudo apt update && sudo apt install -y --no-install-recommends make gcc
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,8 @@ elbow.upx

# Ignore local copy of config file
config.toml


# Ignore local "scatch" directory that may
# contain temporary files that should not be included in the repo
scratch/
8 changes: 7 additions & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ LINTINSTALLCMD = bash testing/install_linting_tools.sh
# Targets will not work properly if a file with the same name is ever created
# in this directory. We explicitly declare our targets to be phony by
# making them a prerequisite of the special target .PHONY
.PHONY: help clean goclean gitclean pristine all windows linux testenv testrun linting lintinstall
.PHONY: help clean goclean gitclean pristine all windows linux testenv testrun linting lintinstall gotests

# WARNING: Make expects you to use tabs to introduce recipe lines
help:
Expand All @@ -67,6 +67,7 @@ help:
@echo " testrun use wrapper script to call binary with test settings"
@echo " lintinstall use wrapper script to install common linting tools"
@echo " linting use wrapper script to run common linting checks"
@echo " gotests go test recursively, verbosely"

testenv:
@echo "Setting up test environment in \"$(TESTENVDIR1)\" and \"$(TESTENVDIR2)\""
Expand All @@ -88,6 +89,11 @@ linting:
@$(LINTINGCMD)
@echo "Finished running linting checks"

gotests:
@echo "Running go tests ..."
@go test ./...
@echo "Finished running go tests"

goclean:
@echo "Removing object files and cached files ..."
@$(GOCLEANCMD)
Expand Down
40 changes: 40 additions & 0 deletions NOTICE.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,46 @@ SOFTWARE.




Stephen Weinberg
https://stackoverflow.com/a/15312097
https://stackoverflow.com/questions/15311969/checking-the-equality-of-two-slices

YOU ARE FREE TO:

Share - copy and redistribute the material in any medium or format
Adapt - remix, transform, and build upon the material for any purpose, even
commercially.

This license is acceptable for Free Cultural Works. The licensor cannot revoke
these freedoms as long as you follow the license terms.

UNDER THE FOLLOWING TERMS:

Attribution - You must give appropriate credit, provide a link to the license,
and indicate if changes were made. You may do so in any reasonable manner, but
not in any way that suggests the licensor endorses you or your use.

ShareAlike - If you remix, transform, or build upon the material, you must
distribute your contributions under the same license as the original.

No additional restrictions - You may not apply legal terms or technological
measures that legally restrict others from doing anything the license permits.

NOTICES:

You do not have to comply with the license for elements of the material in the
public domain or where your use is permitted by an applicable exception or
limitation.

No warranties are given. The license may not give you all of the permissions
necessary for your intended use. For example, other rights such as publicity,
privacy, or moral rights may limit how you use the material.





Stefan Nilsson
https://yourbasic.org/golang/formatting-byte-size-to-human-readable-format/
https://creativecommons.org/licenses/by/3.0/
Expand Down
84 changes: 46 additions & 38 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,51 +157,57 @@ against these newly created test files.
The priority order is (mostly):

1. Command line flags (highest priority)
1. Configuration file
1. Environment variables
1. Environment variables loaded from `.env` files (lowest priority)
- **Not supported yet**
1. Configuration file
1. Default settings

Configuration sources lower in the list are loaded first, with configuration
sources above loaded sequentially (if enabled) after. Any non-default values
specified for later sources (higher in the list) override default values
specified previously. This means that a non-default value specified in the
configuration file (which has to be intentionally loaded) can *survive* a
default value explicitly provided via a command-line option.
sources above loaded sequentially (if enabled) after. Settings are *merged*,
with settings specifically defined in sources with higher precedence overriding values set by configuration sources with lower precedence.

For example, if the configuration file defines `/tmp/elbow/path1` as the path
to process, an environment variable defines `/tmp/elbow/path2` and the
command-line flag for that setting specifies `/tmp/elbow/path3`, the
command-line flag will win and `/tmp/elbow/path3` will be used.

The intent of this behavior is to provide a *feathered* layering of
configuration settings; multiple configuration sources can be used to provide
overrides for default values, but not to override non-default values set
previously by another configuration source.
configuration settings; if a configuration file provides all settings that you
want other than one, you can use the configuration file for the other settings
and specify the settings that you wish to override via environment variable or
command-line flag.

**Note: This behavior is subject to change based on feedback.**

### Command-line Arguments

Aside from the built-in `-h`, short flag names are currently not supported.

| Long | Required | Default | Repeat | Possible | Description |
| ---------------- | -------- | -------------- | ------ | ------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------- |
| `keep` | No | `0` | No | `0+` | Keep specified number of matching files. |
| `paths` | Yes | N/A | No | *one or more valid directory paths* | List of comma or space-separated paths to process. |
| `pattern` | No | *empty string* | No | *valid file name characters* | Substring pattern to compare filenames against. Wildcards are not supported. |
| `extensions` | No | *empty list* | No | *valid file extensions* | Limit search to specified file extension. Specify as needed to match multiple required extensions. |
| `recurse` | No | `false` | No | `true`, `false` | Perform recursive search into subdirectories. |
| `keep-old` | No | `false` | No | `true`, `false` | Keep oldest files instead of newer. |
| `age` | No | `0` | No | `0+` | Limit search to files that are the specified number of days old or older. |
| `remove` | Maybe | `false` | No | `true`, `false` | Remove matched files. The default behavior is to only note what matching files *would* be removed. |
| `ignore-errors` | No | `false` | No | `true`, `false` | Ignore errors encountered during file removal. |
| `log-format` | No | `text` | No | `text`, `json` | Log formatter used by logging package. |
| `log-file` | No | *empty string* | No | *writable directory path* | Optional log file used to hold logged messages. If set, log messages are not displayed on the console. |
| `console-output` | No | `stdout` | No | `stdout`, `stderr` | Specify how log messages are logged to the console. |
| `log-level` | No | `info` | No | `emergency`, `alert`, `critical`, `panic`, `fatal`, `error`, `warn`, `info`, `notice`, `debug`, `trace` | Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. |
| `use-syslog` | No | `false` | No | `true`, `false` | Log messages to syslog in addition to other ouputs. Not supported on Windows. |
| Long | Required | Default | Repeat | Possible | Description |
| ---------------- | -------- | -------------- | ------ | ------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------- |
| `keep` | No | `0` | No | `0+` | Keep specified number of matching files. |
| `paths` | Yes | N/A | No | *one or more valid directory paths* | List of comma or space-separated paths to process. |
| `pattern` | No | *empty string* | No | *valid file name characters* | Substring pattern to compare filenames against. Wildcards are not supported. |
| `extensions` | No | *empty list* | No | *valid file extensions* | Limit search to specified file extension. Specify as needed to match multiple required extensions. |
| `recurse` | No | `false` | No | `true`, `false` | Perform recursive search into subdirectories. |
| `keep-old` | No | `false` | No | `true`, `false` | Keep oldest files instead of newer. |
| `age` | No | `0` | No | `0+` | Limit search to files that are the specified number of days old or older. |
| `remove` | Maybe | `false` | No | `true`, `false` | Remove matched files. The default behavior is to only note what matching files *would* be removed. |
| `ignore-errors` | No | `false` | No | `true`, `false` | Ignore errors encountered during file removal. |
| `log-format` | No | `text` | No | `text`, `json` | Log formatter used by logging package. |
| `log-file` | No | *empty string* | No | *writable directory path* | Optional log file used to hold logged messages. If set, log messages are not displayed on the console. |
| `console-output` | No | `stdout` | No | `stdout`, `stderr` | Specify how log messages are logged to the console. |
| `log-level` | No | `info` | No | `emergency`, `alert`, `critical`, `panic`, `fatal`, `error`, `warn`, `info`, `notice`, `debug`, `trace` | Maximum log level at which messages will be logged. Log messages below this threshold will be discarded. |
| `use-syslog` | No | `false` | No | `true`, `false` | Log messages to syslog in addition to other ouputs. Not supported on Windows. |
| `config-file` | No | *empty string* | No | *valid path to config file* | Full path to optional TOML-formatted configuration file. See `config.example.toml` for a starter template. |

### Environment Variables

If set, command-line arguments override the equivalent environment variables
listed below. See the [Command-line Arguments](#command-line-arguments) table
for more information.
If set, environment variables override settings provided by a configuration
file. If used, command-line arguments override the equivalent environment
variables listed below. See the [Command-line
Arguments](#command-line-arguments) table for more information.

| Flag Name | Environment Variable Name | Notes | Example |
| ---------------- | ------------------------- | ---------------------------- | ----------------------------------------------------------------------------------- |
Expand All @@ -219,13 +225,15 @@ for more information.
| `console-output` | `ELBOW_CONSOLE_OUTPUT` | | `ELBOW_CONSOLE_OUTPUT="stdout"` |
| `log-level` | `ELBOW_LOG_LEVEL` | | `ELBOW_LOG_LEVEL="debug"` |
| `use-syslog` | `ELBOW_USE_SYSLOG` | | `ELBOW_USE_SYSLOG="true"` |
| `config-file` | `ELBOW_CONFIG_FILE` | | `ELBOW_CONFIG_FILE="/usr/local/elbow/config.toml"` |

### Configuration File

If set, configuration file settings override equivalent environment variables,
but "lose" to command-line flags. See the [Command-line
Arguments](#command-line-arguments) table for more information, including the
available values for the listed configuration settings.
Configuration file settings have the lowest priority and are overridden by
settings specified in other configuration sources, except for default values.
See the [Command-line Arguments](#command-line-arguments) table for more
information, including the available values for the listed configuration
settings.

| Flag Name | Config file Setting Name | Section Name | Notes |
| ---------------- | ------------------------ | -------------- | ------------------------------------------------------------------------ |
Expand Down Expand Up @@ -257,14 +265,14 @@ within an Ubuntu Linux Subsystem for Windows (WSL) instance. The `t` volume is
present on the Windows host.

The file extension used in the examples below is for `WAR` files that are
generated on a build system that our group maintains. The idea is that `elbow`
could be run as a cron job to help ensure that only X copies (the most recent
in our case) for each of three branches remain on the build box.
generated on a build system that our group used to maintain. The idea is that
`elbow` could be run as a cron job to help ensure that only X copies (the most
recent in our case) for each of three branches remain on the build box.

There are better approaches to managing build artifacts (e.g., containers), but
that is the problem that this tool seeks to solve in a simple, "low tech" way.
There are better approaches to managing build artifacts (e.g., containers);
this tool seeks to solve in a simple, "low tech" way.

The particular repo that the build system processes has three branches:
The particular repo that the build system processed has three branches:

| Branch Name | Type of build |
| ----------- | ------------- |
Expand Down
Loading

0 comments on commit ca8980d

Please sign in to comment.