Skip to content

Commit 32f06e9

Browse files
adonovangopherbot
adonovan
authored and
gopherbot
committed
go/analysis/unitchecker: rewrite integration test
This CL rewrites the nasty integration test to make is cleaner. Instead of a data-driven test, it's a sequence of scripts using a set of higher-level operators. Also: - break dependency on packagetest; use txtar instead - scenarios are short and compact - it actually parses the JSON now. For golang/go#75432 Change-Id: I9b8f9ba27645a6827bb0b18f6a0daf8aeb2dce08 Reviewed-on: https://go-review.googlesource.com/c/tools/+/703357 Auto-Submit: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
1 parent be644c7 commit 32f06e9

File tree

1 file changed

+134
-108
lines changed

1 file changed

+134
-108
lines changed

‎go/analysis/unitchecker/unitchecker_test.go‎

Lines changed: 134 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
package unitchecker_test
66

77
import (
8+
"encoding/json"
89
"flag"
910
"fmt"
1011
"os"
1112
"os/exec"
12-
"regexp"
1313
"runtime"
1414
"strings"
1515
"testing"
@@ -18,7 +18,9 @@ import (
1818
"golang.org/x/tools/go/analysis/passes/findcall"
1919
"golang.org/x/tools/go/analysis/passes/printf"
2020
"golang.org/x/tools/go/analysis/unitchecker"
21-
"golang.org/x/tools/internal/packagestest"
21+
"golang.org/x/tools/internal/testenv"
22+
"golang.org/x/tools/internal/testfiles"
23+
"golang.org/x/tools/txtar"
2224
)
2325

2426
func TestMain(m *testing.M) {
@@ -52,24 +54,27 @@ func minivet() {
5254
// This is a very basic integration test of modular
5355
// analysis with facts using unitchecker under "go vet".
5456
// It fork/execs the main function above.
55-
func TestIntegration(t *testing.T) { packagestest.TestAll(t, testIntegration) }
56-
func testIntegration(t *testing.T, exporter packagestest.Exporter) {
57+
func TestIntegration(t *testing.T) {
5758
if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
5859
t.Skipf("skipping fork/exec test on this platform")
5960
}
6061

61-
exported := packagestest.Export(t, exporter, []packagestest.Module{{
62-
Name: "golang.org/fake",
63-
Files: map[string]any{
64-
"a/a.go": `package a
62+
const src = `
63+
-- go.mod --
64+
module golang.org/fake
65+
go 1.18
66+
67+
-- a/a.go --
68+
package a
6569
6670
func _() {
6771
MyFunc123()
6872
}
6973
7074
func MyFunc123() {}
71-
`,
72-
"b/b.go": `package b
75+
76+
-- b/b.go --
77+
package b
7378
7479
import "golang.org/fake/a"
7580
@@ -79,119 +84,140 @@ func _() {
7984
}
8085
8186
func MyFunc123() {}
82-
`,
83-
"c/c.go": `package c
87+
88+
-- c/c.go --
89+
package c
8490
8591
func _() {
8692
i := 5
8793
i = i
8894
}
89-
`,
90-
}}})
91-
defer exported.Cleanup()
92-
93-
const wantA = `
94-
.*a/a.go:4:11: call of MyFunc123\(...\)
95-
`
96-
const wantB = `
97-
.*b/b.go:6:13: call of MyFunc123\(...\)
98-
.*b/b.go:7:11: call of MyFunc123\(...\)
99-
`
100-
const wantC = `
101-
.*c/c.go:5:5: self-assignment of i
10295
`
103-
const wantAJSON = `
104-
\{
105-
"golang.org/fake/a": \{
106-
"findcall": \[
107-
\{
108-
"posn": ".*a/a.go:4:11",
109-
"message": "call of MyFunc123\(...\)",
110-
"suggested_fixes": \[
111-
\{
112-
"message": "Add '_TEST_'",
113-
"edits": \[
114-
\{
115-
"filename": ".*a/a.go",
116-
"start": 32,
117-
"end": 32,
118-
"new": "_TEST_"
119-
\}
120-
\]
121-
\}
122-
\]
123-
\}
124-
\]
125-
\}
126-
\}
127-
`
128-
const wantCJSON = `
129-
\{
130-
"golang.org/fake/c": \{
131-
"assign": \[
132-
\{
133-
"posn": ".*c/c.go:5:5",
134-
"message": "self-assignment of i",
135-
"suggested_fixes": \[
136-
\{
137-
"message": "Remove self-assignment",
138-
"edits": \[
139-
\{
140-
"filename": ".*c/c.go",
141-
"start": 37,
142-
"end": 42,
143-
"new": ""
144-
\}
145-
\]
146-
\}
147-
\]
148-
\}
149-
\]
150-
\}
151-
\}
152-
`
153-
for _, test := range []struct {
154-
args string
155-
wantOut string // multiline regular expression
156-
wantExitError bool
157-
}{
158-
{args: "golang.org/fake/a", wantOut: wantA, wantExitError: true},
159-
{args: "golang.org/fake/b", wantOut: wantB, wantExitError: true},
160-
{args: "golang.org/fake/c", wantOut: wantC, wantExitError: true},
161-
{args: "golang.org/fake/a golang.org/fake/b", wantOut: wantA + ".*" + wantB, wantExitError: true},
162-
{args: "-json golang.org/fake/a", wantOut: wantAJSON, wantExitError: false},
163-
{args: "-json golang.org/fake/c", wantOut: wantCJSON, wantExitError: false},
164-
{args: "-c=0 golang.org/fake/a", wantOut: wantA + "4 MyFunc123\\(\\)\n", wantExitError: true},
165-
} {
96+
// Expand archive into tmp tree.
97+
fs, err := txtar.FS(txtar.Parse([]byte(src)))
98+
if err != nil {
99+
t.Fatal(err)
100+
}
101+
tmpdir := testfiles.CopyToTmp(t, fs)
102+
103+
// -- operators --
104+
105+
// vet runs "go vet" with the specified arguments (plus -findcall.name=MyFunc123).
106+
vet := func(t *testing.T, args ...string) (exitcode int, stdout, stderr string) {
166107
cmd := exec.Command("go", "vet", "-vettool="+os.Args[0], "-findcall.name=MyFunc123")
167108
cmd.Stdout = new(strings.Builder)
168109
cmd.Stderr = new(strings.Builder)
169-
cmd.Args = append(cmd.Args, strings.Fields(test.args)...)
170-
cmd.Env = append(exported.Config.Env, "ENTRYPOINT=minivet")
171-
cmd.Dir = exported.Config.Dir
172-
173-
// TODO(golang/go#65729): Rework the test to
174-
// be specific about which output stream to match.
175-
err := cmd.Run()
176-
exitcode := 0
177-
if exitErr, ok := err.(*exec.ExitError); ok {
110+
cmd.Args = append(cmd.Args, args...)
111+
cmd.Env = append(os.Environ(), "ENTRYPOINT=minivet")
112+
cmd.Dir = tmpdir
113+
if err := cmd.Run(); err != nil {
114+
exitErr, ok := err.(*exec.ExitError)
115+
if !ok {
116+
t.Fatalf("couldn't exec %v: %v", cmd, err)
117+
}
178118
exitcode = exitErr.ExitCode()
179119
}
180-
if (exitcode != 0) != test.wantExitError {
181-
want := "zero"
182-
if test.wantExitError {
183-
want = "nonzero"
120+
121+
// Sanitize filenames; this is imperfect due to
122+
// (e.g.) /private/tmp -> /tmp symlink on macOS.
123+
stdout = strings.ReplaceAll(fmt.Sprint(cmd.Stdout), tmpdir, "TMPDIR")
124+
stderr = strings.ReplaceAll(fmt.Sprint(cmd.Stderr), tmpdir, "TMPDIR")
125+
126+
// Show vet information on failure.
127+
t.Cleanup(func() {
128+
if t.Failed() {
129+
t.Logf("command: %v", cmd)
130+
t.Logf("exit code: %d", exitcode)
131+
t.Logf("stdout: %s", stdout)
132+
t.Logf("stderr: %s", stderr)
184133
}
185-
t.Errorf("%s: got exit code %d, want %s", test.args, exitcode, want)
134+
})
135+
return
136+
}
137+
138+
// exitcode asserts that the exit code was "want".
139+
exitcode := func(t *testing.T, got, want int) {
140+
if got != want {
141+
t.Fatalf("vet tool exit code was %d", got)
186142
}
143+
}
187144

188-
out := fmt.Sprintf("stdout:\n%s\nstderr:\n%s\n", cmd.Stdout, cmd.Stderr)
189-
matched, err := regexp.MatchString("(?s)"+test.wantOut, out)
190-
if err != nil {
191-
t.Fatalf("regexp.Match(<<%s>>): %v", test.wantOut, err)
145+
// parseJSON parses the JSON diagnostics into a simple line-oriented form.
146+
parseJSON := func(t *testing.T, stdout string) string {
147+
var v map[string]map[string][]map[string]any
148+
if err := json.Unmarshal([]byte(stdout), &v); err != nil {
149+
t.Fatalf("invalid JSON: %v", err)
192150
}
193-
if !matched {
194-
t.Errorf("%s: got <<%s>>, want match of regexp <<%s>>", test.args, out, test.wantOut)
151+
var res strings.Builder
152+
for pkgpath, v := range v {
153+
for analyzer, v := range v {
154+
for _, v := range v {
155+
fmt.Fprintf(&res, "%s: [%s@%s] %v\n",
156+
v["posn"],
157+
analyzer, pkgpath,
158+
v["message"])
159+
}
160+
}
195161
}
162+
// Show parsed JSON information on failure.
163+
t.Cleanup(func() {
164+
if t.Failed() {
165+
t.Logf("json: %s", &res)
166+
}
167+
})
168+
return res.String()
196169
}
170+
171+
// substring asserts that the labeled output contained the substring.
172+
substring := func(t *testing.T, label, output, substr string) {
173+
if !strings.Contains(output, substr) {
174+
t.Fatalf("%s: expected substring %q", label, substr)
175+
}
176+
}
177+
178+
// -- scenarios --
179+
180+
t.Run("a", func(t *testing.T) {
181+
code, _, stderr := vet(t, "golang.org/fake/a")
182+
exitcode(t, code, 1)
183+
substring(t, "stderr", stderr, "a/a.go:4:11: call of MyFunc123")
184+
})
185+
t.Run("b", func(t *testing.T) {
186+
code, _, stderr := vet(t, "golang.org/fake/b")
187+
exitcode(t, code, 1)
188+
substring(t, "stderr", stderr, "b/b.go:6:13: call of MyFunc123")
189+
substring(t, "stderr", stderr, "b/b.go:7:11: call of MyFunc123")
190+
})
191+
t.Run("c", func(t *testing.T) {
192+
code, _, stderr := vet(t, "golang.org/fake/c")
193+
exitcode(t, code, 1)
194+
substring(t, "stderr", stderr, "c/c.go:5:5: self-assignment of i")
195+
})
196+
t.Run("ab", func(t *testing.T) {
197+
code, _, stderr := vet(t, "golang.org/fake/a", "golang.org/fake/b")
198+
exitcode(t, code, 1)
199+
substring(t, "stderr", stderr, "a/a.go:4:11: call of MyFunc123")
200+
substring(t, "stderr", stderr, "b/b.go:6:13: call of MyFunc123")
201+
substring(t, "stderr", stderr, "b/b.go:7:11: call of MyFunc123")
202+
})
203+
t.Run("a-json", func(t *testing.T) {
204+
code, stdout, _ := vet(t, "-json", "golang.org/fake/a")
205+
exitcode(t, code, 0)
206+
testenv.NeedsGo1Point(t, 26) // depends on CL 702815 (go vet -json => stdout)
207+
json := parseJSON(t, stdout)
208+
substring(t, "json", json, "a/a.go:4:11: [findcall@golang.org/fake/a] call of MyFunc123")
209+
})
210+
t.Run("c-json", func(t *testing.T) {
211+
code, stdout, _ := vet(t, "-json", "golang.org/fake/c")
212+
exitcode(t, code, 0)
213+
testenv.NeedsGo1Point(t, 26) // depends on CL 702815 (go vet -json => stdout)
214+
json := parseJSON(t, stdout)
215+
substring(t, "json", json, "c/c.go:5:5: [assign@golang.org/fake/c] self-assignment of i")
216+
})
217+
t.Run("a-context", func(t *testing.T) {
218+
code, _, stderr := vet(t, "-c=0", "golang.org/fake/a")
219+
exitcode(t, code, 1)
220+
substring(t, "stderr", stderr, "a/a.go:4:11: call of MyFunc123")
221+
substring(t, "stderr", stderr, "4 MyFunc123")
222+
})
197223
}

0 commit comments

Comments
 (0)