diff --git a/.golangci.yml b/.golangci.yml index 1ed22d01..979bdec2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -95,6 +95,7 @@ issues: - path: _test\.go linters: - bodyclose + - errcheck - funlen - gochecknoglobals - gocognit diff --git a/bind/describe_test.go b/bind/describe_test.go deleted file mode 100644 index 50f7e846..00000000 --- a/bind/describe_test.go +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright 2023 Sauce Labs Inc. All rights reserved. -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -package bind - -import ( - "testing" - - "github.com/spf13/pflag" -) - -func TestDescribeFlagsAsPlain(t *testing.T) { - tests := map[string]struct { - input map[string]interface{} - expected string - isErr bool - isHidden bool - showHidden bool - }{ - "keys are sorted": { - input: map[string]interface{}{"foo": false, "bar": true}, - expected: "bar=true\nfoo=false\n", - isErr: false, - }, - "bool is correctly formatted": { - input: map[string]interface{}{"key": false}, - expected: "key=false\n", - isErr: false, - }, - "string is correctly formatted": { - input: map[string]interface{}{"key": "val"}, - expected: "key=val\n", - isErr: false, - }, - "help is not shown": { - input: map[string]interface{}{"key": false, "help": true}, - expected: "key=false\n", - isErr: false, - }, - "hidden is shown": { - input: map[string]interface{}{"key": false}, - expected: "key=false\n", - isErr: false, - isHidden: true, - showHidden: true, - }, - "hidden is not shown": { - input: map[string]interface{}{"key": false}, - expected: ``, - isErr: false, - isHidden: true, - showHidden: false, - }, - } - - for name, tc := range tests { - fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) - - for k, v := range tc.input { - switch val := v.(type) { - case bool: - fs.Bool(k, val, "") - case string: - fs.String(k, val, "") - } - - if tc.isHidden { - err := fs.MarkHidden(k) - if err != nil { - t.Errorf("%s: test setup failed: %s", name, err) - } - } - } - result, err := DescribeFlags(fs, tc.showHidden, Plain) - - if (err != nil) != tc.isErr { - t.Errorf("%s: expected error: %v, got %s", name, tc.isErr, err) - } - - if result != tc.expected { - t.Errorf("%s: expected %s, got %s", name, tc.expected, result) - } - } -} - -func TestDescribeFlagsAsJSON(t *testing.T) { - tests := map[string]struct { - input map[string]interface{} - expected string - isErr bool - isHidden bool - showHidden bool - }{ - "bool is not quoted": { - input: map[string]interface{}{"key": false}, - expected: `{"key":false}`, - isErr: false, - }, - "help is not shown": { - input: map[string]interface{}{"key": false, "help": true}, - expected: `{"key":false}`, - isErr: false, - }, - "hidden is shown": { - input: map[string]interface{}{"key": false}, - expected: `{"key":false}`, - isErr: false, - isHidden: true, - showHidden: true, - }, - "hidden is not shown": { - input: map[string]interface{}{"key": false}, - expected: `{}`, - isErr: false, - isHidden: true, - showHidden: false, - }, - "string is quoted": { - input: map[string]interface{}{"key": "val"}, - expected: `{"key":"val"}`, - isErr: false, - }, - } - - for name, tc := range tests { - fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) - - for k, v := range tc.input { - switch val := v.(type) { - case bool: - fs.Bool(k, val, "") - case string: - fs.String(k, val, "") - } - - if tc.isHidden { - err := fs.MarkHidden(k) - if err != nil { - t.Errorf("%s: test setup failed: %s", name, err) - } - } - } - result, err := DescribeFlags(fs, tc.showHidden, JSON) - - if (err != nil) != tc.isErr { - t.Errorf("%s: expected error: %v, got %s", name, tc.isErr, err) - } - - if result != tc.expected { - t.Errorf("%s: expected %s, got %s", name, tc.expected, result) - } - } -} diff --git a/cmd/forwarder/httpbin/httpbin.go b/cmd/forwarder/httpbin/httpbin.go index 938ea686..6ce41a9e 100644 --- a/cmd/forwarder/httpbin/httpbin.go +++ b/cmd/forwarder/httpbin/httpbin.go @@ -13,6 +13,7 @@ import ( "github.com/saucelabs/forwarder/log" "github.com/saucelabs/forwarder/log/stdlog" "github.com/saucelabs/forwarder/runctx" + "github.com/saucelabs/forwarder/utils/cobrautil" "github.com/saucelabs/forwarder/utils/httpbin" "github.com/spf13/cobra" ) @@ -24,7 +25,7 @@ type command struct { } func (c *command) runE(cmd *cobra.Command, _ []string) error { - config, err := bind.DescribeFlags(cmd.Flags(), false, bind.Plain) + config, err := cobrautil.DescribeFlags(cmd.Flags(), false, cobrautil.Plain) if err != nil { return err } diff --git a/cmd/forwarder/pac/server/server.go b/cmd/forwarder/pac/server/server.go index 6ba5465c..beba27cc 100644 --- a/cmd/forwarder/pac/server/server.go +++ b/cmd/forwarder/pac/server/server.go @@ -18,6 +18,7 @@ import ( "github.com/saucelabs/forwarder/log/stdlog" "github.com/saucelabs/forwarder/pac" "github.com/saucelabs/forwarder/runctx" + "github.com/saucelabs/forwarder/utils/cobrautil" "github.com/saucelabs/forwarder/utils/osdns" "github.com/spf13/cobra" ) @@ -31,7 +32,7 @@ type command struct { } func (c *command) runE(cmd *cobra.Command, _ []string) error { - config, err := bind.DescribeFlags(cmd.Flags(), false, bind.Plain) + config, err := cobrautil.DescribeFlags(cmd.Flags(), false, cobrautil.Plain) if err != nil { return err } diff --git a/cmd/forwarder/run/run.go b/cmd/forwarder/run/run.go index 4f1b3ed7..d51e2c8c 100644 --- a/cmd/forwarder/run/run.go +++ b/cmd/forwarder/run/run.go @@ -21,6 +21,7 @@ import ( "github.com/saucelabs/forwarder/log/stdlog" "github.com/saucelabs/forwarder/pac" "github.com/saucelabs/forwarder/runctx" + "github.com/saucelabs/forwarder/utils/cobrautil" "github.com/saucelabs/forwarder/utils/osdns" "github.com/spf13/cobra" "go.uber.org/goleak" @@ -43,7 +44,7 @@ type command struct { } func (c *command) runE(cmd *cobra.Command, _ []string) error { - config, err := bind.DescribeFlags(cmd.Flags(), false, bind.Plain) + config, err := cobrautil.DescribeFlags(cmd.Flags(), false, cobrautil.Plain) if err != nil { return err } diff --git a/dialvia/http_test.go b/dialvia/http_test.go index 268e1686..3ea7e9ec 100644 --- a/dialvia/http_test.go +++ b/dialvia/http_test.go @@ -114,7 +114,7 @@ func TestHTTPProxyDialerDialContext(t *testing.T) { done := make(chan struct{}) go func() { - serveOne(l, func(conn net.Conn) error { //nolint:errcheck // this function always returns nil error + serveOne(l, func(conn net.Conn) error { cancel() <-done return nil diff --git a/bind/describe.go b/utils/cobrautil/describe.go similarity index 91% rename from bind/describe.go rename to utils/cobrautil/describe.go index 71bf1751..853dea65 100644 --- a/bind/describe.go +++ b/utils/cobrautil/describe.go @@ -4,7 +4,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -package bind +package cobrautil import ( "encoding/json" @@ -13,6 +13,7 @@ import ( "strings" "github.com/spf13/pflag" + "golang.org/x/exp/maps" ) type DescribeFormat int @@ -24,13 +25,11 @@ const ( func DescribeFlags(fs *pflag.FlagSet, showHidden bool, format DescribeFormat) (string, error) { args := make(map[string]any, fs.NFlag()) - keys := make([]string, 0, fs.NFlag()) fs.VisitAll(func(flag *pflag.Flag) { if flag.Name == "help" { return } - if flag.Hidden && !showHidden { return } @@ -40,14 +39,12 @@ func DescribeFlags(fs *pflag.FlagSet, showHidden bool, format DescribeFormat) (s } else { args[flag.Name] = strings.Trim(flag.Value.String(), "[]") } - - keys = append(keys, flag.Name) }) - sort.Strings(keys) - switch format { case Plain: + keys := maps.Keys(args) + sort.Strings(keys) var b strings.Builder for _, name := range keys { b.WriteString(fmt.Sprintf("%s=%s\n", name, args[name])) diff --git a/utils/cobrautil/describe_test.go b/utils/cobrautil/describe_test.go new file mode 100644 index 00000000..51825c48 --- /dev/null +++ b/utils/cobrautil/describe_test.go @@ -0,0 +1,211 @@ +// Copyright 2023 Sauce Labs Inc. All rights reserved. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +package cobrautil + +import ( + "testing" + + "github.com/spf13/pflag" +) + +func TestDescribeFlagsAsPlain(t *testing.T) { + tests := []struct { + name string + flags func() *pflag.FlagSet + showHidden bool + expected string + }{ + { + name: "keys are sorted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("c", true, "") + fs.Bool("d", false, "") + fs.Bool("a", true, "") + fs.Bool("b", false, "") + return fs + }, + showHidden: false, + expected: "a=true\nb=false\nc=true\nd=false\n", + }, + { + name: "bool is correctly formatted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + return fs + }, + showHidden: false, + expected: "key=false\n", + }, + { + name: "string is correctly formatted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.String("key", "val", "") + return fs + }, + showHidden: false, + expected: "key=val\n", + }, + { + name: "help is not shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + fs.Bool("help", true, "") + return fs + }, + showHidden: false, + expected: "key=false\n", + }, + { + name: "hidden is shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + _ = fs.MarkHidden("key") + return fs + }, + showHidden: true, + expected: "key=false\n", + }, + { + name: "hidden is not shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + _ = fs.MarkHidden("key") + return fs + }, + showHidden: false, + expected: ``, + }, + { + name: "list of values", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.StringSlice("list", []string{"item1", "item2"}, "") + return fs + }, + showHidden: false, + expected: "list=item1,item2\n", + }, + } + + for i := range tests { + tc := tests[i] + t.Run(tc.name, func(t *testing.T) { + result, err := DescribeFlags(tc.flags(), tc.showHidden, Plain) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if result != tc.expected { + t.Errorf("expected %s, got %s", tc.expected, result) + } + }) + } +} + +func TestDescribeFlagsAsJSON(t *testing.T) { + tests := []struct { + name string + flags func() *pflag.FlagSet + showHidden bool + expected string + }{ + { + name: "bool is not quoted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + return fs + }, + showHidden: false, + expected: `{"key":false}`, + }, + { + name: "help is not shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + fs.Bool("help", true, "") + return fs + }, + showHidden: false, + expected: `{"key":false}`, + }, + { + name: "hidden is shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + _ = fs.MarkHidden("key") + return fs + }, + showHidden: true, + expected: `{"key":false}`, + }, + { + name: "hidden is not shown", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("key", false, "") + _ = fs.MarkHidden("key") + return fs + }, + showHidden: false, + expected: `{}`, + }, + { + name: "string is quoted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.String("key", "val", "") + return fs + }, + showHidden: false, + expected: `{"key":"val"}`, + }, + { + name: "keys are sorted", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.Bool("c", false, "") + fs.String("b", "val", "") + fs.Bool("a", false, "") + fs.String("d", "val", "") + return fs + }, + showHidden: false, + expected: `{"a":false,"b":"val","c":false,"d":"val"}`, + }, + { + name: "list of values", + flags: func() *pflag.FlagSet { + fs := pflag.NewFlagSet("flags", pflag.ContinueOnError) + fs.StringSlice("list", []string{"item1", "item2"}, "") + return fs + }, + showHidden: false, + expected: `{"list":"item1,item2"}`, + }, + } + + for i := range tests { + tc := tests[i] + t.Run(tc.name, func(t *testing.T) { + result, err := DescribeFlags(tc.flags(), tc.showHidden, JSON) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if result != tc.expected { + t.Errorf("expected %s, got %s", tc.expected, result) + } + }) + } +}