From 49df0bb00fe50fcc8fe4ba4c4e716cfc19ed2a0b Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Mon, 25 Nov 2024 14:35:44 -0500 Subject: cmd/golangorg/testdatascreentest: fix learn test Remove the Go version HTML from the screenshot so the test will pass. The version mismatch is an artifact of the test setup. Also, have screentest warn about using eval with quoted expressions. I was doing this and couldn't understand why my fix to the learn test wasn't working. Lastly, describe how we could add a feature that shows the result of evaluating each expression. My buggy version of this feature helped me diagnose the above problem, but a proper implementation is harder and not worth it now. Change-Id: I411717cd2aafab97193082a392bcd792bd3c7534 Reviewed-on: https://go-review.googlesource.com/c/website/+/631715 Reviewed-by: Hyang-Ah Hana Kim LUCI-TryBot-Result: Go LUCI --- cmd/golangorg/testdata/screentest/godev.txt | 15 +++++++-------- cmd/screentest/screentest.go | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) (limited to 'cmd') diff --git a/cmd/golangorg/testdata/screentest/godev.txt b/cmd/golangorg/testdata/screentest/godev.txt index ddba5efb..28fad371 100644 --- a/cmd/golangorg/testdata/screentest/godev.txt +++ b/cmd/golangorg/testdata/screentest/godev.txt @@ -20,14 +20,13 @@ path /solutions/use-cases 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 getting started +path /learn/ +# The local server uses fake download information, so the download button +# will have a different Go version. Remove it. +eval document.querySelector("div.js-latestGoVersion").remove(); +capture fullscreen +capture fullscreen 540x1080 test docs path /doc/ diff --git a/cmd/screentest/screentest.go b/cmd/screentest/screentest.go index 59902e1b..04483684 100644 --- a/cmd/screentest/screentest.go +++ b/cmd/screentest/screentest.go @@ -4,6 +4,13 @@ // TODO(jba): remove ints function in template (see cmd/golangorg/testdata/screentest/relnotes.txt) +// TODO(jba): Provide a way to capture the results of an eval directive. +// If the second argument to chromedp.Evaluate is a *[]byte, the result will be written +// there. The problem is that we may screenshot twice, and currently the same list of +// tasks is used for both, and what's more the evaluations happen concurrently (see testcase.run). +// We need two locations for Evaluate, not one. Probably the simplest thing would be to build +// two copies of the tasks slice. But that is a lot of complexity for this one debugging feature. + package main import ( @@ -414,6 +421,18 @@ func readTests(file, testURL, wantURL string, common common) (_ []*testcase, err if test == nil { return nil, errors.New("directive must be in a test") } + // Warn about a quoted argument to eval. + // The quotes are not stripped, so JS sees a string, not an interesting + // expression. + // It's only a warning, not an error, because without more sophisticated + // parsing we can't distinguish 'ab' from 'a' + 'b'. + if len(args) >= 2 { + s := args[0] + e := args[len(args)-1] + if (s == '\'' && e == '\'') || (s == '"' && e == '"') { + fmt.Printf("WARNING: quoted argument %s to eval will evaluate to itself\n", args) + } + } test.tasks = append(test.tasks, chromedp.Evaluate(args, nil)) case "SLEEP": -- cgit v1.3