Skip to content

Commit

Permalink
Added error check for fs.Parse() call
Browse files Browse the repository at this point in the history
This is just for tests since from main.go, the FlagSet is set
with ExitOnError so Parse() would call os.Exit(2).
Regardless, I wanted to add error checking and a test for that.

Related to #578
  • Loading branch information
kozlovic committed Sep 7, 2017
1 parent dbe9bc7 commit 8f6d3f5
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
7 changes: 6 additions & 1 deletion server/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,9 @@ func ConfigureOptions(fs *flag.FlagSet, args []string) (*Options, error) {
// Calling Parse() before processing config file is necessary since configFile
// itself is a command line argument, and also Parse() is required in order
// to know if user wants simply to show "help" or "version", etc...
fs.Parse(args)
if err := fs.Parse(args); err != nil {
return nil, err
}

// Show version and exit
if showVersion {
Expand Down Expand Up @@ -1026,6 +1028,9 @@ func ConfigureOptions(fs *flag.FlagSet, args []string) (*Options, error) {
return nil, err
}
// Call this again to override config file options with options from command line.
// Note: We don't need to check error here since if there was an error, it would
// have been caught the first time this function was called (after setting up the
// flags).
fs.Parse(args)
}

Expand Down
8 changes: 8 additions & 0 deletions server/opts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package server

import (
"bytes"
"crypto/tls"
"flag"
"io/ioutil"
Expand Down Expand Up @@ -853,6 +854,10 @@ func TestConfigureOptions(t *testing.T) {
// Helper function that expect configuration to fail.
expectToFail := func(args []string, errContent ...string) {
fs := flag.NewFlagSet("test", flag.ContinueOnError)
// Silence the flagSet so that on failure nothing is printed.
// (flagSet would print error message about unknown flags, etc..)
silenceOuput := &bytes.Buffer{}
fs.SetOutput(silenceOuput)
opts, err := ConfigureOptions(fs, args)
if opts != nil || err == nil {
stackFatalf(t, "Expected no option and an error, got opts=%v and err=%v", opts, err)
Expand All @@ -875,6 +880,9 @@ func TestConfigureOptions(t *testing.T) {
// Should fail because of unknown parameter
expectToFail([]string{"foo"}, "command")

// Should fail because unkown flag
expectToFail([]string{"-xxx", "foo"}, "flag")

// Should fail because of config file missing
expectToFail([]string{"-c", "xxx.cfg"}, "file")

Expand Down

0 comments on commit 8f6d3f5

Please sign in to comment.