apidiff: add subtests

The Changes test used to read in a single file, testdata/tests.go, with all the test cases. Now support multiple testdata/*.go files, each running in its own subtest. This will help to isolate the tests. Make a single split as a demonstration. Change-Id: Ibccdca813f099e196be5e8381c2b3b905220ef56 Reviewed-on: https://go-review.googlesource.com/c/exp/+/512616 Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Jonathan Amsterdam <jba@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
golang · Jul 25, 2023 · e86e1b1 · e86e1b1
1 parent 5dae85e
commit e86e1b1
Show file tree

Hide file tree

Showing 4 changed files with 158 additions and 134 deletions.
diff --git a/apidiff/apidiff_test.go b/apidiff/apidiff_test.go
@@ -80,38 +80,47 @@ func testModuleChanges(t *testing.T, x packagestest.Exporter) {
 }
 
 func TestChanges(t *testing.T) {
-	dir := filepath.Join(t.TempDir(), "go")
-	wanti, wantc := splitIntoPackages(t, dir)
-	sort.Strings(wanti)
-	sort.Strings(wantc)
-
-	oldpkg, err := loadPackage(t, "apidiff/old", dir)
-	if err != nil {
-		t.Fatal(err)
-	}
-	newpkg, err := loadPackage(t, "apidiff/new", dir)
+	testfiles, err := filepath.Glob(filepath.Join("testdata", "*.go"))
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	report := Changes(oldpkg.Types, newpkg.Types)
-
-	got := report.messages(false)
-	if diff := cmp.Diff(wanti, got); diff != "" {
-		t.Errorf("incompatibles: mismatch (-want, +got)\n%s", diff)
-	}
-	got = report.messages(true)
-	if diff := cmp.Diff(wantc, got); diff != "" {
-		t.Errorf("compatibles: mismatch (-want, +got)\n%s", diff)
+	for _, testfile := range testfiles {
+		name := strings.TrimSuffix(filepath.Base(testfile), ".go")
+		t.Run(name, func(t *testing.T) {
+			dir := filepath.Join(t.TempDir(), "go")
+			wanti, wantc := splitIntoPackages(t, testfile, dir)
+			sort.Strings(wanti)
+			sort.Strings(wantc)
+
+			oldpkg, err := loadPackage(t, "apidiff/old", dir)
+			if err != nil {
+				t.Fatal(err)
+			}
+			newpkg, err := loadPackage(t, "apidiff/new", dir)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			report := Changes(oldpkg.Types, newpkg.Types)
+
+			got := report.messages(false)
+			if diff := cmp.Diff(wanti, got); diff != "" {
+				t.Errorf("incompatibles: mismatch (-want, +got)\n%s", diff)
+			}
+			got = report.messages(true)
+			if diff := cmp.Diff(wantc, got); diff != "" {
+				t.Errorf("compatibles: mismatch (-want, +got)\n%s", diff)
+			}
+		})
 	}
 }
 
-func splitIntoPackages(t *testing.T, dir string) (incompatibles, compatibles []string) {
+func splitIntoPackages(t *testing.T, file, dir string) (incompatibles, compatibles []string) {
 	// Read the input file line by line.
 	// Write a line into the old or new package,
 	// dependent on comments.
 	// Also collect expected messages.
-	f, err := os.Open("testdata/tests.go")
+	f, err := os.Open(file)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/apidiff/testdata/README.md b/apidiff/testdata/README.md
@@ -0,0 +1,12 @@
+The .go files in this directory are split into two packages, old and new.
+They are syntactically valid Go so that gofmt can process them.
+
+```
+If a comment begins with:  Then:
+old                        write subsequent lines to the "old" package
+new                        write subsequent lines to the "new" package
+both                       write subsequent lines to both packages
+c                          expect a compatible error with the following text
+i                          expect an incompatible error with the following text
+
+```
diff --git a/apidiff/testdata/basics.go b/apidiff/testdata/basics.go
@@ -0,0 +1,110 @@
+package p
+
+// Same type in both: OK.
+// both
+type A int
+
+// Changing the type is an incompatible change.
+// old
+type B int
+
+// new
+// i B: changed from int to string
+type B string
+
+// Adding a new type, whether alias or not, is a compatible change.
+// new
+// c AA: added
+type AA = A
+
+// c B1: added
+type B1 bool
+
+// Change of type for an unexported name doesn't matter...
+// old
+type t int
+
+// new
+type t string // OK: t isn't part of the API
+
+// ...unless it is exposed.
+// both
+var V2 u
+
+// old
+type u string
+
+// new
+// i u: changed from string to int
+type u int
+
+// An exposed, unexported type can be renamed.
+// both
+type u2 int
+
+// old
+type u1 int
+
+var V5 u1
+
+// new
+var V5 u2 // OK: V5 has changed type, but old u1 corresopnds to new u2
+
+// Splitting a single type into two is an incompatible change.
+// both
+type u3 int
+
+// old
+type (
+	Split1 = u1
+	Split2 = u1
+)
+
+// new
+type (
+	Split1 = u2 // OK, since old u1 corresponds to new u2
+
+	// This tries to make u1 correspond to u3
+	// i Split2: changed from u1 to u3
+	Split2 = u3
+)
+
+// Merging two types into one is OK.
+// old
+type (
+	GoodMerge1 = u2
+	GoodMerge2 = u3
+)
+
+// new
+type (
+	GoodMerge1 = u3
+	GoodMerge2 = u3
+)
+
+// Merging isn't OK here because a method is lost.
+// both
+type u4 int
+
+func (u4) M() {}
+
+// old
+type (
+	BadMerge1 = u3
+	BadMerge2 = u4
+)
+
+// new
+type (
+	BadMerge1 = u3
+	// i u4.M: removed
+	// What's really happening here is that old u4 corresponds to new u3,
+	// and new u3's method set is not a superset of old u4's.
+	BadMerge2 = u3
+)
+
+// old
+type Rem int
+
+// new
+// i Rem: removed
diff --git a/apidiff/testdata/tests.go b/apidiff/testdata/tests.go
@@ -1,127 +1,20 @@
-// This file is split into two packages, old and new.
-// It is syntactically valid Go so that gofmt can process it.
-//
-//	If a comment begins with:  Then:
-//	old                        write subsequent lines to the "old" package
-//	new                        write subsequent lines to the "new" package
-//	both                       write subsequent lines to both packages
-//	c                          expect a compatible error with the following text
-//	i                          expect an incompatible error with the following text
-package ignore
+package p
 
 // both
 import "io"
 
-//////////////// Basics
-
-// Same type in both: OK.
-// both
-type A int
-
-// Changing the type is an incompatible change.
-// old
-type B int
-
-// new
-// i B: changed from int to string
-type B string
-
-// Adding a new type, whether alias or not, is a compatible change.
-// new
-// c AA: added
-type AA = A
-
-// c B1: added
-type B1 bool
-
-// Change of type for an unexported name doesn't matter...
-// old
-type t int
-
-// new
-type t string // OK: t isn't part of the API
-
-// ...unless it is exposed.
-// both
-var V2 u
-
-// old
-type u string
-
-// new
-// i u: changed from string to int
-type u int
-
-// An exposed, unexported type can be renamed.
-// both
-type u2 int
-
 // old
 type u1 int
 
-var V5 u1
-
-// new
-var V5 u2 // OK: V5 has changed type, but old u1 corresopnds to new u2
-
-// Splitting a single type into two is an incompatible change.
 // both
-type u3 int
-
-// old
-type (
-	Split1 = u1
-	Split2 = u1
-)
-
-// new
-type (
-	Split1 = u2 // OK, since old u1 corresponds to new u2
-
-	// This tries to make u1 correspond to u3
-	// i Split2: changed from u1 to u3
-	Split2 = u3
-)
-
-// Merging two types into one is OK.
-// old
-type (
-	GoodMerge1 = u2
-	GoodMerge2 = u3
-)
-
-// new
-type (
-	GoodMerge1 = u3
-	GoodMerge2 = u3
-)
+type u2 int
 
-// Merging isn't OK here because a method is lost.
 // both
-type u4 int
-
-func (u4) M() {}
-
-// old
-type (
-	BadMerge1 = u3
-	BadMerge2 = u4
-)
-
-// new
-type (
-	BadMerge1 = u3
-	// i u4.M: removed
-	// What's really happening here is that old u4 corresponds to new u3,
-	// and new u3's method set is not a superset of old u4's.
-	BadMerge2 = u3
-)
-
-// old
-type Rem int
+type A int
 
 // new
-// i Rem: removed
+// c AA: added
+type AA = A
 
 //////////////// Constants