goss-org / goss

Quick and Easy server testing/validation
https://goss.rocks
Apache License 2.0
5.5k stars 470 forks source link

Added: warning when duplicate keys cause tests to be overwritten #843

Closed aelsabbahy closed 9 months ago

aelsabbahy commented 9 months ago
Checklist

Description of change

Prints out warning messages when overlapping keys are detected, for example:

goss.yaml:

command:
  example_test:
    exec: echo "hi"
    exit-status: 0
    stdout:
    - hi
    stderr: ""
    timeout: 10000
gossfile:
  goss_embed.yaml: {}

goss_embed.yaml:

command:
  example_test:
    exec: echo "bye"
    exit-status: 0
    stdout:
    - bye
    timeout: 10000
$ goss -l warn v
2023-09-16T20:36:48Z [WARN] Duplicate key detected: 'command: example_test'. The value from a later-loaded goss file has overwritten the previous value.
..

Total Duration: 0.002s
Count: 2, Failed: 0, Skipped: 0

Same with render:

$ goss -l warn render > /dev/null
2023/09/16 13:37:07 [WARN] Duplicate key detected: 'command: example_test'. The value from a later-loaded goss file has overwritten the previous value.

This warning is printed for both failing and passing tests, essentially it is informing the user that the tests were overwritten.. which may or may not be desired.

As a consequence of this change, I moved some existing log levels around to match what I would expect as a user:

closes #743

Questions

  1. Given the above classification, should the default log level be set to INFO? Users who find it noisy can do -l warn or -l error
aelsabbahy commented 9 months ago

CC: @obourdon, @ripienaar,@plannigan, @GhuulCZ

Since you all have been involved with discussions around this.

plannigan commented 9 months ago

Seems reasonable to me. :+1:

My preference would have been to actually error, this is much better than silently overwriting.

aelsabbahy commented 9 months ago

@plannigan how about a layered approach

  1. Default, user gets a warning if they're not aware of this behavior. (This pr)
  2. User can pass --log-level ERROR to silence warnings.. perhaps they're intentionally using overrides. (This pr)
  3. User wants to explicitly block this behavior, they pass --strict (new pr)
plannigan commented 9 months ago

Yeah, that makes sense. Default to showing the warning. People can silence it if they want. People can also elevate it to an error if they want.