cmd/screentest: doc headless-shell; improve output

- Document the preference for headless-shell, the stripped-down
  Chrome binary. It is more reliable.

- Clarify output, reduce vertical whitespace.

- Fix race condition.

Change-Id: I8d879bf4442a089e1b3eeb47074c76fe46e87a53
Reviewed-on: https://go-review.googlesource.com/c/website/+/628056
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: jba

cmd/golangorg/README.md

@@ -31,8 +31,11 @@ The go.dev web site has a suite of visual checks that can be run with:
 	./cmd/golangorg/screentest.sh
 
 These checks can be run locally and will generate visual diffs of web pages
-from the set of testcases in `cmd/golangorg/testdata/screentest/*.txt`, comparing screenshots
+from the set of test cases in `cmd/golangorg/testdata/screentest/*.txt`, comparing screenshots
 of the live server and a locally running instance of cmd/golangorg.
+Screentest will start Chrome locally to render the pages, but that can be unreliable.
+Prefer Chrome headless-shell. See the documentation at the top of cmd/screenshot/main.go
+for more.
 
 ## Deploying to go.dev and golang.org
 

cmd/golangorg/testdata/screentest/godev.txt

@@ -2,11 +2,16 @@ windowsize 1536x960
 
 test homepage
 path /
+# Wait for the playground to run the sample program.
+sleep 4s
 capture fullscreen
 capture fullscreen 540x1080
 
 test why go case studies
 path /solutions/case-studies
+# Scrolling to bottom causes lazy-loading images to load.
+eval window.scrollTo({top: document.body.scrollHeight});
+sleep 1s
 capture fullscreen
 capture fullscreen 540x1080
 
@@ -15,10 +20,14 @@ path /solutions/use-cases
 capture fullscreen
 capture fullscreen 540x1080
 
-test getting started
-path /learn/
-capture fullscreen
-capture fullscreen 540x1080
+# This test will fail because the local server
+# uses fake download information, so the download button
+# will have a different Go version.
+#
+# test getting started
+# path /learn/
+# capture fullscreen
+# capture fullscreen 540x1080
 
 test docs
 path /doc/

cmd/screentest/main.go

@@ -19,27 +19,45 @@ can be slash-separated file paths (even on Windows).
 
 The flags are:
 
-	 -c
-		  Number of test cases to run concurrently.
+	-c
+	  Number of test cases to run concurrently.
 	-d
-		  URL of a Chrome websocket debugger. If omitted, screentest tries to find the
-		  Chrome executable on the system and starts a new instance.
+	  URL of a Chrome websocket debugger. If omitted, screentest uses the
+	  Chrome executable on the command path. It will look first for the
+	  headless-shell binary, which is preferred.
 	-headers
-		  HTTP(S) headers to send with each request, as a comma-separated list of name:value.
+	  HTTP(S) headers to send with each request, as a comma-separated list of name:value.
 	-run REGEXP
-		  Run only tests matching regexp.
-	 -o
-		  URL or slash-separated path for output files. If omitted, files are written
-		  to a subdirectory of the user's cache directory. Each test file is given
-		  its own directory, so test names in two files can be identical. But the directory
-		  name is the basename of the test file with the extension removed. Conflicting
-		  file names will overwrite each other.
-	 -u
-		  Instead of comparing screenshots, use the test screenshots to update the
-		  want screenshots. This only makes sense if wantURL is a storage location
-		  like a file path or GCS bucket.
-	 -v
-		  Variables provided to script templates as comma separated KEY:VALUE pairs.
+	  Run only tests matching regexp.
+	-o
+	  URL or slash-separated path where output files for failing tests are written.
+	  If omitted, files are written to a subdirectory of the user's cache directory.
+	  At the start of each run, existing files are removed.
+	  Each test file is given its own directory, so test names in two files can be identical,
+	  but the directory name is the basename of the test file with the extension removed, so
+	  files with identical basenames will overwrite each other.
+	-u
+	  Instead of comparing screenshots, use the test screenshots to update the
+	  want screenshots. This only makes sense if wantURL is a storage location
+	  like a file path or GCS bucket.
+	-v
+	  Variables provided to script templates as comma-separated KEY:VALUE pairs.
+
+# Headless Chrome
+
+Screentest needs a headless Chrome process to render web pages. Although it can use a full
+Chrome browser, we have found the headless-shell build of Chrome to be more reliable.
+Install headless-shell on your local machine with this command:
+
+	npx @puppeteer/browsers install chrome-headless-shell@VERSION
+
+Put the binary on your path and screentest will find it. Omit the -d flag in this case.
+
+You can also run headless-shell in docker. We use this command:
+
+	docker run --detach --rm --network host --shm-size 8G --name headless-shell chromedp/headless-shell:VERSION
+
+Then pass "-d ws://localhost:9222" to screentest.
 
 # Scripts
 
@@ -105,6 +123,11 @@ some other way.
 	eval 'document.querySelector(".selector").remove();'
 	eval 'window.scrollTo({top: 0});'
 
+Use sleep DURATION to pause the browser for the duration. This is a last resort
+for deflaking; prefer to wait for an element.
+
+	sleep 50ms
+
 Use capture [SIZE] [ARG] to create a test case with the properties
 defined in the test case. If present, the first argument to capture must be one of
 'fullscreen', 'viewport' or 'element'. The optional second argument provides
@@ -169,6 +192,8 @@ type options struct {
 }
 
 func main() {
+	log.SetFlags(0)
+	log.SetPrefix("screentest: ")
 	flag.Usage = func() {
 		fmt.Printf("usage: screentest [flags] testURL wantURL path ...\n")
 		fmt.Printf("\ttestURL is the URL or file path to be tested\n")
@@ -184,5 +209,7 @@ func main() {
 	}
 	if err := run(context.Background(), flag.Arg(0), flag.Arg(1), flag.Args()[2:], flags); err != nil {
 		log.Fatal(err)
+	} else {
+		log.Print("PASS")
 	}
 }