From 05d007b96d0b65d7498a3a9413d9a23212346d68 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Fri, 3 Jan 2025 23:48:06 -0500 Subject: [PATCH] cmd/watchflakes: add some script parsing tests Watchflakes so far had no tests at all. As part of fixing a bug in the script parser, add some. The tests were based on test coverage present in the go/build/constraint package (which parses a similar language). The tests are certainly not complete, but enough for the current needs of the bug being fixed. The high-level script parsing tests could also work well in the script package itself, but I moved them to the watchflakes package to make it easier to reuse the watchflakes-specific fields variable. For golang/go#71119. Change-Id: I86e15613b740fe330766fa27d70477034af1cf3b Reviewed-on: https://go-review.googlesource.com/c/build/+/640295 LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov Reviewed-by: Cherry Mui Auto-Submit: Dmitri Shuralyov --- cmd/watchflakes/github.go | 2 - .../internal/script/script_test.go | 70 +++++++++ cmd/watchflakes/script_test.go | 134 ++++++++++++++++++ 3 files changed, 204 insertions(+), 2 deletions(-) create mode 100644 cmd/watchflakes/internal/script/script_test.go create mode 100644 cmd/watchflakes/script_test.go diff --git a/cmd/watchflakes/github.go b/cmd/watchflakes/github.go index 3314ebe6de..5ef743cebb 100644 --- a/cmd/watchflakes/github.go +++ b/cmd/watchflakes/github.go @@ -42,8 +42,6 @@ var ( testFlakes *github.Project ) -var scriptRE = regexp.MustCompile(`(?m)(^( {4}|\t)#!watchflakes\n((( {4}|\t).*)?\n)+|^\x60{3}\n#!watchflakes\n(([^\x60].*)?\n)+\x60{3}\n)`) - // readIssues reads the GitHub issues in the Test Flakes project. // It also sets up the repo, labels, and testFlakes variables for // use by other functions below. diff --git a/cmd/watchflakes/internal/script/script_test.go b/cmd/watchflakes/internal/script/script_test.go new file mode 100644 index 0000000000..073de57c51 --- /dev/null +++ b/cmd/watchflakes/internal/script/script_test.go @@ -0,0 +1,70 @@ +// Copyright 2025 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. + +package script + +import ( + "fmt" + "testing" +) + +var lexTests = [...]struct { + in string + out string +}{ + {"", ""}, + {"x", "a"}, + {"x.y", "a err: :1.2: invalid syntax at '.' (U+002e)"}, + {"x_y", "a"}, + {"αx", "err: :1.1: invalid syntax at 'α' (U+03b1)"}, + {"x y", "a a"}, + {"x!y", "a ! a"}, + {"&&||!()xy yx ", "&& || ! ( ) a a"}, + {"x~", "a ~"}, + {"x ~", "a ~"}, + {"x &", "a err: :1.3: invalid syntax at &"}, + {"x &y", "a err: :1.3: invalid syntax at &"}, + {"output !~ `content`", "a ! ~ `"}, +} + +func TestLex(t *testing.T) { + for i, tt := range lexTests { + t.Run(fmt.Sprint(i), func(t *testing.T) { + p := &parser{s: tt.in} + out := "" + for { + tok, err := lex(p) + if tok == "" && err == nil { + break + } + if out != "" { + out += " " + } + if err != nil { + out += "err: " + err.Error() + break + } + out += tok + } + if out != tt.out { + t.Errorf("lex(%q):\nhave %s\nwant %s", tt.in, out, tt.out) + } + }) + } +} + +func lex(p *parser) (tok string, err error) { + defer func() { + if e := recover(); e != nil { + if e, ok := e.(*SyntaxError); ok { + err = e + return + } + panic(e) + } + }() + + p.lex() + return p.tok, nil +} diff --git a/cmd/watchflakes/script_test.go b/cmd/watchflakes/script_test.go new file mode 100644 index 0000000000..deb5d8d2b9 --- /dev/null +++ b/cmd/watchflakes/script_test.go @@ -0,0 +1,134 @@ +// Copyright 2025 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. + +package main + +import ( + "fmt" + "regexp" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "golang.org/x/build/cmd/watchflakes/internal/script" +) + +var scriptTests = [...]struct { + in string + out []*script.Rule + err string +}{ + { + "post <- pkg == \"cmd/go\" && test == \"\" && `unexpected files left in tmpdir`", + []*script.Rule{{ + Action: "post", + Pattern: &script.AndExpr{ + X: &script.AndExpr{ + X: &script.CmpExpr{Field: "pkg", Op: "==", Literal: "cmd/go"}, + Y: &script.CmpExpr{Field: "test", Op: "==", Literal: ""}, + }, + Y: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)unexpected files left in tmpdir`)}, + }, + }}, + "", + }, + { + "post <- goos == \"openbsd\" && `unlinkat .*: operation not permitted`", + []*script.Rule{{ + Action: "post", + Pattern: &script.AndExpr{ + X: &script.CmpExpr{Field: "goos", Op: "==", Literal: "openbsd"}, + Y: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)unlinkat .*: operation not permitted`)}, + }, + }}, + "", + }, + { + "post <- pkg ~ `^cmd/go` && `appspot.com.*: 503`", + []*script.Rule{{ + Action: "post", + Pattern: &script.AndExpr{ + X: &script.RegExpr{Field: "pkg", Not: false, Regexp: regexp.MustCompile(`(?m)^cmd/go`)}, + Y: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)appspot.com.*: 503`)}, + }, + }}, + "", + }, + { + `post <- goos == "windows" && + (` + "`dnsquery: DNS server failure` || `getaddrinfow: This is usually a temporary error`)", + []*script.Rule{{ + Action: "post", + Pattern: &script.AndExpr{ + X: &script.CmpExpr{Field: "goos", Op: "==", Literal: "windows"}, + Y: &script.OrExpr{ + X: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)dnsquery: DNS server failure`)}, + Y: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)getaddrinfow: This is usually a temporary error`)}, + }, + }, + }}, + "", + }, + { + `post <- builder == "darwin-arm64-12" && pkg == "" && test == ""`, + []*script.Rule{{ + Action: "post", + Pattern: &script.AndExpr{ + X: &script.AndExpr{ + X: &script.CmpExpr{Field: "builder", Op: "==", Literal: "darwin-arm64-12"}, + Y: &script.CmpExpr{Field: "pkg", Op: "==", Literal: ""}, + }, + Y: &script.CmpExpr{Field: "test", Op: "==", Literal: ""}, + }, + }}, + "", + }, + { + `# note: sometimes the URL is printed with one / + default <- ` + "`" + `(Get|read) "https://?(goproxy.io|proxy.golang.com.cn|goproxy.cn)` + "`", + []*script.Rule{{ + Action: "default", + Pattern: &script.RegExpr{Field: "", Not: false, Regexp: regexp.MustCompile(`(?m)(Get|read) "https://?(goproxy.io|proxy.golang.com.cn|goproxy.cn)`)}, + }}, + "", + }, + { + `default <- pkg == "cmd/go" && test == "TestScript" && + output !~ ` + "`" + `The process cannot access the file because it is being used by another process.` + "`" + ` # tracked in go.dev/issue/71112`, + nil, + "script:2.22: unexpected !", + }, + { + `post <- pkg ~ "^cmd/go"`, + nil, + "script:1.15: ~ requires backquoted regexp", + }, +} + +func TestParseScript(t *testing.T) { + for i, tt := range scriptTests { + t.Run(fmt.Sprint(i), func(t *testing.T) { + s, err := script.Parse("script", tt.in, fields) + if err != nil { + if tt.err == "" { + t.Errorf("Parse(%q): unexpected error: %v", tt.in, err) + } else if !strings.Contains(fmt.Sprint(err), tt.err) { + t.Errorf("Parse(%q): error %v, want %v", tt.in, err, tt.err) + } + return + } + if tt.err != "" { + t.Errorf("Parse(%q) = %v, want error %v", tt.in, s, tt.err) + return + } + want := &script.Script{ + File: "script", + Rules: tt.out, + } + if diff := cmp.Diff(want, s, cmp.Comparer(func(x, y *regexp.Regexp) bool { return x.String() == y.String() })); diff != "" { + t.Errorf("Parse(%q) mismatch (-want +got):\n%s", tt.in, diff) + } + }) + } +}