From a1e4657d5a89d683a2bc0c5c8d6c5d2698bb081a Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Mon, 29 Mar 2021 12:33:45 -0700 Subject: [PATCH] cmd/compile/internal/types2: review of check_test.go The changes between (equivalent, and reviewed) go/types/check_test.go and check_test.go can be seen by comparing patchset 1 and 2. The actual changes are removing the "// UNREVIEWED" marker, and minor adjustments to get the code slightly closer to go/types/check_test.go. The primary differences compared to go/types are: - use of syntax rather than go/ast package - re-implemented mechanism for error matching and elimination based on the syntax.ErrorMap mechanism (there's no exported access to the syntax scanner) - error matching permits for column tolerances because types2 column information doesn't match go/types column information Change-Id: I8ae6bc93dfa2b517673b642064a1f09166755286 Reviewed-on: https://go-review.googlesource.com/c/go/+/305573 Trust: Robert Griesemer Run-TryBot: Robert Griesemer TryBot-Result: Go Bot Reviewed-by: Robert Findley --- src/cmd/compile/internal/types2/check_test.go | 62 ++++++++----------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go index fc6f46b4b8..ac21c3458e 100644 --- a/src/cmd/compile/internal/types2/check_test.go +++ b/src/cmd/compile/internal/types2/check_test.go @@ -1,4 +1,3 @@ -// UNREVIEWED // Copyright 2011 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. @@ -29,9 +28,7 @@ package types2_test import ( "cmd/compile/internal/syntax" "flag" - "fmt" "internal/testenv" - "io/ioutil" "os" "path/filepath" "regexp" @@ -73,6 +70,7 @@ func unpackError(err error) syntax.Error { } } +// delta returns the absolute difference between x and y. func delta(x, y uint) uint { switch { case x < y: @@ -98,17 +96,17 @@ func asGoVersion(s string) string { return "" } -func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, trace bool) { - if len(sources) == 0 { +func checkFiles(t *testing.T, filenames []string, goVersion string, colDelta uint, trace bool) { + if len(filenames) == 0 { t.Fatal("no source files") } var mode syntax.Mode - if strings.HasSuffix(sources[0], ".go2") { + if strings.HasSuffix(filenames[0], ".go2") { mode |= syntax.AllowGenerics } // parse files and collect parser errors - files, errlist := parseFiles(t, sources, mode) + files, errlist := parseFiles(t, filenames, mode) pkgName := "" if len(files) > 0 { @@ -133,7 +131,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, conf.AcceptMethodTypeParams = true conf.InferFromConstraints = true // special case for importC.src - if len(sources) == 1 && strings.HasSuffix(sources[0], "importC.src") { + if len(filenames) == 1 && strings.HasSuffix(filenames[0], "importC.src") { conf.FakeImportC = true } conf.Trace = trace @@ -156,7 +154,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, // collect expected errors errmap := make(map[string]map[uint][]syntax.Error) - for _, filename := range sources { + for _, filename := range filenames { f, err := os.Open(filename) if err != nil { t.Error(err) @@ -169,16 +167,16 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, } // match against found errors + // TODO(gri) sort err list to avoid mismatched when having multiple errors for _, err := range errlist { got := unpackError(err) // find list of errors for the respective error line filename := got.Pos.Base().Filename() filemap := errmap[filename] - var line uint + line := got.Pos.Line() var list []syntax.Error if filemap != nil { - line = got.Pos.Line() list = filemap[line] } // list may be nil @@ -210,6 +208,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, // eliminate from list if n := len(list) - 1; n > 0 { // not the last entry - swap in last element and shorten list by 1 + // TODO(gri) avoid changing the order of entries list[index] = list[n] filemap[line] = list[:n] } else { @@ -247,48 +246,41 @@ func TestCheck(t *testing.T) { checkFiles(t, strings.Split(*testFiles, ","), *goVersion, 0, testing.Verbose()) } -func TestTestdata(t *testing.T) { DefPredeclaredTestFuncs(); testDir(t, 75, "testdata") } // TODO(gri) narrow column tolerance -func TestExamples(t *testing.T) { testDir(t, 0, "examples") } -func TestFixedbugs(t *testing.T) { testDir(t, 0, "fixedbugs") } +// TODO(gri) go/types has an extra TestLongConstants test -func testDir(t *testing.T, colDelta uint, dir string) { +func TestTestdata(t *testing.T) { DefPredeclaredTestFuncs(); testDir(t, "testdata", 75) } // TODO(gri) narrow column tolerance +func TestExamples(t *testing.T) { testDir(t, "examples", 0) } +func TestFixedbugs(t *testing.T) { testDir(t, "fixedbugs", 0) } + +func testDir(t *testing.T, dir string, colDelta uint) { testenv.MustHaveGoBuild(t) - fis, err := ioutil.ReadDir(dir) + fis, err := os.ReadDir(dir) if err != nil { t.Error(err) return } - for count, fi := range fis { + for _, fi := range fis { path := filepath.Join(dir, fi.Name()) // if fi is a directory, its files make up a single package + var filenames []string if fi.IsDir() { - if testing.Verbose() { - fmt.Printf("%3d %s\n", count, path) - } - fis, err := ioutil.ReadDir(path) + fis, err := os.ReadDir(path) if err != nil { t.Error(err) continue } - files := make([]string, len(fis)) - for i, fi := range fis { - // if fi is a directory, checkFiles below will complain - files[i] = filepath.Join(path, fi.Name()) - if testing.Verbose() { - fmt.Printf("\t%s\n", files[i]) - } + for _, fi := range fis { + filenames = append(filenames, filepath.Join(path, fi.Name())) } - checkFiles(t, files, "", colDelta, false) - continue + } else { + filenames = []string{path} } - // otherwise, fi is a stand-alone file - if testing.Verbose() { - fmt.Printf("%3d %s\n", count, path) - } - checkFiles(t, []string{path}, "", colDelta, false) + t.Run(filepath.Base(path), func(t *testing.T) { + checkFiles(t, filenames, *goVersion, colDelta, false) + }) } }