Skip to content

Commit

Permalink
chore: pkg: flagcodec: more test coverage and docs
Browse files Browse the repository at this point in the history 
Add more docs and test coverage; most notably, clarify
it is safe and supported to just call Delete() against
nonexistent options. Much like `delete` on maps,
it will turn as no-operation silently and succesfully.

Signed-off-by: Francesco Romani <[email protected]>
  • Loading branch information
@ffromani
ffromani committed Nov 27, 2023
1 parent f60b493 commit 62d1406
Show file tree
Hide file tree
Showing 2 changed files with 207 additions and 2 deletions.
14 changes: 12 additions & 2 deletions pkg/flagcodec/flagcodec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,28 @@ type Flags struct {
keys []string
}

// ParseArgvKeyValue parses a clean (trimmed) argv whose components
// are either toggles or key=value pairs. IOW, this is a restricted and easier
// to parse flavor of argv on which option and value are guaranteed to
// be in the same item.
// IOW, we expect
// "--opt=foo"
// AND NOT
// "--opt", "foo"
// The value of argv[0], whatever it is, is taken at command.
func ParseArgvKeyValue(args []string) *Flags {
return ParseArgvKeyValueWithCommand("", args)
}

// ParseArgvKeyValue parses a clean (trimmed) argv whose components are
// either toggles or key=value pairs. IOW, this is a restricted and easier
// ParseArgvKeyValueWithCommand parses a clean (trimmed) argv whose components
// are either toggles or key=value pairs. IOW, this is a restricted and easier
// to parse flavor of argv on which option and value are guaranteed to
// be in the same item.
// IOW, we expect
// "--opt=foo"
// AND NOT
// "--opt", "foo"
// The command is supplied explicitely as parameter.
func ParseArgvKeyValueWithCommand(command string, args []string) *Flags {
ret := &Flags{
command: command,
Expand Down
195 changes: 195 additions & 0 deletions pkg/flagcodec/flagcodec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,60 @@ import (
)

func TestParseStringRoundTrip(t *testing.T) {
type testCase struct {
name string
argv []string
expected []string
}

testCases := []testCase{
{
name: "nil",
},
{
name: "empty",
argv: []string{},
},
{
name: "only command",
argv: []string{
"/bin/true",
},
expected: []string{
"/bin/true",
},
},
{
name: "simple",
argv: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
expected: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fl := ParseArgvKeyValue(tc.argv)
got := fl.Argv()
if !reflect.DeepEqual(tc.expected, got) {
t.Errorf("expected %v got %v", tc.expected, got)
}
})
}
}

func TestParseStringRoundTripWithCommand(t *testing.T) {
type testCase struct {
name string
command string
Expand Down Expand Up @@ -164,6 +218,26 @@ func TestDeleteFlags(t *testing.T) {
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
},
{
name: "remove-option-inexistent",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
options: []string{
"--foo=bar",
},
expected: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
},
{
name: "remove-toggle",
command: "/bin/resource-topology-exporter",
Expand All @@ -185,6 +259,28 @@ func TestDeleteFlags(t *testing.T) {
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
},
{
name: "remove-toggle-inexistent",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--pods-fingerprint",
},
options: []string{
"--fizz-buzz",
},
expected: []string{
"/bin/resource-topology-exporter",
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--pods-fingerprint",
},
},
}

for _, tc := range testCases {
Expand All @@ -200,3 +296,102 @@ func TestDeleteFlags(t *testing.T) {
})
}
}

func TestGetFlags(t *testing.T) {
type testOpt struct {
value Val
found bool
}

type testCase struct {
name string
command string
args []string
params []string
expected []testOpt
}

testCases := []testCase{
{
name: "get-option-missing",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
params: []string{
"--blah",
},
expected: []testOpt{
{
value: Val{},
found: false,
},
},
},
{
name: "get-flag-option-existing",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
},
params: []string{
"--sysfs",
},
expected: []testOpt{
{
value: Val{
Kind: FlagOption,
Data: "/host-sys",
},
found: true,
},
},
},
{
name: "get-toggle-option-existing",
command: "/bin/resource-topology-exporter",
args: []string{
"--sleep-interval=10s",
"--sysfs=/host-sys",
"--kubelet-state-dir=/host-var/lib/kubelet",
"--podresources-socket=unix:///host-var/lib/kubelet/pod-resources/kubelet.sock",
"--pods-fingerprint",
},
params: []string{
"--pods-fingerprint",
},
expected: []testOpt{
{
value: Val{
Kind: FlagToggle,
Data: "",
},
found: true,
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
fl := ParseArgvKeyValueWithCommand(tc.command, tc.args)
for idx := range tc.params {
param := tc.params[idx]
exp := tc.expected[idx]
got, ok := fl.GetFlag(param)
if ok != exp.found {
t.Fatalf("flag %q found %v expected %v", param, ok, exp.found)
}
if !reflect.DeepEqual(got, exp.value) {
t.Errorf(" flag %q got %+v expected %+v", param, got, exp.value)
}
}
})
}
}

0 comments on commit 62d1406

Please sign in to comment.