From 4712f746cc7390b789b6fa9a5537a46c05f4b88d Mon Sep 17 00:00:00 2001 From: Rob Percival Date: Fri, 19 May 2017 17:33:19 +0100 Subject: [PATCH 01/10] Example of loading flags from a file --- cmd/createtree/main.go | 18 ++++++++++++++++++ cmd/flags.go | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 cmd/flags.go diff --git a/cmd/createtree/main.go b/cmd/createtree/main.go index ce72149683..9cc0a0bac3 100644 --- a/cmd/createtree/main.go +++ b/cmd/createtree/main.go @@ -41,12 +41,14 @@ import ( "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/any" "github.com/google/trillian" + "github.com/google/trillian/cmd" "github.com/google/trillian/crypto/keyspb" "github.com/google/trillian/crypto/sigpb" "google.golang.org/grpc" ) var ( + configFile = flag.String("config", "", "Config file containing flags") adminServerAddr = flag.String("admin_server", "", "Address of the gRPC Trillian Admin Server (host:port)") treeState = flag.String("tree_state", trillian.TreeState_ACTIVE.String(), "State of the new tree") @@ -175,6 +177,22 @@ func newOptsFromFlags() *createOpts { func main() { flag.Parse() + if *configFile != "" { + if flag.NFlag() != 1 { + fmt.Printf("No other flags can be provided when --config is set") + os.Exit(1) + } + + if err := cmd.ParseFlagFile(*configFile); err != nil { + fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) + os.Exit(1) + } + + // Alternative to printing error if more than just "--config" flag is provided: + // let command-line flags take precedent by re-parsing from the command-line. + // flag.Parse() + } + ctx := context.Background() tree, err := createTree(ctx, newOptsFromFlags()) if err != nil { diff --git a/cmd/flags.go b/cmd/flags.go new file mode 100644 index 0000000000..25232ae211 --- /dev/null +++ b/cmd/flags.go @@ -0,0 +1,24 @@ +package cmd + +import ( + "flag" + "io/ioutil" + + "github.com/mattn/go-shellwords" +) + +func ParseFlagFile(path string) error { + file, err := ioutil.ReadFile(path) + if err != nil { + return err + } + + p := shellwords.NewParser() + p.ParseEnv = true + args, err := p.Parse(string(file)) + if err != nil { + return err + } + + return flag.CommandLine.Parse(args) +} From ddcc786ee68d97d0dfa555fa8bf4d666a747d491 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 22 May 2017 17:55:49 -0700 Subject: [PATCH 02/10] Expand config file functionality to log_server, log_signer, and map_server --- cmd/createtree/main.go | 12 ++---------- cmd/flags.go | 12 +++++++++--- server/trillian_log_server/main.go | 13 +++++++++++++ server/trillian_log_signer/main.go | 11 +++++++++++ server/vmap/trillian_map_server/main.go | 12 ++++++++++++ 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/cmd/createtree/main.go b/cmd/createtree/main.go index 9cc0a0bac3..f35c1673ae 100644 --- a/cmd/createtree/main.go +++ b/cmd/createtree/main.go @@ -48,7 +48,6 @@ import ( ) var ( - configFile = flag.String("config", "", "Config file containing flags") adminServerAddr = flag.String("admin_server", "", "Address of the gRPC Trillian Admin Server (host:port)") treeState = flag.String("tree_state", trillian.TreeState_ACTIVE.String(), "State of the new tree") @@ -62,6 +61,8 @@ var ( privateKeyFormat = flag.String("private_key_format", "PEMKeyFile", "Type of private key to be used") pemKeyPath = flag.String("pem_key_path", "", "Path to the private key PEM file") pemKeyPassword = flag.String("pem_key_password", "", "Password of the private key PEM file") + + configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") ) // createOpts contains all user-supplied options required to run the program. @@ -178,19 +179,10 @@ func main() { flag.Parse() if *configFile != "" { - if flag.NFlag() != 1 { - fmt.Printf("No other flags can be provided when --config is set") - os.Exit(1) - } - if err := cmd.ParseFlagFile(*configFile); err != nil { fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) os.Exit(1) } - - // Alternative to printing error if more than just "--config" flag is provided: - // let command-line flags take precedent by re-parsing from the command-line. - // flag.Parse() } ctx := context.Background() diff --git a/cmd/flags.go b/cmd/flags.go index 25232ae211..daf7575a95 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -3,8 +3,6 @@ package cmd import ( "flag" "io/ioutil" - - "github.com/mattn/go-shellwords" ) func ParseFlagFile(path string) error { @@ -20,5 +18,13 @@ func ParseFlagFile(path string) error { return err } - return flag.CommandLine.Parse(args) + err = flag.CommandLine.Parse(args) + if err != nil { + return err + } + + // Call flag.Parse() again so that command line flags + // can override flags provided in the provided flag file. + flag.Parse() + return nil } diff --git a/server/trillian_log_server/main.go b/server/trillian_log_server/main.go index 84b741f403..8ca07c4585 100644 --- a/server/trillian_log_server/main.go +++ b/server/trillian_log_server/main.go @@ -17,7 +17,9 @@ package main import ( "context" "flag" + "fmt" _ "net/http/pprof" + "os" "strings" "time" @@ -27,6 +29,7 @@ import ( etcdnaming "github.com/coreos/etcd/clientv3/naming" "github.com/golang/glog" "github.com/google/trillian" + "github.com/google/trillian/cmd" "github.com/google/trillian/crypto/keys" "github.com/google/trillian/extension" "github.com/google/trillian/monitoring" @@ -48,10 +51,20 @@ var ( etcdServers = flag.String("etcd_servers", "", "A comma-separated list of etcd servers; no etcd registration if empty") etcdService = flag.String("etcd_service", "trillian-log", "Service name to announce ourselves under") maxUnsequencedRows = flag.Int("max_unsequenced_rows", mysqlq.DefaultMaxUnsequenced, "Max number of unsequenced rows before rate limiting kicks in") + + configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") ) func main() { flag.Parse() + + if *configFile != "" { + if err := cmd.ParseFlagFile(*configFile); err != nil { + fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) + os.Exit(1) + } + } + ctx := context.Background() // First make sure we can access the database, quit if not diff --git a/server/trillian_log_signer/main.go b/server/trillian_log_signer/main.go index 93ce5d72f3..2a5e242b71 100644 --- a/server/trillian_log_signer/main.go +++ b/server/trillian_log_signer/main.go @@ -23,6 +23,7 @@ import ( _ "github.com/go-sql-driver/mysql" // Load MySQL driver "github.com/golang/glog" + "github.com/google/trillian/cmd" "github.com/google/trillian/crypto/keys" "github.com/google/trillian/extension" "github.com/google/trillian/monitoring/metric" @@ -50,10 +51,20 @@ var ( masterCheckInterval = flag.Duration("master_check_interval", 5*time.Second, "Interval between checking mastership still held") masterHoldInterval = flag.Duration("master_hold_interval", 60*time.Second, "Minimum interval to hold mastership for") resignOdds = flag.Int("resign_odds", 10, "Chance of resigning mastership after each check, the N in 1-in-N") + + configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") ) func main() { flag.Parse() + + if *configFile != "" { + if err := cmd.ParseFlagFile(*configFile); err != nil { + fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) + os.Exit(1) + } + } + glog.CopyStandardLogTo("WARNING") glog.Info("**** Log Signer Starting ****") diff --git a/server/vmap/trillian_map_server/main.go b/server/vmap/trillian_map_server/main.go index 4a08525164..39eb01e7f9 100644 --- a/server/vmap/trillian_map_server/main.go +++ b/server/vmap/trillian_map_server/main.go @@ -17,12 +17,15 @@ package main import ( "context" "flag" + "fmt" _ "net/http/pprof" + "os" _ "github.com/go-sql-driver/mysql" // Load MySQL driver "github.com/golang/glog" "github.com/google/trillian" + "github.com/google/trillian/cmd" "github.com/google/trillian/crypto/keys" "github.com/google/trillian/extension" mysqlq "github.com/google/trillian/quota/mysql" @@ -39,11 +42,20 @@ var ( rpcEndpoint = flag.String("rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") httpEndpoint = flag.String("http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") maxUnsequencedRows = flag.Int("max_unsequenced_rows", mysqlq.DefaultMaxUnsequenced, "Max number of unsequenced rows before rate limiting kicks in") + + configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") ) func main() { flag.Parse() + if *configFile != "" { + if err := cmd.ParseFlagFile(*configFile); err != nil { + fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) + os.Exit(1) + } + } + db, err := mysql.OpenDB(*mySQLURI) if err != nil { glog.Exitf("Failed to open database: %v", err) From 4c06b2e06b37b4d7f102659a7ebd5cabf01b2545 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 22 May 2017 18:00:52 -0700 Subject: [PATCH 03/10] Add comment to ParseFlagFile --- cmd/flags.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/flags.go b/cmd/flags.go index daf7575a95..9870ac3842 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -5,6 +5,10 @@ import ( "io/ioutil" ) +// ParseFlagFile parses a set of flags from a file at the provided +// path. Re-calls flag.Parse() after parsing the flags in the file +// so that flags provided on the command line take precedence over +// flags provided in the file. func ParseFlagFile(path string) error { file, err := ioutil.ReadFile(path) if err != nil { From a6656ac48e310dd82c27e337bef3f4c2692b396e Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 22 May 2017 18:07:08 -0700 Subject: [PATCH 04/10] Add cmd/flags.go license header --- cmd/flags.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/flags.go b/cmd/flags.go index 9870ac3842..8b4e5c09fc 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -1,3 +1,17 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package cmd import ( From cc9922c052f161d5eb413ccd70c1b1f1d7241835 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Mon, 22 May 2017 18:17:52 -0700 Subject: [PATCH 05/10] Re-add go-shellwords import --- cmd/flags.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/flags.go b/cmd/flags.go index 8b4e5c09fc..3aac5dc54b 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -17,6 +17,8 @@ package cmd import ( "flag" "io/ioutil" + + "github.com/mattn/go-shellwords" ) // ParseFlagFile parses a set of flags from a file at the provided From 4fd46e5971fa2e854d6ac7cb2a7f332a5dd8c435 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 23 May 2017 11:40:42 -0700 Subject: [PATCH 06/10] Switch from github.com/mattn/go-shellwords to bitbucket.org/creachadair/shell --- cmd/flags.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cmd/flags.go b/cmd/flags.go index 3aac5dc54b..687ef191dd 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -15,10 +15,11 @@ package cmd import ( + "errors" "flag" "io/ioutil" - "github.com/mattn/go-shellwords" + "bitbucket.org/creachadair/shell" ) // ParseFlagFile parses a set of flags from a file at the provided @@ -31,11 +32,9 @@ func ParseFlagFile(path string) error { return err } - p := shellwords.NewParser() - p.ParseEnv = true - args, err := p.Parse(string(file)) - if err != nil { - return err + args, valid := shell.Split(string(file)) + if !valid { + return errors.New("flag file contains unclosed quotations") } err = flag.CommandLine.Parse(args) From d2cc80d634f07bf065092cbbfc011e86c706936c Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 23 May 2017 11:48:52 -0700 Subject: [PATCH 07/10] Expand env vars in flag file if present --- cmd/flags.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/flags.go b/cmd/flags.go index 687ef191dd..b730ffb0fd 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -18,6 +18,7 @@ import ( "errors" "flag" "io/ioutil" + "os" "bitbucket.org/creachadair/shell" ) @@ -32,7 +33,10 @@ func ParseFlagFile(path string) error { return err } - args, valid := shell.Split(string(file)) + // Expand any environment variables in the file + flagsString := os.ExpandEnv(string(file)) + + args, valid := shell.Split(flagsString) if !valid { return errors.New("flag file contains unclosed quotations") } From 54850a141e147e829ed75fb9a9d3911ce9eea7a8 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 24 May 2017 14:38:55 -0700 Subject: [PATCH 08/10] Add flag file parsing tests and env cleanups --- cmd/flags.go | 35 +++++++++------- cmd/flags_test.go | 104 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 16 deletions(-) create mode 100644 cmd/flags_test.go diff --git a/cmd/flags.go b/cmd/flags.go index b730ffb0fd..cbef6e2ae9 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -23,26 +23,17 @@ import ( "bitbucket.org/creachadair/shell" ) -// ParseFlagFile parses a set of flags from a file at the provided -// path. Re-calls flag.Parse() after parsing the flags in the file -// so that flags provided on the command line take precedence over -// flags provided in the file. -func ParseFlagFile(path string) error { - file, err := ioutil.ReadFile(path) - if err != nil { - return err - } - - // Expand any environment variables in the file - flagsString := os.ExpandEnv(string(file)) - - args, valid := shell.Split(flagsString) +func parseFlags(file string) error { + args, valid := shell.Split(file) if !valid { return errors.New("flag file contains unclosed quotations") } + // Expand any environment variables in the args + for i := range args { + args[i] = os.ExpandEnv(args[i]) + } - err = flag.CommandLine.Parse(args) - if err != nil { + if err := flag.CommandLine.Parse(args); err != nil { return err } @@ -51,3 +42,15 @@ func ParseFlagFile(path string) error { flag.Parse() return nil } + +// ParseFlagFile parses a set of flags from a file at the provided +// path. Re-calls flag.Parse() after parsing the flags in the file +// so that flags provided on the command line take precedence over +// flags provided in the file. +func ParseFlagFile(path string) error { + file, err := ioutil.ReadFile(path) + if err != nil { + return err + } + return parseFlags(string(file)) +} diff --git a/cmd/flags_test.go b/cmd/flags_test.go new file mode 100644 index 0000000000..48cedbbf4c --- /dev/null +++ b/cmd/flags_test.go @@ -0,0 +1,104 @@ +// Copyright 2017 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cmd + +import ( + "flag" + "os" + "testing" +) + +func TestParseFlags(t *testing.T) { + var a, b string + flag.StringVar(&a, "a", "", "") + flag.StringVar(&b, "b", "", "") + + flag.CommandLine.Init(os.Args[0], flag.ContinueOnError) + + tests := []struct { + contents string + env map[string]string + cliArgs []string + expectedErr string + expectedA string + expectedB string + }{ + { + contents: "-a one -b two", + expectedA: "one", + expectedB: "two", + }, + { + contents: "-a one\n-b two", + expectedA: "one", + expectedB: "two", + }, + { + contents: "-a one \\\n-b two", + expectedA: "one", + expectedB: "two", + }, + { + contents: "-a one", + cliArgs: []string{"-b", "two"}, + expectedA: "one", + expectedB: "two", + }, + { + contents: "-a one\n-b two", + cliArgs: []string{"-b", "three"}, + expectedA: "one", + expectedB: "three", + }, + { + contents: "-a one\n-b $TEST_VAR", + env: map[string]string{"TEST_VAR": "from env"}, + expectedA: "one", + expectedB: "from env", + }, + { + contents: "-a one -b two -c three", + expectedErr: "flag provided but not defined: -c", + }, + } + + initalArgs := os.Args[:] + for _, tc := range tests { + a, b = "", "" + os.Args = initalArgs[:] + if len(tc.cliArgs) > 0 { + os.Args = append(os.Args, tc.cliArgs...) + } + for k, v := range tc.env { + if err := os.Setenv(k, v); err != nil { + t.Errorf("os.SetEnv failed: %s", err) + } + } + err := parseFlags(tc.contents) + if err != nil { + if err.Error() == tc.expectedErr { + continue + } + t.Errorf("parseFlags failed: wanted: %q, got: %q", tc.expectedErr, err) + continue + } + if tc.expectedA != a { + t.Errorf("flag 'a' not properly set: wanted: %q, got %q", tc.expectedA, a) + } + if tc.expectedB != b { + t.Errorf("flag 'b' not properly set: wanted: %q, got %q", tc.expectedB, b) + } + } +} From 661daaf2ff13a1fee5f895b7d1350e59fa3331ab Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Wed, 24 May 2017 14:46:20 -0700 Subject: [PATCH 09/10] Use glog.Exitf instead of fmt.Fprintf and os.Exit --- cmd/createtree/main.go | 4 ++-- server/trillian_log_server/main.go | 5 +---- server/trillian_log_signer/main.go | 3 +-- server/vmap/trillian_map_server/main.go | 5 +---- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/cmd/createtree/main.go b/cmd/createtree/main.go index f35c1673ae..55e7f8398a 100644 --- a/cmd/createtree/main.go +++ b/cmd/createtree/main.go @@ -38,6 +38,7 @@ import ( "fmt" "os" + "github.com/golang/glog" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/any" "github.com/google/trillian" @@ -180,8 +181,7 @@ func main() { if *configFile != "" { if err := cmd.ParseFlagFile(*configFile); err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) - os.Exit(1) + glog.Exitf("Failed to load flags from config file %q: %s", *configFile, err) } } diff --git a/server/trillian_log_server/main.go b/server/trillian_log_server/main.go index 8ca07c4585..d5471a7e6f 100644 --- a/server/trillian_log_server/main.go +++ b/server/trillian_log_server/main.go @@ -17,9 +17,7 @@ package main import ( "context" "flag" - "fmt" _ "net/http/pprof" - "os" "strings" "time" @@ -60,8 +58,7 @@ func main() { if *configFile != "" { if err := cmd.ParseFlagFile(*configFile); err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) - os.Exit(1) + glog.Exitf("Failed to load flags from config file %q: %s", *configFile, err) } } diff --git a/server/trillian_log_signer/main.go b/server/trillian_log_signer/main.go index 2a5e242b71..1fe705085e 100644 --- a/server/trillian_log_signer/main.go +++ b/server/trillian_log_signer/main.go @@ -60,8 +60,7 @@ func main() { if *configFile != "" { if err := cmd.ParseFlagFile(*configFile); err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) - os.Exit(1) + glog.Exitf("Failed to load flags from config file %q: %s", *configFile, err) } } diff --git a/server/vmap/trillian_map_server/main.go b/server/vmap/trillian_map_server/main.go index 39eb01e7f9..325045ae3e 100644 --- a/server/vmap/trillian_map_server/main.go +++ b/server/vmap/trillian_map_server/main.go @@ -17,9 +17,7 @@ package main import ( "context" "flag" - "fmt" _ "net/http/pprof" - "os" _ "github.com/go-sql-driver/mysql" // Load MySQL driver @@ -51,8 +49,7 @@ func main() { if *configFile != "" { if err := cmd.ParseFlagFile(*configFile); err != nil { - fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) - os.Exit(1) + glog.Exitf("Failed to load flags from config file %q: %s", *configFile, err) } } From c49bc08df7fa878b12731114c064ee4eecf3bb0d Mon Sep 17 00:00:00 2001 From: Rob Percival Date: Thu, 25 May 2017 09:51:14 +0100 Subject: [PATCH 10/10] Address review comments --- cmd/flags_test.go | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/cmd/flags_test.go b/cmd/flags_test.go index 48cedbbf4c..734f83308b 100644 --- a/cmd/flags_test.go +++ b/cmd/flags_test.go @@ -28,6 +28,7 @@ func TestParseFlags(t *testing.T) { flag.CommandLine.Init(os.Args[0], flag.ContinueOnError) tests := []struct { + name string contents string env map[string]string cliArgs []string @@ -36,69 +37,73 @@ func TestParseFlags(t *testing.T) { expectedB string }{ { + name: "two flags per line", contents: "-a one -b two", expectedA: "one", expectedB: "two", }, { + name: "one flag per line", contents: "-a one\n-b two", expectedA: "one", expectedB: "two", }, { + name: "one flag per line, with line continuation", contents: "-a one \\\n-b two", expectedA: "one", expectedB: "two", }, { + name: "one flag in file, one flag on command-line", contents: "-a one", cliArgs: []string{"-b", "two"}, expectedA: "one", expectedB: "two", }, { + name: "two flags, one overridden by command-line", contents: "-a one\n-b two", cliArgs: []string{"-b", "three"}, expectedA: "one", expectedB: "three", }, { + name: "two flags, one using an environment variable", contents: "-a one\n-b $TEST_VAR", env: map[string]string{"TEST_VAR": "from env"}, expectedA: "one", expectedB: "from env", }, { + name: "three flags, one undefined", contents: "-a one -b two -c three", expectedErr: "flag provided but not defined: -c", }, } - initalArgs := os.Args[:] + initialArgs := os.Args[:] for _, tc := range tests { a, b = "", "" - os.Args = initalArgs[:] - if len(tc.cliArgs) > 0 { - os.Args = append(os.Args, tc.cliArgs...) - } + os.Args = append(initialArgs, tc.cliArgs...) for k, v := range tc.env { if err := os.Setenv(k, v); err != nil { - t.Errorf("os.SetEnv failed: %s", err) + t.Errorf("%v: os.SetEnv(%q, %q) = %q", tc.name, k, v, err) } } - err := parseFlags(tc.contents) - if err != nil { - if err.Error() == tc.expectedErr { - continue + + if err := parseFlags(tc.contents); err != nil { + if err.Error() != tc.expectedErr { + t.Errorf("%v: parseFlags() = %q, want %q", tc.name, err, tc.expectedErr) } - t.Errorf("parseFlags failed: wanted: %q, got: %q", tc.expectedErr, err) continue } + if tc.expectedA != a { - t.Errorf("flag 'a' not properly set: wanted: %q, got %q", tc.expectedA, a) + t.Errorf("%v: flag 'a' not properly set: got %q, want %q", tc.name, a, tc.expectedA) } if tc.expectedB != b { - t.Errorf("flag 'b' not properly set: wanted: %q, got %q", tc.expectedB, b) + t.Errorf("%v: flag 'b' not properly set: got %q, want %q", tc.name, b, tc.expectedB) } } }