cmd/watchflakes: coalesce a package-level failure to a test failure in the same package

Recently we added reporting package-level failures (that is not
associated to a specific test) to LUCI. In some cases, e.g. a test
timeout could also cause a package-level failure. E.g.
https://ci.chromium.org/b/8745232168515795905 . Currently
watchflakes reports them as two issues. But they are the same
cause. This CL changes it to combine them, when there is a
package-level failure and also a test failure in the same package.

Also update splitTestID to handle package-level failure. If it
cannot split, treat the whole TestID as package, not the test.

Fixes golang/go#68597.

Change-Id: I0679a0345194589e84b351ea7519b2d4da2731e3
Reviewed-on: https://go-review.googlesource.com/c/build/+/601436
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>

Author: cherrymui

cmd/watchflakes/luci.go

@@ -538,7 +538,8 @@ func splitTestID(testid string) (string, string) {
 			return pkg, sep[1:] + test // add back "Test" prefix (without ".")
 		}
 	}
-	return "", testid
+	// Maybe a package-level target (e.g. build failure).
+	return testid, ""
 }
 
 func buildURL(buildID int64) string { // keep in sync with buildUrlRE in github.go

cmd/watchflakes/main.go

@@ -597,13 +597,20 @@ func snippet(log string) string {
 func coalesceFailures(fs []*Failure) []*Failure {
 	var res []*Failure
 	// A subtest fail may cause the parent test to fail, combine them.
+	// So is a test failure causing package-level failure.
 	var cur *Failure
 	for _, f := range fs {
-		if cur != nil && strings.HasPrefix(f.TestID, cur.TestID+"/") {
-			// f is a subtest of cur. Consume cur, replace with f.
-			res[len(res)-1] = f
-			cur = f
-			continue
+		if cur != nil {
+			cpkg, ctst := splitTestID(cur.TestID)
+			if (ctst == "" && strings.HasPrefix(f.TestID, cpkg+".")) ||
+				strings.HasPrefix(f.TestID, cur.TestID+"/") {
+				// 1. cur is a package-level failure, and f is a test in that package
+				// 2. f is a subtest of cur.
+				// In either case, consume cur, replace with f.
+				res[len(res)-1] = f
+				cur = f
+				continue
+			}
 		}
 		cur = f
 		res = append(res, f)
@@ -619,10 +626,21 @@ func coalesceFailures(fs []*Failure) []*Failure {
 		return strings.Contains(f.LogText, "--- FAIL") &&
 			(!strings.Contains(last.LogText, "--- FAIL") || len(f.LogText) > len(last.LogText))
 	}
+	siblingSubtests := func(f, last *Failure) bool {
+		pkg, tst := splitTestID(f.TestID)
+		pkg2, tst2 := splitTestID(last.TestID)
+		if pkg != pkg2 {
+			return false
+		}
+		par, _, ok := strings.Cut(tst, "/")
+		par2, _, ok2 := strings.Cut(tst2, "/")
+		return ok && ok2 && par == par2
+	}
 	cur = nil
+	fs = res
 	res = fs[:0]
 	for _, f := range fs {
-		if cur != nil && strings.HasPrefix(f.TestID, cur.TestID+"/") {
+		if cur != nil && siblingSubtests(f, cur) {
 			if moreLikelyUseful(f, res[len(res)-1]) {
 				res[len(res)-1] = f
 			}