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>
diff --git a/apidiff/apidiff_test.go b/apidiff/apidiff_test.go index b439f4e..56a1fab 100644 --- a/apidiff/apidiff_test.go +++ b/apidiff/apidiff_test.go
@@ -80,38 +80,47 @@ } 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) + testfiles, err := filepath.Glob(filepath.Join("testdata", "*.go")) if err != nil { t.Fatal(err) } - newpkg, err := loadPackage(t, "apidiff/new", dir) - if err != nil { - t.Fatal(err) - } + 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) - report := Changes(oldpkg.Types, newpkg.Types) + 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) + } - 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) + 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 new file mode 100644 index 0000000..8029a13 --- /dev/null +++ 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 new file mode 100644 index 0000000..94c0940 --- /dev/null +++ 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 index edee045..655239a 100644 --- 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 +type u2 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 -) +type A int // 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 +// c AA: added +type AA = A //////////////// Constants