Add some more tests for config validation, and fix some gaps (#274)

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

@@ -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.
 #

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)
 		}

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())
+		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())
 	})
 }
-}

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",
 		}},
 		{"OptionalPointer", "", []string{"ListInput.graphql", "QueryWithSlices.graphql"}, &Config{
-			Generated: "generated.go",
 			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},
 			},

generate/testdata/invalid-config/InvalidCasing.yaml

@@ -1,3 +1,4 @@
+package: invalidConfig
 casing:
   enums:
     MyType: bogus

generate/testdata/invalid-config/InvalidOptional.yaml

@@ -0,0 +1,2 @@
+package: invalidConfig
+optional: bogus

generate/testdata/invalid-config/InvalidPackage.yaml

@@ -0,0 +1 @@
+package: bogus-package-name

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"

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

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'

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

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) <nil>,
+  PackageBindings: ([]*generate.PackageBinding) <nil>,
+  Casing: (generate.Casing) {
+    AllEnums: (generate.CasingAlgorithm) "",
+    Enums: (map[string]generate.CasingAlgorithm) <nil>
+  },
+  Optional: (string) "",
+  OptionalGenericType: (string) "",
+  StructReferences: (bool) false,
+  Extensions: (bool) false,
+  AllowBrokenFeatures: (bool) false,
+  baseDir: (string) (len=21) "testdata/valid-config"
+})

generate/testdata/snapshots/TestValidConfigs-Simple.yaml

@@ -0,0 +1,21 @@
+(*generate.Config)({
+  Schema: (generate.StringList) <nil>,
+  Operations: (generate.StringList) <nil>,
+  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) <nil>,
+  PackageBindings: ([]*generate.PackageBinding) <nil>,
+  Casing: (generate.Casing) {
+    AllEnums: (generate.CasingAlgorithm) "",
+    Enums: (map[string]generate.CasingAlgorithm) <nil>
+  },
+  Optional: (string) "",
+  OptionalGenericType: (string) "",
+  StructReferences: (bool) false,
+  Extensions: (bool) false,
+  AllowBrokenFeatures: (bool) false,
+  baseDir: (string) (len=21) "testdata/valid-config"
+})

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) <nil>,
+  PackageBindings: ([]*generate.PackageBinding) <nil>,
+  Casing: (generate.Casing) {
+    AllEnums: (generate.CasingAlgorithm) "",
+    Enums: (map[string]generate.CasingAlgorithm) <nil>
+  },
+  Optional: (string) "",
+  OptionalGenericType: (string) "",
+  StructReferences: (bool) false,
+  Extensions: (bool) false,
+  AllowBrokenFeatures: (bool) false,
+  baseDir: (string) (len=21) "testdata/valid-config"
+})

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

generate/testdata/valid-config/Simple.yaml

@@ -0,0 +1 @@
+package: validConfig

generate/testdata/valid-config/Strings.yaml

@@ -0,0 +1,3 @@
+schema: schema.graphql
+operations: operations.graphql
+package: validConfig