Skip to content

Commit 9a92e95

Browse files
committed
test: cover the gate lookup and the validate error path
codecov/patch reported 69.56% of the diff hit against a 80% target. The gaps were the branches a caller reaches when there is nothing to consult: gateIsOpen with no enclosing struct, and its skip over fields that carry the right key with the wrong type or the right type with the wrong key — either would silently turn a required field optional if it were mistaken for the gate. Also adds a unit test for validate rejecting a semantically invalid config without the strict flag, which until now was only asserted through the e2e suite. gateIsOpen goes to 100%, ValidateCommand.Execute from 81.8% to 90.9%. The two blocks still uncovered there predate this change: the log-level warning branch and the json.Marshal error path. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
1 parent ec9380b commit 9a92e95

2 files changed

Lines changed: 86 additions & 0 deletions

File tree

cli/validate_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,40 @@ func TestValidateExecuteMissingFile(t *testing.T) {
6767
err := cmd.Execute(nil)
6868
assert.Error(t, err)
6969
}
70+
71+
// TestValidateExecuteRunsValidatorWithoutStrictFlag pins that running validate
72+
// is itself the request to have the config checked. The checks used to sit
73+
// behind enable-strict-validation, which defaults to false, so the one command
74+
// whose purpose is validation reported success on a config it had not
75+
// inspected.
76+
//
77+
// The config below parses cleanly as INI and is only wrong semantically, so it
78+
// exercises the validator rather than the loader.
79+
func TestValidateExecuteRunsValidatorWithoutStrictFlag(t *testing.T) {
80+
// Not parallel: modifies global os.Stdout which races with other tests.
81+
82+
configFile := filepath.Join(t.TempDir(), "config.ini")
83+
content := `
84+
[global]
85+
web-address = definitely-not-an-address
86+
87+
[job-exec "foo"]
88+
schedule = @every 10s
89+
command = echo "foo"
90+
`
91+
require.NoError(t, os.WriteFile(configFile, []byte(content), 0o644))
92+
93+
r, w, _ := os.Pipe()
94+
oldStdout := os.Stdout
95+
os.Stdout = w
96+
defer func() { os.Stdout = oldStdout }()
97+
98+
cmd := ValidateCommand{ConfigFile: configFile, Logger: test.NewTestLogger()}
99+
err := cmd.Execute(nil)
100+
101+
w.Close()
102+
_, _ = io.ReadAll(r)
103+
104+
require.Error(t, err, "an invalid web-address was accepted without the strict flag")
105+
assert.Contains(t, err.Error(), "web-address")
106+
}

config/validator_conditional_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package config
55

66
import (
7+
"reflect"
78
"strings"
89
"testing"
910
)
@@ -77,3 +78,51 @@ func TestConditionalRequired_UnknownGateStaysRequired(t *testing.T) {
7778
t.Error("with no gate to consult the field should stay required, got no error")
7879
}
7980
}
81+
82+
// TestGateIsOpen_InvalidParentKeepsFieldRequired covers the guard for a caller
83+
// that has no enclosing struct to offer — the internal helpers are called that
84+
// way in tests. With nothing to consult, the field stays required, which is
85+
// the same safe direction as an unresolvable gate.
86+
func TestGateIsOpen_InvalidParentKeepsFieldRequired(t *testing.T) {
87+
t.Parallel()
88+
89+
cv := NewConfigValidator(nil)
90+
if !cv.gateIsOpen(reflect.Value{}, "web-secret-key") {
91+
t.Error("with no parent to inspect the field should stay required")
92+
}
93+
}
94+
95+
// TestGateIsOpen_IgnoresMismatchedFields covers the skip inside the search: a
96+
// field is only the gate if it is a bool AND carries the expected key. A
97+
// string field named like the gate, or a bool with a different key, must not
98+
// be mistaken for it.
99+
func TestGateIsOpen_IgnoresMismatchedFields(t *testing.T) {
100+
t.Parallel()
101+
102+
type decoys struct {
103+
// Right key, wrong type.
104+
WebAuthEnabled string `gcfg:"web-auth-enabled"`
105+
// Right type, wrong key.
106+
SomethingElse bool `gcfg:"some-other-flag"`
107+
}
108+
109+
cv := NewConfigValidator(nil)
110+
parent := reflect.ValueOf(decoys{WebAuthEnabled: "true", SomethingElse: true})
111+
112+
// Neither decoy qualifies, so the search finds no gate and the field stays
113+
// required rather than being switched on by the wrong field.
114+
if !cv.gateIsOpen(parent, "web-secret-key") {
115+
t.Error("a string field and an unrelated bool were treated as the gate")
116+
}
117+
}
118+
119+
// TestGateIsOpen_UnconditionalFieldsAlwaysRequired pins the common case: a
120+
// field with no entry in requiredWhen is not gated at all.
121+
func TestGateIsOpen_UnconditionalFieldsAlwaysRequired(t *testing.T) {
122+
t.Parallel()
123+
124+
cv := NewConfigValidator(nil)
125+
if !cv.gateIsOpen(reflect.ValueOf(gatedConfig{}), "web-address") {
126+
t.Error("an ungated field reported as not required")
127+
}
128+
}

0 commit comments

Comments
 (0)