diff --git a/ffcli/command_test.go b/ffcli/command_test.go index 6c90c5b..e1988cb 100644 --- a/ffcli/command_test.go +++ b/ffcli/command_test.go @@ -133,13 +133,13 @@ func TestCommandRun(t *testing.T) { err := root.ParseAndRun(context.Background(), testcase.args) assertNoError(t, err) - assertNoError(t, fftest.Compare(&testcase.rootvars, rootvars)) + fftest.Compare(t, &testcase.rootvars, rootvars) assertBool(t, testcase.rootran, rootran) assertStringSlice(t, testcase.rootargs, rootargs) - assertNoError(t, fftest.Compare(&testcase.foovars, foovars)) + fftest.Compare(t, &testcase.foovars, foovars) assertBool(t, testcase.fooran, fooran) assertStringSlice(t, testcase.fooargs, fooargs) - assertNoError(t, fftest.Compare(&testcase.barvars, barvars)) + fftest.Compare(t, &testcase.barvars, barvars) assertBool(t, testcase.barran, barran) assertStringSlice(t, testcase.barargs, barargs) }) diff --git a/fftest/vars.go b/fftest/vars.go index a181c8c..d7e3d39 100644 --- a/fftest/vars.go +++ b/fftest/vars.go @@ -3,19 +3,23 @@ package fftest import ( "errors" "flag" - "fmt" "reflect" "strings" + "testing" "time" ) -// Pair returns a predefined flag set, and a predefined set of variables that -// have been registered into it. Tests can call parse on the flag set with a -// variety of flags, config files, and env vars, and check the resulting effect -// on the vars. +// Pair defines and returns an empty flag set and vars assigned to it. func Pair() (*flag.FlagSet, *Vars) { fs := flag.NewFlagSet("fftest", flag.ContinueOnError) + vars := DefaultVars(fs) + return fs, vars +} +// DefaultVars registers a predefined set of variables to the flag set. +// Tests can call parse on the flag set with a variety of flags, config files, +// and env vars, and check the resulting effect on the vars. +func DefaultVars(fs *flag.FlagSet) *Vars { var v Vars fs.StringVar(&v.S, "s", "", "string") fs.IntVar(&v.I, "i", 0, "int") @@ -23,8 +27,21 @@ func Pair() (*flag.FlagSet, *Vars) { fs.BoolVar(&v.B, "b", false, "bool") fs.DurationVar(&v.D, "d", 0*time.Second, "time.Duration") fs.Var(&v.X, "x", "collection of strings (repeatable)") + return &v +} - return fs, &v +// NonzeroDefaultVars is like DefaultVars, but provides each primitive flag with +// a nonzero default value. This is useful for tests that explicitly provide a +// zero value for the type. +func NonzeroDefaultVars(fs *flag.FlagSet) *Vars { + var v Vars + fs.StringVar(&v.S, "s", "foo", "string") + fs.IntVar(&v.I, "i", 123, "int") + fs.Float64Var(&v.F, "f", 9.99, "float64") + fs.BoolVar(&v.B, "b", true, "bool") + fs.DurationVar(&v.D, "d", 3*time.Hour, "time.Duration") + fs.Var(&v.X, "x", "collection of strings (repeatable)") + return &v } // Vars are a common set of variables used for testing. @@ -51,52 +68,51 @@ type Vars struct { } // Compare one set of vars with another -// and return an error on any difference. -func Compare(want, have *Vars) error { +// and t.Error on any difference. +func Compare(t *testing.T, want, have *Vars) { + t.Helper() + if want.WantParseErrorIs != nil || want.WantParseErrorString != "" { if want.WantParseErrorIs != nil && have.ParseError == nil { - return fmt.Errorf("want error (%v), have none", want.WantParseErrorIs) + t.Errorf("want error (%v), have none", want.WantParseErrorIs) } - if want.WantParseErrorString != "" && have.ParseError == nil { - return fmt.Errorf("want error (%q), have none", want.WantParseErrorString) + t.Errorf("want error (%q), have none", want.WantParseErrorString) } - if want.WantParseErrorIs == nil && want.WantParseErrorString == "" && have.ParseError != nil { - return fmt.Errorf("want clean parse, have error (%v)", have.ParseError) + t.Errorf("want clean parse, have error (%v)", have.ParseError) } - if want.WantParseErrorIs != nil && have.ParseError != nil && !errors.Is(have.ParseError, want.WantParseErrorIs) { - return fmt.Errorf("want wrapped error (%#+v), have error (%#+v)", want.WantParseErrorIs, have.ParseError) + t.Errorf("want wrapped error (%#+v), have error (%#+v)", want.WantParseErrorIs, have.ParseError) } - if want.WantParseErrorString != "" && have.ParseError != nil && !strings.Contains(have.ParseError.Error(), want.WantParseErrorString) { - return fmt.Errorf("want error string (%q), have error (%v)", want.WantParseErrorString, have.ParseError) + t.Errorf("want error string (%q), have error (%v)", want.WantParseErrorString, have.ParseError) } + return + } - return nil + if have.ParseError != nil { + t.Errorf("error: %v", have.ParseError) } if want.S != have.S { - return fmt.Errorf("var S: want %q, have %q", want.S, have.S) + t.Errorf("var S: want %q, have %q", want.S, have.S) } if want.I != have.I { - return fmt.Errorf("var I: want %d, have %d", want.I, have.I) + t.Errorf("var I: want %d, have %d", want.I, have.I) } if want.F != have.F { - return fmt.Errorf("var F: want %f, have %f", want.F, have.F) + t.Errorf("var F: want %f, have %f", want.F, have.F) } if want.B != have.B { - return fmt.Errorf("var B: want %v, have %v", want.B, have.B) + t.Errorf("var B: want %v, have %v", want.B, have.B) } if want.D != have.D { - return fmt.Errorf("var D: want %s, have %s", want.D, have.D) + t.Errorf("var D: want %s, have %s", want.D, have.D) } if !reflect.DeepEqual(want.X, have.X) { - return fmt.Errorf("var X: want %v, have %v", want.X, have.X) + t.Errorf("var X: want %v, have %v", want.X, have.X) } - - return nil } // StringSlice is a flag.Value that collects each Set string diff --git a/fftoml/fftoml_test.go b/fftoml/fftoml_test.go index c2b035b..55de143 100644 --- a/fftoml/fftoml_test.go +++ b/fftoml/fftoml_test.go @@ -48,9 +48,7 @@ func TestParser(t *testing.T) { ff.WithConfigFile(testcase.file), ff.WithConfigFileParser(fftoml.Parser), ) - if err := fftest.Compare(&testcase.want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &testcase.want, vars) }) } } diff --git a/ffyaml/ffyaml.go b/ffyaml/ffyaml.go index 9421998..8fe88e4 100644 --- a/ffyaml/ffyaml.go +++ b/ffyaml/ffyaml.go @@ -1,4 +1,4 @@ -// Package ffyaml provides a YAML config file paser. +// Package ffyaml provides a YAML config file parser. package ffyaml import ( @@ -68,6 +68,8 @@ func valToStr(val interface{}) (string, error) { return strconv.FormatInt(v, 10), nil case float64: return strconv.FormatFloat(v, 'g', -1, 64), nil + case nil: + return "", nil default: return "", ff.StringConversionError{Value: val} } diff --git a/ffyaml/ffyaml_test.go b/ffyaml/ffyaml_test.go index 8cc42e1..7667c09 100644 --- a/ffyaml/ffyaml_test.go +++ b/ffyaml/ffyaml_test.go @@ -1,6 +1,8 @@ package ffyaml_test import ( + "flag" + "os" "testing" "time" @@ -13,13 +15,15 @@ func TestParser(t *testing.T) { t.Parallel() for _, testcase := range []struct { + vars func(*flag.FlagSet) *fftest.Vars name string file string + miss bool // AllowMissingConfigFiles want fftest.Vars }{ { - name: "empty input", - file: "testdata/empty_input.yaml", + name: "empty", + file: "testdata/empty.yaml", want: fftest.Vars{}, }, { @@ -33,9 +37,16 @@ func TestParser(t *testing.T) { want: fftest.Vars{WantParseErrorString: "found character that cannot start any token"}, }, { - name: "no value", - file: "testdata/no_value.yaml", - want: fftest.Vars{WantParseErrorIs: ff.StringConversionError{}}, + vars: fftest.NonzeroDefaultVars, + name: "no value for s", + file: "testdata/no_value_s.yaml", + want: fftest.Vars{S: "", I: 123, F: 9.99, B: true, D: 3 * time.Hour}, + }, + { + vars: fftest.NonzeroDefaultVars, + name: "no value for i", + file: "testdata/no_value_i.yaml", + want: fftest.Vars{WantParseErrorString: "parse error"}, }, { name: "basic arrays", @@ -57,17 +68,31 @@ func TestParser(t *testing.T) { file: "testdata/unquoted_string_array.yaml", want: fftest.Vars{X: []string{"one", "two", "three"}}, }, + { + name: "missing config file allowed", + file: "testdata/this_file_does_not_exist.yaml", + miss: true, + want: fftest.Vars{}, + }, + { + name: "missing config file not allowed", + file: "testdata/this_file_does_not_exist.yaml", + miss: false, + want: fftest.Vars{WantParseErrorIs: os.ErrNotExist}, + }, } { t.Run(testcase.name, func(t *testing.T) { - fs, vars := fftest.Pair() + if testcase.vars == nil { + testcase.vars = fftest.DefaultVars + } + fs := flag.NewFlagSet("fftest", flag.ContinueOnError) + vars := testcase.vars(fs) vars.ParseError = ff.Parse(fs, []string{}, ff.WithConfigFile(testcase.file), ff.WithConfigFileParser(ffyaml.Parser), - ff.WithAllowMissingConfigFile(true), + ff.WithAllowMissingConfigFile(testcase.miss), ) - if err := fftest.Compare(&testcase.want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &testcase.want, vars) }) } } diff --git a/ffyaml/testdata/no_value_i.yaml b/ffyaml/testdata/no_value_i.yaml new file mode 100644 index 0000000..3301247 --- /dev/null +++ b/ffyaml/testdata/no_value_i.yaml @@ -0,0 +1,2 @@ +s: woozlewozzle +i: diff --git a/ffyaml/testdata/no_value.yaml b/ffyaml/testdata/no_value_s.yaml similarity index 100% rename from ffyaml/testdata/no_value.yaml rename to ffyaml/testdata/no_value_s.yaml index 5c1f509..3b977b1 100644 --- a/ffyaml/testdata/no_value.yaml +++ b/ffyaml/testdata/no_value_s.yaml @@ -1,2 +1,2 @@ -i: 123 s: +i: 123 diff --git a/json_test.go b/json_test.go index 7929da6..39a1b90 100644 --- a/json_test.go +++ b/json_test.go @@ -49,9 +49,7 @@ func TestJSONParser(t *testing.T) { ff.WithConfigFile(testcase.file), ff.WithConfigFileParser(ff.JSONParser), ) - if err := fftest.Compare(&testcase.want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &testcase.want, vars) }) } } diff --git a/parse_test.go b/parse_test.go index 2ca2ef3..8524c5b 100644 --- a/parse_test.go +++ b/parse_test.go @@ -152,9 +152,7 @@ func TestParseBasics(t *testing.T) { fs, vars := fftest.Pair() vars.ParseError = ff.Parse(fs, testcase.args, testcase.opts...) - if err := fftest.Compare(&testcase.want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &testcase.want, vars) }) } } @@ -204,9 +202,7 @@ func TestParseIssue16(t *testing.T) { ) want := fftest.Vars{S: testcase.want} - if err := fftest.Compare(&want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &want, vars) }) } } @@ -251,9 +247,7 @@ func TestParseConfigFile(t *testing.T) { vars.ParseError = ff.Parse(fs, []string{}, options...) want := fftest.Vars{WantParseErrorIs: testcase.parseError} - if err := fftest.Compare(&want, vars); err != nil { - t.Fatal(err) - } + fftest.Compare(t, &want, vars) }) } }