From cbdaff82c1134b3f1302f8de07e98877aa377f70 Mon Sep 17 00:00:00 2001 From: Ben Kraft Date: Sun, 7 May 2023 13:13:40 -0700 Subject: [PATCH] Add some more tests for config validation, and fix some gaps I set these tests up in #270, but I realized there's a lot more we could test unrelated to that PR. In this commit I add some more tests. They're not really exhaustive yet, but they did catch a few bugs, which I fixed: - we weren't validating the package-name if you do set it (only if we guess it) - If you omitted `generated`, you would try to write generated code to the directory containing `genqlient.yaml`, which makes no sense; now we default to `generated.go`. In the real world it's probably good to set explicitly, but it's actually very convenient in tests that we don't have to, and maybe in small projects too. --- docs/genqlient.yaml | 3 +- generate/config.go | 15 ++++++-- generate/config_test.go | 37 +++++++++++++++---- generate/generate_test.go | 19 +--------- .../invalid-config/CantGuessPackage.yaml | 0 .../invalid-config/InvalidCasing.yaml | 1 + .../invalid-config/InvalidOptional.yaml | 2 + .../invalid-config/InvalidPackage.yaml | 1 + ...ageBindings-testdata-queries-generated.go} | 2 +- .../TestInvalidConfigs-CantGuessPackage.yaml | 3 ++ .../TestInvalidConfigs-InvalidOptional.yaml | 1 + .../TestInvalidConfigs-InvalidPackage.yaml | 1 + .../snapshots/TestValidConfigs-Lists.yaml | 27 ++++++++++++++ .../snapshots/TestValidConfigs-Simple.yaml | 21 +++++++++++ .../snapshots/TestValidConfigs-Strings.yaml | 25 +++++++++++++ generate/testdata/valid-config/Lists.yaml | 9 +++++ generate/testdata/valid-config/Simple.yaml | 1 + generate/testdata/valid-config/Strings.yaml | 3 ++ 18 files changed, 141 insertions(+), 30 deletions(-) create mode 100644 generate/testdata/invalid-config/CantGuessPackage.yaml create mode 100644 generate/testdata/invalid-config/InvalidOptional.yaml create mode 100644 generate/testdata/invalid-config/InvalidPackage.yaml rename generate/testdata/snapshots/{TestGenerateWithConfig-PackageBindings-testdata-queries => TestGenerateWithConfig-PackageBindings-testdata-queries-generated.go} (98%) create mode 100644 generate/testdata/snapshots/TestInvalidConfigs-CantGuessPackage.yaml create mode 100644 generate/testdata/snapshots/TestInvalidConfigs-InvalidOptional.yaml create mode 100644 generate/testdata/snapshots/TestInvalidConfigs-InvalidPackage.yaml create mode 100644 generate/testdata/snapshots/TestValidConfigs-Lists.yaml create mode 100644 generate/testdata/snapshots/TestValidConfigs-Simple.yaml create mode 100644 generate/testdata/snapshots/TestValidConfigs-Strings.yaml create mode 100644 generate/testdata/valid-config/Lists.yaml create mode 100644 generate/testdata/valid-config/Simple.yaml create mode 100644 generate/testdata/valid-config/Strings.yaml diff --git a/docs/genqlient.yaml b/docs/genqlient.yaml index 1281321e..ddfb760b 100644 --- a/docs/genqlient.yaml +++ b/docs/genqlient.yaml @@ -26,7 +26,7 @@ operations: - "pkg/*.go" # The filename to which to write the generated code, relative to -# genqlient.yaml. +# genqlient.yaml. Default: generated.go. generated: generated/genqlient.go # The package name for the output code; defaults to the directory name of @@ -77,7 +77,6 @@ context_type: context.Context # without making a query. client_getter: "github.com/you/yourpkg.GetClient" - # If set, fields with a struct type will default to having # the "pointer: true, omitempty: true" flag. # diff --git a/generate/config.go b/generate/config.go index 65bc53cf..94ee12c4 100644 --- a/generate/config.go +++ b/generate/config.go @@ -145,6 +145,9 @@ func (c *Config) ValidateAndFillDefaults(baseDir string) error { for i := range c.Operations { c.Operations[i] = pathJoin(baseDir, c.Operations[i]) } + if c.Generated == "" { + c.Generated = "generated.go" + } c.Generated = pathJoin(baseDir, c.Generated) if c.ExportOperations != "" { c.ExportOperations = pathJoin(baseDir, c.ExportOperations) @@ -164,17 +167,23 @@ func (c *Config) ValidateAndFillDefaults(baseDir string) error { "\nExample: \"github.com/Org/Repo/optional.Value\"") } - if c.Package == "" { + if c.Package != "" { + if !token.IsIdentifier(c.Package) { + // No need for link here -- if you're already setting the package + // you know where to set the package. + return errorf(nil, "invalid package in genqlient.yaml: '%v' is not a valid identifier", c.Package) + } + } else { abs, err := filepath.Abs(c.Generated) if err != nil { - return errorf(nil, "unable to guess package-name: %v is not a valid identifier"+ + return errorf(nil, "unable to guess package-name: %v"+ "\nSet package name in genqlient.yaml"+ "\nExample: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6", err) } base := filepath.Base(filepath.Dir(abs)) if !token.IsIdentifier(base) { - return errorf(nil, "unable to guess package-name: %v is not a valid identifier"+ + return errorf(nil, "unable to guess package-name: '%v' is not a valid identifier"+ "\nSet package name in genqlient.yaml"+ "\nExample: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6", base) } diff --git a/generate/config_test.go b/generate/config_test.go index 5e0b53e8..004c8f49 100644 --- a/generate/config_test.go +++ b/generate/config_test.go @@ -12,6 +12,7 @@ import ( const ( findConfigDir = "testdata/find-config" + validConfigDir = "testdata/valid-config" invalidConfigDir = "testdata/invalid-config" ) @@ -114,18 +115,40 @@ func TestAbsoluteAndRelativePathsInConfigFiles(t *testing.T) { require.Equal(t, "/tmp/genqlient.graphql", config.Operations[0]) } -func TestInvalidConfigs(t *testing.T) { - files, err := os.ReadDir(invalidConfigDir) +func testAllSnapshots( + t *testing.T, + dir string, + testfunc func(t *testing.T, filename string), +) { + files, err := os.ReadDir(dir) if err != nil { t.Fatal(err) } for _, file := range files { - t.Run(file.Name(), func(t *testing.T) { - filename := filepath.Join(invalidConfigDir, file.Name()) - _, err := ReadAndValidateConfig(filename) - require.Error(t, err) - testutil.Cupaloy.SnapshotT(t, err.Error()) + name := file.Name() + if name[0] == '.' { + continue // editor backup files, etc. + } + t.Run(name, func(t *testing.T) { + filename := filepath.Join(dir, file.Name()) + testfunc(t, filename) }) } } + +func TestValidConfigs(t *testing.T) { + testAllSnapshots(t, validConfigDir, func(t *testing.T, filename string) { + config, err := ReadAndValidateConfig(filename) + require.NoError(t, err) + testutil.Cupaloy.SnapshotT(t, config) + }) +} + +func TestInvalidConfigs(t *testing.T) { + testAllSnapshots(t, invalidConfigDir, func(t *testing.T, filename string) { + _, err := ReadAndValidateConfig(filename) + require.Error(t, err) + testutil.Cupaloy.SnapshotT(t, err.Error()) + }) +} diff --git a/generate/generate_test.go b/generate/generate_test.go index 6baab562..62390ebd 100644 --- a/generate/generate_test.go +++ b/generate/generate_test.go @@ -164,20 +164,16 @@ func TestGenerateWithConfig(t *testing.T) { Package: "mypkg", }}, {"ExportOperations", "", nil, &Config{ - Generated: "generated.go", ExportOperations: "operations.json", }}, {"CustomContext", "", nil, &Config{ - Generated: "generated.go", ContextType: "github.com/Khan/genqlient/internal/testutil.MyContext", }}, {"CustomContextWithAlias", "", nil, &Config{ - Generated: "generated.go", ContextType: "github.com/Khan/genqlient/internal/testutil/junk---fun.name.MyContext", }}, {"StructReferences", "", []string{"InputObject.graphql", "QueryWithStructs.graphql"}, &Config{ StructReferences: true, - Generated: "generated.go", Bindings: map[string]*TypeBinding{ "Date": { Type: "time.Time", @@ -189,7 +185,6 @@ func TestGenerateWithConfig(t *testing.T) { {"StructReferencesAndOptionalPointer", "", []string{"InputObject.graphql", "QueryWithStructs.graphql"}, &Config{ StructReferences: true, Optional: "pointer", - Generated: "generated.go", Bindings: map[string]*TypeBinding{ "Date": { Type: "time.Time", @@ -204,48 +199,38 @@ func TestGenerateWithConfig(t *testing.T) { }, }}, {"NoContext", "", nil, &Config{ - Generated: "generated.go", ContextType: "-", }}, {"ClientGetter", "", nil, &Config{ - Generated: "generated.go", ClientGetter: "github.com/Khan/genqlient/internal/testutil.GetClientFromContext", }}, {"ClientGetterCustomContext", "", nil, &Config{ - Generated: "generated.go", ClientGetter: "github.com/Khan/genqlient/internal/testutil.GetClientFromMyContext", ContextType: "github.com/Khan/genqlient/internal/testutil.MyContext", }}, {"ClientGetterNoContext", "", nil, &Config{ - Generated: "generated.go", ClientGetter: "github.com/Khan/genqlient/internal/testutil.GetClientFromNowhere", ContextType: "-", }}, {"Extensions", "", nil, &Config{ - Generated: "generated.go", Extensions: true, }}, {"OptionalValue", "", []string{"ListInput.graphql", "QueryWithSlices.graphql"}, &Config{ - Generated: "generated.go", - Optional: "value", + Optional: "value", }}, {"OptionalPointer", "", []string{"ListInput.graphql", "QueryWithSlices.graphql"}, &Config{ - Generated: "generated.go", - Optional: "pointer", + Optional: "pointer", }}, {"OptionalGeneric", "", []string{"ListInput.graphql", "QueryWithSlices.graphql"}, &Config{ - Generated: "generated.go", Optional: "generic", OptionalGenericType: "github.com/Khan/genqlient/internal/testutil.Option", }}, {"EnumRawCasingAll", "", []string{"QueryWithEnums.graphql"}, &Config{ - Generated: "generated.go", Casing: Casing{ AllEnums: CasingRaw, }, }}, {"EnumRawCasingSpecific", "", []string{"QueryWithEnums.graphql"}, &Config{ - Generated: "generated.go", Casing: Casing{ Enums: map[string]CasingAlgorithm{"Role": CasingRaw}, }, diff --git a/generate/testdata/invalid-config/CantGuessPackage.yaml b/generate/testdata/invalid-config/CantGuessPackage.yaml new file mode 100644 index 00000000..e69de29b diff --git a/generate/testdata/invalid-config/InvalidCasing.yaml b/generate/testdata/invalid-config/InvalidCasing.yaml index 1f63ceed..67754977 100644 --- a/generate/testdata/invalid-config/InvalidCasing.yaml +++ b/generate/testdata/invalid-config/InvalidCasing.yaml @@ -1,3 +1,4 @@ +package: invalidConfig casing: enums: MyType: bogus diff --git a/generate/testdata/invalid-config/InvalidOptional.yaml b/generate/testdata/invalid-config/InvalidOptional.yaml new file mode 100644 index 00000000..67151e6a --- /dev/null +++ b/generate/testdata/invalid-config/InvalidOptional.yaml @@ -0,0 +1,2 @@ +package: invalidConfig +optional: bogus diff --git a/generate/testdata/invalid-config/InvalidPackage.yaml b/generate/testdata/invalid-config/InvalidPackage.yaml new file mode 100644 index 00000000..9f7372fc --- /dev/null +++ b/generate/testdata/invalid-config/InvalidPackage.yaml @@ -0,0 +1 @@ +package: bogus-package-name diff --git a/generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries b/generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries-generated.go similarity index 98% rename from generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries rename to generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries-generated.go index 5ca60d3f..5838a0ad 100644 --- a/generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries +++ b/generate/testdata/snapshots/TestGenerateWithConfig-PackageBindings-testdata-queries-generated.go @@ -1,6 +1,6 @@ // Code generated by github.com/Khan/genqlient, DO NOT EDIT. -package testdata +package queries import ( "context" diff --git a/generate/testdata/snapshots/TestInvalidConfigs-CantGuessPackage.yaml b/generate/testdata/snapshots/TestInvalidConfigs-CantGuessPackage.yaml new file mode 100644 index 00000000..2571f712 --- /dev/null +++ b/generate/testdata/snapshots/TestInvalidConfigs-CantGuessPackage.yaml @@ -0,0 +1,3 @@ +invalid config file testdata/invalid-config/CantGuessPackage.yaml: unable to guess package-name: 'invalid-config' is not a valid identifier +Set package name in genqlient.yaml +Example: https://github.com/Khan/genqlient/blob/main/example/genqlient.yaml#L6 diff --git a/generate/testdata/snapshots/TestInvalidConfigs-InvalidOptional.yaml b/generate/testdata/snapshots/TestInvalidConfigs-InvalidOptional.yaml new file mode 100644 index 00000000..fc5394bf --- /dev/null +++ b/generate/testdata/snapshots/TestInvalidConfigs-InvalidOptional.yaml @@ -0,0 +1 @@ +invalid config file testdata/invalid-config/InvalidOptional.yaml: optional must be one of: 'value' (default), 'pointer', or 'generic' diff --git a/generate/testdata/snapshots/TestInvalidConfigs-InvalidPackage.yaml b/generate/testdata/snapshots/TestInvalidConfigs-InvalidPackage.yaml new file mode 100644 index 00000000..a3d047a2 --- /dev/null +++ b/generate/testdata/snapshots/TestInvalidConfigs-InvalidPackage.yaml @@ -0,0 +1 @@ +invalid config file testdata/invalid-config/InvalidPackage.yaml: invalid package in genqlient.yaml: 'bogus-package-name' is not a valid identifier diff --git a/generate/testdata/snapshots/TestValidConfigs-Lists.yaml b/generate/testdata/snapshots/TestValidConfigs-Lists.yaml new file mode 100644 index 00000000..7ff70462 --- /dev/null +++ b/generate/testdata/snapshots/TestValidConfigs-Lists.yaml @@ -0,0 +1,27 @@ +(*generate.Config)({ + Schema: (generate.StringList) (len=2) { + (string) (len=42) "testdata/valid-config/first_schema.graphql", + (string) (len=43) "testdata/valid-config/second_schema.graphql" + }, + Operations: (generate.StringList) (len=2) { + (string) (len=46) "testdata/valid-config/first_operations.graphql", + (string) (len=47) "testdata/valid-config/second_operations.graphql" + }, + Generated: (string) (len=34) "testdata/valid-config/generated.go", + Package: (string) (len=11) "validConfig", + ExportOperations: (string) "", + ContextType: (string) (len=15) "context.Context", + ClientGetter: (string) "", + Bindings: (map[string]*generate.TypeBinding) , + PackageBindings: ([]*generate.PackageBinding) , + Casing: (generate.Casing) { + AllEnums: (generate.CasingAlgorithm) "", + Enums: (map[string]generate.CasingAlgorithm) + }, + Optional: (string) "", + OptionalGenericType: (string) "", + StructReferences: (bool) false, + Extensions: (bool) false, + AllowBrokenFeatures: (bool) false, + baseDir: (string) (len=21) "testdata/valid-config" +}) diff --git a/generate/testdata/snapshots/TestValidConfigs-Simple.yaml b/generate/testdata/snapshots/TestValidConfigs-Simple.yaml new file mode 100644 index 00000000..77cc1b32 --- /dev/null +++ b/generate/testdata/snapshots/TestValidConfigs-Simple.yaml @@ -0,0 +1,21 @@ +(*generate.Config)({ + Schema: (generate.StringList) , + Operations: (generate.StringList) , + Generated: (string) (len=34) "testdata/valid-config/generated.go", + Package: (string) (len=11) "validConfig", + ExportOperations: (string) "", + ContextType: (string) (len=15) "context.Context", + ClientGetter: (string) "", + Bindings: (map[string]*generate.TypeBinding) , + PackageBindings: ([]*generate.PackageBinding) , + Casing: (generate.Casing) { + AllEnums: (generate.CasingAlgorithm) "", + Enums: (map[string]generate.CasingAlgorithm) + }, + Optional: (string) "", + OptionalGenericType: (string) "", + StructReferences: (bool) false, + Extensions: (bool) false, + AllowBrokenFeatures: (bool) false, + baseDir: (string) (len=21) "testdata/valid-config" +}) diff --git a/generate/testdata/snapshots/TestValidConfigs-Strings.yaml b/generate/testdata/snapshots/TestValidConfigs-Strings.yaml new file mode 100644 index 00000000..1638e8da --- /dev/null +++ b/generate/testdata/snapshots/TestValidConfigs-Strings.yaml @@ -0,0 +1,25 @@ +(*generate.Config)({ + Schema: (generate.StringList) (len=1) { + (string) (len=36) "testdata/valid-config/schema.graphql" + }, + Operations: (generate.StringList) (len=1) { + (string) (len=40) "testdata/valid-config/operations.graphql" + }, + Generated: (string) (len=34) "testdata/valid-config/generated.go", + Package: (string) (len=11) "validConfig", + ExportOperations: (string) "", + ContextType: (string) (len=15) "context.Context", + ClientGetter: (string) "", + Bindings: (map[string]*generate.TypeBinding) , + PackageBindings: ([]*generate.PackageBinding) , + Casing: (generate.Casing) { + AllEnums: (generate.CasingAlgorithm) "", + Enums: (map[string]generate.CasingAlgorithm) + }, + Optional: (string) "", + OptionalGenericType: (string) "", + StructReferences: (bool) false, + Extensions: (bool) false, + AllowBrokenFeatures: (bool) false, + baseDir: (string) (len=21) "testdata/valid-config" +}) diff --git a/generate/testdata/valid-config/Lists.yaml b/generate/testdata/valid-config/Lists.yaml new file mode 100644 index 00000000..8348498b --- /dev/null +++ b/generate/testdata/valid-config/Lists.yaml @@ -0,0 +1,9 @@ +schema: +- first_schema.graphql +- second_schema.graphql + +operations: +- first_operations.graphql +- second_operations.graphql + +package: validConfig diff --git a/generate/testdata/valid-config/Simple.yaml b/generate/testdata/valid-config/Simple.yaml new file mode 100644 index 00000000..3084ef55 --- /dev/null +++ b/generate/testdata/valid-config/Simple.yaml @@ -0,0 +1 @@ +package: validConfig diff --git a/generate/testdata/valid-config/Strings.yaml b/generate/testdata/valid-config/Strings.yaml new file mode 100644 index 00000000..7830e0f9 --- /dev/null +++ b/generate/testdata/valid-config/Strings.yaml @@ -0,0 +1,3 @@ +schema: schema.graphql +operations: operations.graphql +package: validConfig