Repository: google/pprof
Pull request #798 (2023-08-16 16:33:20)

Introduce first test for browser-side behavior.
Added a test that uses chromedp to drive web page interactions and check the resulting display.

Caveat: just adding a single test initially (it checks sorting of the "Top" view table). Other tests will be added later.

Caveat: coverage of browser Javascript is not gathered, so we won't be able to rely on coverage to measure quality/changes in Javascript testing.

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/google/pprof/pull/798?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
> Merging [#798](https://app.codecov.io/gh/google/pprof/pull/798?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (9218c12) into [main](https://app.codecov.io/gh/google/pprof/commit/76846d4d6a3b69af9b9331821b7d40d2f5f4b58d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) (76846d4) will **decrease** coverage by `0.47%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main     #798      +/-   ##
==========================================
- Coverage   66.99%   66.52%   -0.47%     
==========================================
  Files          45       45              
  Lines        9861     9861              
==========================================
- Hits         6606     6560      -46     
- Misses       2795     2839      +44     
- Partials      460      462       +2     
```


[see 3 files with indirect coverage changes](https://app.codecov.io/gh/google/pprof/pull/798/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google)


aalexand: Will merge once tests complete.
ghemawat: I sync-ed with other changes, dropped the unnecessary action to setup chrome (it is already available by default in github runners), and dropped the "-v" from the "go test" invocation.

This should now be fine to merge.


Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 866c5ee..05569d7 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -130,6 +130,10 @@ jobs:
         with:
           path: ${{ env.WORKING_DIR }}
 
+      - name: Check chrome for browser tests
+        run: |
+          google-chrome --version
+
       - name: Fetch dependencies
         run: |
           sudo apt-get install graphviz
diff --git a/go.mod b/go.mod
index c358f0d..1a623f8 100644
--- a/go.mod
+++ b/go.mod
@@ -3,8 +3,18 @@ module github.com/google/pprof
 go 1.19
 
 require (
+	github.com/chromedp/chromedp v0.9.2
 	github.com/chzyer/readline v1.5.1
 	github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab
 )
 
-require golang.org/x/sys v0.1.0 // indirect
+require (
+	github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89 // indirect
+	github.com/chromedp/sysutil v1.0.0 // indirect
+	github.com/gobwas/httphead v0.1.0 // indirect
+	github.com/gobwas/pool v0.2.1 // indirect
+	github.com/gobwas/ws v1.2.1 // indirect
+	github.com/josharian/intern v1.0.0 // indirect
+	github.com/mailru/easyjson v0.7.7 // indirect
+	golang.org/x/sys v0.6.0 // indirect
+)
diff --git a/go.sum b/go.sum
index ee82676..5e6b8b7 100644
--- a/go.sum
+++ b/go.sum
@@ -1,11 +1,31 @@
+github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89 h1:aPflPkRFkVwbW6dmcVqfgwp1i+UWGFH6VgR1Jim5Ygc=
+github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs=
+github.com/chromedp/chromedp v0.9.2 h1:dKtNz4kApb06KuSXoTQIyUC2TrA0fhGDwNZf3bcgfKw=
+github.com/chromedp/chromedp v0.9.2/go.mod h1:LkSXJKONWTCHAfQasKFUZI+mxqS4tZqhmtGzzhLsnLs=
+github.com/chromedp/sysutil v1.0.0 h1:+ZxhTpfpZlmchB58ih/LBHX52ky7w2VhQVKQMucy3Ic=
+github.com/chromedp/sysutil v1.0.0/go.mod h1:kgWmDdq8fTzXYcKIBqIYvRRTnYb9aNS9moAV0xufSww=
 github.com/chzyer/logex v1.2.1 h1:XHDu3E6q+gdHgsdTPH6ImJMIp436vR6MPtH8gP05QzM=
 github.com/chzyer/logex v1.2.1/go.mod h1:JLbx6lG2kDbNRFnfkgvh4eRJRPX1QCoOIWomwysCBrQ=
 github.com/chzyer/readline v1.5.1 h1:upd/6fQk4src78LMRzh5vItIt361/o4uq553V8B5sGI=
 github.com/chzyer/readline v1.5.1/go.mod h1:Eh+b79XXUwfKfcPLepksvw2tcLE/Ct21YObkaSkeBlk=
 github.com/chzyer/test v1.0.0 h1:p3BQDXSxOhOG0P9z6/hGnII4LGiEPOYBhs8asl/fC04=
 github.com/chzyer/test v1.0.0/go.mod h1:2JlltgoNkt4TW/z9V/IzDdFaMTM2JPIi26O1pF38GC8=
+github.com/gobwas/httphead v0.1.0 h1:exrUm0f4YX0L7EBwZHuCF4GDp8aJfVeBrlLQrs6NqWU=
+github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM=
+github.com/gobwas/pool v0.2.1 h1:xfeeEhW7pwmX8nuLVlqbzVc7udMDrwetjEv+TZIz1og=
+github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
+github.com/gobwas/ws v1.2.1 h1:F2aeBZrm2NDsc7vbovKrWSogd4wvfAxg0FQ89/iqOTk=
+github.com/gobwas/ws v1.2.1/go.mod h1:hRKAFb8wOxFROYNsT1bqfWnhX+b5MFeJM9r2ZSwg/KY=
 github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab h1:BA4a7pe6ZTd9F8kXETBoijjFJ/ntaa//1wiH9BZu4zU=
 github.com/ianlancetaylor/demangle v0.0.0-20230524184225-eabc099b10ab/go.mod h1:gx7rwoVhcfuVKG5uya9Hs3Sxj7EIvldVofAWIUtGouw=
+github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY=
+github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y=
+github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80 h1:6Yzfa6GP0rIo/kULo2bwGEkFvCePZ3qHDDTC3/J9Swo=
+github.com/ledongthuc/pdf v0.0.0-20220302134840-0c2507a12d80/go.mod h1:imJHygn/1yfhB7XSJJKlFZKl/J+dCPAknuiaGOshXAs=
+github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
+github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
+github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde h1:x0TT0RDC7UhAVbbWWBzr41ElhJx5tXPWkIHA2HWPRuw=
+github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
 golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
-golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ=
+golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=


Test patch:
diff --git a/internal/driver/browser_test.go b/internal/driver/browser_test.go
new file mode 100644
index 0000000..dc5c0dd
--- /dev/null
+++ b/internal/driver/browser_test.go
@@ -0,0 +1,121 @@
+// Copyright 2023 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package driver
+
+import (
+	"context"
+	"fmt"
+	"os/exec"
+	"regexp"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/chromedp/chromedp"
+)
+
+func maybeSkipBrowserTest(t *testing.T) {
+	// Limit to just Linux for now since this is expensive and the
+	// browser interactions should be platform agnostic.  If we ever
+	// see a benefit from wider testing, we can relax this.
+	if runtime.GOOS != "linux" || runtime.GOARCH != "amd64" {
+		t.Skip("This test only works on x86-64 Linux")
+	}
+
+	// Check that browser is available.
+	if _, err := exec.LookPath("google-chrome"); err == nil {
+		return
+	}
+	if _, err := exec.LookPath("chrome"); err == nil {
+		return
+	}
+	t.Skip("chrome not available")
+}
+
+// browserDeadline is the deadline to use for browser tests. This is long to
+// reduce flakiness in CI workflows.
+const browserDeadline = time.Second * 60
+
+func TestTopTable(t *testing.T) {
+	maybeSkipBrowserTest(t)
+
+	prof := makeFakeProfile()
+	server := makeTestServer(t, prof)
+	ctx, cancel := context.WithTimeout(context.Background(), browserDeadline)
+	defer cancel()
+	ctx, cancel = chromedp.NewContext(ctx)
+	defer cancel()
+
+	err := chromedp.Run(ctx,
+		chromedp.Navigate(server.URL+"/top"),
+		chromedp.WaitVisible(`#toptable`, chromedp.ByID),
+
+		// Check that fake profile entries show up in the right order.
+		matchRegexp(t, "#node0", `200ms.*F2`),
+		matchInOrder(t, "#toptable", "F2", "F3", "F1"),
+
+		// Check sorting by cumulative count.
+		chromedp.Click(`#cumhdr1`, chromedp.ByID),
+		matchInOrder(t, "#toptable", "F1", "F2", "F3"),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+// matchRegexp is a chromedp.Action that fetches the text of the first
+// node that matched query and checks that the text matches regexp re.
+func matchRegexp(t *testing.T, query, re string) chromedp.ActionFunc {
+	return func(ctx context.Context) error {
+		var value string
+		err := chromedp.Text(query, &value, chromedp.ByQuery).Do(ctx)
+		if err != nil {
+			return fmt.Errorf("text %s: %v", query, err)
+		}
+		t.Logf("text %s:\n%s", query, value)
+		m, err := regexp.MatchString(re, value)
+		if err != nil {
+			return err
+		}
+		if !m {
+			return fmt.Errorf("%s: did not find %q in\n%s", query, re, value)
+		}
+		return nil
+	}
+}
+
+// matchInOrder is a chromedp.Action that fetches the text of the first
+// node that matched query and checks that the supplied sequence of
+// strings occur in order in the text.
+func matchInOrder(t *testing.T, query string, sequence ...string) chromedp.ActionFunc {
+	return func(ctx context.Context) error {
+		var value string
+		err := chromedp.Text(query, &value, chromedp.ByQuery).Do(ctx)
+		if err != nil {
+			return fmt.Errorf("text %s: %v", query, err)
+		}
+		t.Logf("text %s:\n%s", query, value)
+		remaining := value
+		for _, s := range sequence {
+			pos := strings.Index(remaining, s)
+			if pos < 0 {
+				return fmt.Errorf("%s: did not find %q in expected order %v  in\n%s", query, s, sequence, value)
+			}
+			remaining = remaining[pos+len(s):]
+		}
+		return nil
+	}
+}
diff --git a/test.sh b/test.sh
index e5f8802..53cff73 100755
--- a/test.sh
+++ b/test.sh
@@ -27,7 +27,7 @@ fi
 # Packages that have any tests.
 PKG=$(go list -f '{{if .TestGoFiles}} {{.ImportPath}} {{end}}' ./...)
 
-go test -v $PKG
+go test $PKG
 
 for d in $PKG; do
   go test -race -coverprofile=profile.out -covermode=$MODE $d