Repository: Khan/genqlient
Pull request #196 (2022-05-15 19:17:36)

Simplify errors tests a bit so they don't all have to write a schema
Some of the errors tests need to have their own schema, so the schema
can do something weird (or even be entirely invalid!).  But most can
still share a schema.  In this commit I have those indeed share a
schema, to avoid having to have a bunch of copies of mostly the same
schema.  While doing so I noticed one error whose location wasn't very
useful, and fixed it.

Test plan: make check

<!--
Thanks for your contribution! Check out the
[contributing docs](https://github.com/Khan/genqlient/blob/main/docs/CONTRIBUTING.md)
for more on contributing to genqlient.
-->



I have:
- [x] Written a clear PR title and description (above)
- [x] Signed the [Khan Academy CLA](https://www.khanacademy.org/r/cla)
- [x] Added tests covering my changes, if applicable
- [x] Included a link to the issue fixed, if applicable
- [x] Included documentation, for new features
- [x] Added an entry to the changelog



Patch:
diff --git a/generate/convert.go b/generate/convert.go
index bde539d..38035f5 100644
--- a/generate/convert.go
+++ b/generate/convert.go
@@ -294,7 +294,8 @@ func (g *generator) convertDefinition(
 	globalBinding, ok := g.Config.Bindings[def.Name]
 	if ok && options.Bind != "-" {
 		if options.TypeName != "" {
-			return nil, errorf(pos,
+			// The option position (in the query) is more useful here.
+			return nil, errorf(options.pos,
 				"typename option conflicts with global binding for %s; "+
 					"use `bind: \"-\"` to override it", def.Name)
 		}


Test patch:
diff --git a/generate/generate_test.go b/generate/generate_test.go
index 762cb47..64690a2 100644
--- a/generate/generate_test.go
+++ b/generate/generate_test.go
@@ -1,6 +1,7 @@
 package generate
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -235,14 +236,15 @@ func TestGenerateWithConfig(t *testing.T) {
 	}
 }
 
-// TestGenerate is a snapshot-based test of error text.
+// TestGenerateErrors is a snapshot-based test of error text.
 //
-// For each .go or .graphql file in testdata/errors, and corresponding
-// .schema.graphql file, it asserts that the given query returns an error, and
-// that that error's string-text matches the snapshot.  The snapshotting is
-// useful to ensure we don't accidentally make the text less readable, drop the
-// line numbers, etc.  We include both .go and .graphql tests, to make sure the
-// line numbers work in both cases.
+// For each .go or .graphql file in testdata/errors, it asserts that the given
+// query returns an error, and that that error's string-text matches the
+// snapshot.  The snapshotting is useful to ensure we don't accidentally make
+// the text less readable, drop the line numbers, etc.  We include both .go and
+// .graphql tests for some of the test cases, to make sure the line numbers
+// work in both cases.  Tests may include a .schema.graphql file of their own,
+// or use the shared schema.graphql in the same directory for convenience.
 func TestGenerateErrors(t *testing.T) {
 	files, err := os.ReadDir(errorsDir)
 	if err != nil {
@@ -253,14 +255,25 @@ func TestGenerateErrors(t *testing.T) {
 		sourceFilename := file.Name()
 		if !strings.HasSuffix(sourceFilename, ".graphql") &&
 			!strings.HasSuffix(sourceFilename, ".go") ||
-			strings.HasSuffix(sourceFilename, ".schema.graphql") {
+			strings.HasSuffix(sourceFilename, ".schema.graphql") ||
+			sourceFilename == "schema.graphql" {
 			continue
 		}
 
 		baseFilename := strings.TrimSuffix(sourceFilename, filepath.Ext(sourceFilename))
-		schemaFilename := baseFilename + ".schema.graphql"
 		testFilename := strings.ReplaceAll(sourceFilename, ".", "/")
 
+		// Schema is either <base>.schema.graphql, or <dir>/schema.graphql if
+		// that doesn't exist.
+		schemaFilename := baseFilename + ".schema.graphql"
+		if _, err := os.Stat(filepath.Join(errorsDir, schemaFilename)); err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				schemaFilename = "schema.graphql"
+			} else {
+				t.Fatal(err)
+			}
+		}
+
 		t.Run(testFilename, func(t *testing.T) {
 			_, err := Generate(&Config{
 				Schema:      []string{filepath.Join(errorsDir, schemaFilename)},
diff --git a/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql b/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql
deleted file mode 100644
index 314f707..0000000
--- a/generate/testdata/errors/ConflictingDirectiveArguments.schema.graphql
+++ /dev/null
@@ -1,3 +0,0 @@
-type Query {
-  f: String
-}
diff --git a/generate/testdata/errors/ConflictingDirectives.schema.graphql b/generate/testdata/errors/ConflictingDirectives.schema.graphql
deleted file mode 100644
index 314f707..0000000
--- a/generate/testdata/errors/ConflictingDirectives.schema.graphql
+++ /dev/null
@@ -1,3 +0,0 @@
-type Query {
-  f: String
-}
diff --git a/generate/testdata/errors/ConflictingTypeNameAndForFieldBind.schema.graphql b/generate/testdata/errors/ConflictingTypeNameAndForFieldBind.schema.graphql
deleted file mode 100644
index ac8ac54..0000000
--- a/generate/testdata/errors/ConflictingTypeNameAndForFieldBind.schema.graphql
+++ /dev/null
@@ -1,8 +0,0 @@
-type Query {
-  user: User
-}
-
-type User {
-  id: ID!
-  name: String!
-}
diff --git a/generate/testdata/errors/ConflictingTypeNameAndLocalBind.schema.graphql b/generate/testdata/errors/ConflictingTypeNameAndLocalBind.schema.graphql
deleted file mode 100644
index ac8ac54..0000000
--- a/generate/testdata/errors/ConflictingTypeNameAndLocalBind.schema.graphql
+++ /dev/null
@@ -1,8 +0,0 @@
-type Query {
-  user: User
-}
-
-type User {
-  id: ID!
-  name: String!
-}
diff --git a/generate/testdata/errors/InvalidQuery.schema.graphql b/generate/testdata/errors/InvalidQuery.schema.graphql
deleted file mode 100644
index 314f707..0000000
--- a/generate/testdata/errors/InvalidQuery.schema.graphql
+++ /dev/null
@@ -1,3 +0,0 @@
-type Query {
-  f: String
-}
diff --git a/generate/testdata/errors/NoQuery.schema.graphql b/generate/testdata/errors/NoQuery.schema.graphql
deleted file mode 100644
index 75cab93..0000000
--- a/generate/testdata/errors/NoQuery.schema.graphql
+++ /dev/null
@@ -1,1 +0,0 @@
-type Query { f: String }
diff --git a/generate/testdata/errors/ConflictingTypeNameAndGlobalBind.schema.graphql b/generate/testdata/errors/schema.graphql
similarity index 88%
rename from generate/testdata/errors/ConflictingTypeNameAndGlobalBind.schema.graphql
rename to generate/testdata/errors/schema.graphql
index 42dfe46..9521976 100644
--- a/generate/testdata/errors/ConflictingTypeNameAndGlobalBind.schema.graphql
+++ b/generate/testdata/errors/schema.graphql
@@ -1,4 +1,5 @@
 type Query {
+  f: String
   user: User
 }
 
diff --git a/generate/testdata/snapshots/TestGenerateErrors-ConflictingTypeNameAndGlobalBind-graphql b/generate/testdata/snapshots/TestGenerateErrors-ConflictingTypeNameAndGlobalBind-graphql
index 164bc4c..433b617 100644
--- a/generate/testdata/snapshots/TestGenerateErrors-ConflictingTypeNameAndGlobalBind-graphql
+++ b/generate/testdata/snapshots/TestGenerateErrors-ConflictingTypeNameAndGlobalBind-graphql
@@ -1,1 +1,1 @@
-testdata/errors/ConflictingTypeNameAndGlobalBind.schema.graphql:8: typename option conflicts with global binding for ValidScalar; use `bind: "-"` to override it
+testdata/errors/ConflictingTypeNameAndGlobalBind.graphql:4: typename option conflicts with global binding for ValidScalar; use `bind: "-"` to override it