cmd/gogio: make e2e test output consistent

Fix a long-standing TODO: instead of each sub-test handling its own
output separately, just make each expose its output via an io.Reader.
Then, the shared driverBase code can tell if any of the lines contain
the magic "gio frame ready" string.

Reduces the amount of code a bit, but most importantly, it keeps the "is
a frame ready?" logic in a single place.

In the future, this also enables us to do more with all the e2e test app
output consistently. For example, we might want to add a -debug flag to
always log output lines as they happen.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>

cmd/gogio/android_test.go

@@ -3,7 +3,6 @@
 package main_test
 
 import (
-	"bufio"
 	"bytes"
 	"context"
 	"fmt"
@@ -13,7 +12,6 @@ import (
 	"os/exec"
 	"path/filepath"
 	"regexp"
-	"strings"
 )
 
 type AndroidTestDriver struct {
@@ -80,24 +78,16 @@ func (d *AndroidTestDriver) Start(path string) {
 			"-T1",      // don't show prevoius log messages
 			appid+":*", // show all logs from our gio app ID
 		)
-		logcat, err := cmd.StdoutPipe()
+		output, err := cmd.StdoutPipe()
 		if err != nil {
 			d.Fatal(err)
 		}
-		cmd.Stderr = os.Stderr
+		cmd.Stderr = cmd.Stdout
+		d.output = output
 		if err := cmd.Start(); err != nil {
 			d.Fatal(err)
 		}
 		d.Cleanup(cancel)
-		go func() {
-			scanner := bufio.NewScanner(logcat)
-			for scanner.Scan() {
-				line := scanner.Text()
-				if strings.HasSuffix(line, ": frame ready") {
-					d.frameNotifs <- true
-				}
-			}
-		}()
 	}
 
 	// Start the app.

cmd/gogio/e2e_test.go

@@ -3,11 +3,13 @@
 package main_test
 
 import (
+	"bufio"
 	"errors"
 	"flag"
 	"fmt"
 	"image"
 	"image/color"
+	"io"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -50,17 +52,13 @@ type driverBase struct {
 
 	width, height int
 
-	// TODO(mvdan): Make this lower-level, so that each driver can simply
-	// send us each line of output from the app. That will let us
-	// deduplicate some code, and also show app output as test logs in a
-	// consistent way.
+	output      io.Reader
 	frameNotifs chan bool
 }
 
 func (d *driverBase) initBase(t *testing.T, width, height int) {
 	d.T = t
 	d.width, d.height = width, height
-	d.frameNotifs = make(chan bool, 1)
 }
 
 func TestEndToEnd(t *testing.T) {
@@ -251,6 +249,32 @@ func checkImageCorners(img image.Image, topLeft, topRight, botLeft, botRight col
 func (d *driverBase) waitForFrame() {
 	d.Helper()
 
+	if d.frameNotifs == nil {
+		// Start the goroutine that reads output lines and notifies of
+		// new frames via frameNotifs. The test doesn't wait for this
+		// goroutine to finish; it will naturally end when the output
+		// reader reaches an error like EOF.
+		d.frameNotifs = make(chan bool, 1)
+		if d.output == nil {
+			d.Fatal("need an output reader to be notified of frames")
+		}
+		go func() {
+			scanner := bufio.NewScanner(d.output)
+			for scanner.Scan() {
+				line := scanner.Text()
+				if strings.Contains(line, "gio frame ready") {
+					d.frameNotifs <- true
+				}
+			}
+			// Since we're only interested in the output while the
+			// app runs, and we don't know when it finishes here,
+			// ignore "already closed" pipe errors.
+			if err := scanner.Err(); err != nil && !errors.Is(err, os.ErrClosed) {
+				d.Errorf("reading app output: %v", err)
+			}
+		}()
+	}
+
 	// Unfortunately, there isn't a way to select on a test failing, since
 	// testing.T doesn't have anything like a context or a "done" channel.
 	//

cmd/gogio/js_test.go

@@ -8,10 +8,10 @@ import (
 	"errors"
 	"image"
 	"image/png"
+	"io"
 	"net/http"
 	"net/http/httptest"
 	"os/exec"
-	"strings"
 
 	"github.com/chromedp/cdproto/runtime"
 	"github.com/chromedp/chromedp"
@@ -73,27 +73,26 @@ func (d *JSTestDriver) Start(path string) {
 		}
 		d.Fatal(err)
 	}
+	pr, pw := io.Pipe()
+	d.Cleanup(func() { pw.Close() })
+	d.output = pr
 	chromedp.ListenTarget(ctx, func(ev interface{}) {
 		switch ev := ev.(type) {
 		case *runtime.EventConsoleAPICalled:
-			if ev.Type == "log" && len(ev.Args) == 1 &&
-				// Note that the argument values are JSON.
-				string(ev.Args[0].Value) == `"frame ready"` {
-
-				d.frameNotifs <- true
-				// These logs are expected. Don't show them.
-				break
-			}
 			switch ev.Type {
 			case "log", "info", "warning", "error":
-				var args strings.Builder
+				var b bytes.Buffer
+				b.WriteString("console.")
+				b.WriteString(string(ev.Type))
+				b.WriteString("(")
 				for i, arg := range ev.Args {
 					if i > 0 {
-						args.WriteString(", ")
+						b.WriteString(", ")
 					}
-					args.Write(arg.Value)
+					b.Write(arg.Value)
 				}
-				d.Logf("console %s: %s", ev.Type, args.String())
+				b.WriteString(")\n")
+				pw.Write(b.Bytes())
 			}
 		}
 	})

cmd/gogio/testdata/red.go

@@ -89,7 +89,7 @@ func loop(w *app.Window) error {
 				w.Invalidate()
 			case notifyPrint:
 				notify = notifyNone
-				fmt.Println("frame ready")
+				fmt.Println("gio frame ready")
 			}
 		}
 	}

cmd/gogio/wayland_test.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"image"
 	"image/png"
-	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -137,12 +136,12 @@ func (d *WaylandTestDriver) Start(path string) {
 		ctx, cancel := context.WithCancel(context.Background())
 		cmd := exec.CommandContext(ctx, bin)
 		cmd.Env = []string{"XDG_RUNTIME_DIR=" + d.runtimeDir, "WAYLAND_DISPLAY=" + d.display}
-		stdout, err := cmd.StdoutPipe()
+		output, err := cmd.StdoutPipe()
 		if err != nil {
 			d.Fatal(err)
 		}
-		stderr := &bytes.Buffer{}
-		cmd.Stderr = stderr
+		cmd.Stderr = cmd.Stdout
+		d.output = output
 		if err := cmd.Start(); err != nil {
 			d.Fatal(err)
 		}
@@ -150,21 +149,10 @@ func (d *WaylandTestDriver) Start(path string) {
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
-				// Print stderr and error.
-				io.Copy(os.Stdout, stderr)
 				d.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.

cmd/gogio/windows_test.go

@@ -3,11 +3,8 @@
 package main_test
 
 import (
-	"bufio"
-	"bytes"
 	"context"
 	"image"
-	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -95,13 +92,12 @@ func (d *WineTestDriver) Start(path string) {
 			"WINEDEBUG=-all", // hide warnings and other noise
 			"WINEPREFIX=" + wineprefix,
 		}
-		stdout, err := cmd.StdoutPipe()
+		output, err := cmd.StdoutPipe()
 		if err != nil {
 			d.Fatal(err)
 		}
-		stderr := &bytes.Buffer{}
-		cmd.Stderr = stderr
-
+		cmd.Stderr = cmd.Stdout
+		d.output = output
 		if err := cmd.Start(); err != nil {
 			d.Fatal(err)
 		}
@@ -109,22 +105,10 @@ func (d *WineTestDriver) Start(path string) {
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
-				// Print stderr and error.
-				io.Copy(os.Stdout, stderr)
 				d.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.
 	d.waitForFrame()

cmd/gogio/x11_test.go

@@ -3,7 +3,6 @@
 package main_test
 
 import (
-	"bufio"
 	"bytes"
 	"context"
 	"fmt"
@@ -47,13 +46,12 @@ func (d *X11TestDriver) Start(path string) {
 		ctx, cancel := context.WithCancel(context.Background())
 		cmd := exec.CommandContext(ctx, bin)
 		cmd.Env = []string{"DISPLAY=" + d.display}
-		stdout, err := cmd.StdoutPipe()
+		output, err := cmd.StdoutPipe()
 		if err != nil {
 			d.Fatal(err)
 		}
-		stderr := &bytes.Buffer{}
-		cmd.Stderr = stderr
-
+		cmd.Stderr = cmd.Stdout
+		d.output = output
 		if err := cmd.Start(); err != nil {
 			d.Fatal(err)
 		}
@@ -61,22 +59,10 @@ func (d *X11TestDriver) Start(path string) {
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
-				// Print stderr and error.
-				io.Copy(os.Stdout, stderr)
 				d.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.