From 97d13922ddfd483ff5786492135931123f9392d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Sat, 2 Nov 2019 11:45:57 +0000
Subject: [PATCH] cmd/gogio: remove sleeps in x11 e2e test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We can instead synchronize with the gio app via stdout. We need three
states, since we need to first invalidate a frame and then print when
the next frame is drawn.

This is not happening on the JS test yet, because stdout printing
crashes in that case. See the comment.

This change should make the X11 test a bit faster on fast machines,
while making it more stable in small or headless machines like CI.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
 cmd/gogio/e2e_test.go     |  5 +++--
 cmd/gogio/testdata/red.go | 32 ++++++++++++++++++++++++++++++++
 cmd/gogio/x11_test.go     | 34 +++++++++++++++++++++++++---------
 3 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/cmd/gogio/e2e_test.go b/cmd/gogio/e2e_test.go
index eb542cf4..e2ee5646 100644
--- a/cmd/gogio/e2e_test.go
+++ b/cmd/gogio/e2e_test.go
@@ -21,7 +21,7 @@ type TestDriver interface {
 	// white.
 	//
 	// When the function returns, the gio app must be ready to use on the
-	// platform.
+	// platform, with its initial frame fully drawn.
 	//
 	// The returned cleanup funcs must be run in reverse order, to mimic
 	// deferred funcs.
@@ -32,7 +32,8 @@ type TestDriver interface {
 	Screenshot() image.Image
 
 	// Click performs a pointer click at the specified coordinates,
-	// including both press and release.
+	// including both press and release. It returns when the next frame is
+	// fully drawn.
 	Click(x, y int)
 }
 
diff --git a/cmd/gogio/testdata/red.go b/cmd/gogio/testdata/red.go
index 0450cf3b..9e59e1d0 100644
--- a/cmd/gogio/testdata/red.go
+++ b/cmd/gogio/testdata/red.go
@@ -4,9 +4,11 @@
 package main
 
 import (
+	"fmt"
 	"image"
 	"image/color"
 	"log"
+	"runtime"
 
 	"gioui.org/app"
 	"gioui.org/f32"
@@ -26,6 +28,18 @@ func main() {
 	app.Main()
 }
 
+type notifyFrame int
+
+const (
+	notifyNone notifyFrame = iota
+	notifyInvalidate
+	notifyPrint
+)
+
+// notify keeps track of whether we want to print to stdout to notify the user
+// when a frame is ready. Initially we want to notify about the first frame.
+var notify = notifyInvalidate
+
 func loop(w *app.Window) error {
 	topLeft := quarterWidget{
 		color: color.RGBA{R: 0xde, G: 0xad, B: 0xbe, A: 0xff},
@@ -67,6 +81,22 @@ func loop(w *app.Window) error {
 			rows.Layout(gtx, r1, r2)
 
 			e.Frame(gtx.Ops)
+
+			if runtime.GOOS == "js" {
+				// TODO(mvdan): Unfortunately, printing to
+				// stdout crashes the js/wasm port. Use the
+				// prints once the issue is fixed:
+				// https://github.com/golang/go/issues/35256
+				break
+			}
+			switch notify {
+			case notifyInvalidate:
+				notify = notifyPrint
+				w.Invalidate()
+			case notifyPrint:
+				notify = notifyNone
+				fmt.Println("frame ready")
+			}
 		}
 	}
 }
@@ -100,6 +130,8 @@ func (w *quarterWidget) Layout(gtx *layout.Context) {
 	for _, e := range gtx.Events(w) {
 		if e, ok := e.(pointer.Event); ok && e.Type == pointer.Press {
 			w.clicked = !w.clicked
+			// notify when we're done updating the frame.
+			notify = notifyInvalidate
 		}
 	}
 }
diff --git a/cmd/gogio/x11_test.go b/cmd/gogio/x11_test.go
index 72113b0b..5b772312 100644
--- a/cmd/gogio/x11_test.go
+++ b/cmd/gogio/x11_test.go
@@ -7,6 +7,7 @@
 package main_test
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"fmt"
@@ -26,10 +27,13 @@ import (
 type X11TestDriver struct {
 	t *testing.T
 
+	frameNotifs chan bool
+
 	display string
 }
 
 func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cleanups []func()) {
+	d.frameNotifs = make(chan bool, 1)
 	d.t = t_
 
 	// Pick a random display number between 1 and 100,000. Most machines
@@ -125,10 +129,14 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 	{
 		ctx, cancel := context.WithCancel(context.Background())
 		cmd := exec.CommandContext(ctx, bin)
-		out := &bytes.Buffer{}
 		cmd.Env = []string{"DISPLAY=" + d.display}
-		cmd.Stdout = out
-		cmd.Stderr = out
+		stdout, err := cmd.StdoutPipe()
+		if err != nil {
+			d.t.Fatal(err)
+		}
+		stderr := &bytes.Buffer{}
+		cmd.Stderr = stderr
+
 		if err := cmd.Start(); err != nil {
 			d.t.Fatal(err)
 		}
@@ -136,17 +144,26 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
-				// Print all output and error.
-				io.Copy(os.Stdout, out)
+				// Print stderr and error.
+				io.Copy(os.Stdout, stderr)
 				d.t.Error(err)
 			}
 			wg.Done()
 		}()
+		go func() {
+			scanner := bufio.NewScanner(stdout)
+			for scanner.Scan() {
+				line := scanner.Text()
+				if line == "frame ready" {
+					d.frameNotifs <- true
+				}
+			}
+		}()
+
 	}
 
 	// Wait for the gio app to render.
-	// TODO(mvdan): synchronize with the app instead
-	time.Sleep(400 * time.Millisecond)
+	<-d.frameNotifs
 
 	return cleanups
 }
@@ -183,8 +200,7 @@ func (d *X11TestDriver) Click(x, y int) {
 	d.xdotool("mousemove", x, y)
 	d.xdotool("click", "1")
 
-	// TODO(mvdan): synchronize with the app instead
-	time.Sleep(200 * time.Millisecond)
+	<-d.frameNotifs
 }
 
 func TestX11(t *testing.T) {