cmd/screentest: compare image sizes
The imgdiff package we use assumes the images are the same size.
So check the sizes first.
Also:
- Fix a bug where we retry instead of failing immediately.
- Make the default concurrency 1. Screen tests are flaky enough
without adding concurrency.
- Remove the check in sleep for integers. time.ParseDuration already
does that.
Change-Id: I63442d29c0846e3b6648eef993e8a113f4019bda
Reviewed-on: https://go-review.googlesource.com/c/website/+/634078
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/cmd/screentest/main.go b/cmd/screentest/main.go
index 7998f97..b1edfe8 100644
--- a/cmd/screentest/main.go
+++ b/cmd/screentest/main.go
@@ -168,7 +168,6 @@
"fmt"
"log"
"os"
- "runtime"
)
var flags options
@@ -176,7 +175,7 @@
func init() {
flag.BoolVar(&flags.update, "update", false, "update want with test")
flag.StringVar(&flags.vars, "v", "", "variables provided to script templates as comma separated KEY:VALUE pairs")
- flag.IntVar(&flags.maxConcurrency, "c", (runtime.NumCPU()+1)/2, "number of test cases to run concurrently")
+ flag.IntVar(&flags.maxConcurrency, "c", 1, "number of test cases to run concurrently")
flag.StringVar(&flags.debuggerURL, "d", "", "chrome debugger URL")
flag.StringVar(&flags.outputDirURL, "o", "", "path for output: file path or URL with 'file' or 'gs' scheme")
flag.StringVar(&flags.headers, "headers", "", "HTTP headers: comma-separated list of name:value")
diff --git a/cmd/screentest/screentest.go b/cmd/screentest/screentest.go
index 74c6c2b..44018d3 100644
--- a/cmd/screentest/screentest.go
+++ b/cmd/screentest/screentest.go
@@ -134,7 +134,7 @@
log.Print("no tests to run")
return nil
}
- log.Printf("ran %d tests in %s\n\n", nTests, time.Since(start).Truncate(time.Millisecond))
+ log.Printf("ran %d tests in %s\n", nTests, time.Since(start).Truncate(time.Millisecond))
if summary.Len() > 0 {
os.Stdout.Write(summary.Bytes())
if len(failedTests) > 0 {
@@ -440,9 +440,6 @@
if test == nil {
return nil, errors.New("directive must be in a test")
}
- if ns, err := strconv.Atoi(args); err == nil {
- return nil, fmt.Errorf("sleep argument of %d is in nanoseconds; did you mean %[1]ds?", ns)
- }
dur, err := time.ParseDuration(args)
if err != nil {
return nil, err
@@ -623,6 +620,7 @@
testScreen, wantScreen image.Image
)
fmt.Fprintf(&tc.output, "test %s ", tc.name)
+ var failReason string
for try := 0; try < maxRetries; try++ {
g, gctx := errgroup.WithContext(ctx)
g.Go(func() error {
@@ -646,6 +644,19 @@
return tc.wantImageReadWriter.writeImage(ctx, tc.wantPath, testScreen)
}
+ // Expect the images to start at (0, 0).
+ if p := testScreen.Bounds().Min; p != image.ZP {
+ return fmt.Errorf("test image starts at %s, not (0, 0)", p)
+ }
+ if p := wantScreen.Bounds().Min; p != image.ZP {
+ return fmt.Errorf("want image starts at %s, not (0, 0)", p)
+ }
+ // If the images are different sizes, don't even compare them. imgdiff does
+ // not handle differently sized images properly.
+ if tmax, wmax := testScreen.Bounds().Max, wantScreen.Bounds().Max; tmax != wmax {
+ failReason = fmt.Sprintf("test image is %s but want image is %s", tmax, wmax)
+ break
+ }
result = imgdiff.Diff(testScreen, wantScreen, &imgdiff.Options{
Threshold: 0.1,
DiffImage: true,
@@ -655,16 +666,20 @@
fmt.Fprintf(&tc.output, "(%s)", since)
return nil
}
- if result.DiffPixelsCount <= uint64(tc.retryPixels) {
- fmt.Fprintf(&tc.output, "difference is <= %d pixels\n", tc.retryPixels)
+ failReason = fmt.Sprintf("%d pixels differ", result.DiffPixelsCount)
+ if result.DiffPixelsCount > uint64(tc.retryPixels) {
+ break
}
+ fmt.Fprintf(&tc.output, "difference is <= %d pixels\n", tc.retryPixels)
}
- fmt.Fprintf(&tc.output, "(%s)\n FAIL %s != %s (%d pixels differ)\n",
- since, tc.testOrigin(), tc.wantOrigin(), result.DiffPixelsCount)
+ fmt.Fprintf(&tc.output, "(%s)\n FAIL %s != %s: %s\n",
+ since, tc.testOrigin(), tc.wantOrigin(), failReason)
g, gctx := errgroup.WithContext(ctx)
g.Go(func() error { return tc.failImageWriter.writeImage(gctx, tc.testPath, testScreen) })
g.Go(func() error { return tc.failImageWriter.writeImage(gctx, tc.wantPath, wantScreen) })
- g.Go(func() error { return tc.failImageWriter.writeImage(gctx, tc.diffPath, result.Image) })
+ if result != nil && result.Image != nil {
+ g.Go(func() error { return tc.failImageWriter.writeImage(gctx, tc.diffPath, result.Image) })
+ }
if err := g.Wait(); err != nil {
return err
}
