~eliasnaur/gio-patches

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] cmd/gogio: start using testing.T.Cleanup

Details
Message ID
<20191107141519.102318-1-mvdan@mvdan.cc>
DKIM signature
pass
Download raw message
Patch: +19 -35
Now that we use tip due to breaking changes in Go's JS APIs, let's
replace our hacky cleanup list code with 1.14's upcoming
testing.T.Cleanup.

Adding more deliberate uses of tip would ususally be best avoided, but
these will only affect developers working on gio's tests, not regular
users of gio.

While at it, remove some debug t.Logf calls I forgot to remove.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
 cmd/gogio/e2e_test.go     | 11 ++---------
 cmd/gogio/js_test.go      | 12 +++++-------
 cmd/gogio/wayland_test.go | 17 ++++++-----------
 cmd/gogio/x11_test.go     | 14 ++++++--------
 4 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/cmd/gogio/e2e_test.go b/cmd/gogio/e2e_test.go
index fe853ac..d16d90f 100644
--- a/cmd/gogio/e2e_test.go
+++ b/cmd/gogio/e2e_test.go
@@ -24,11 +24,7 @@ type TestDriver interface {
 	//
 	// When the function returns, the gio app must be ready to use on the
 	// platform, with its initial frame fully drawn.
-	//
-	// The returned cleanup funcs must be run in reverse order, to mimic
-	// deferred funcs.
-	// TODO(mvdan): replace with testing.T.Cleanup once Go 1.14 is out.
-	Start(t *testing.T, path string, width, height int) (cleanups []func())
+	Start(t *testing.T, path string, width, height int)
 
 	// Screenshot takes a screenshot of the Gio app on the platform.
 	Screenshot() image.Image
@@ -67,10 +63,7 @@ func TestEndToEnd(t *testing.T) {
 
 func runEndToEndTest(t *testing.T, driver TestDriver) {
 	size := image.Point{X: 800, Y: 600}
-	cleanups := driver.Start(t, "testdata/red.go", size.X, size.Y)
-	for _, cleanup := range cleanups {
-		defer cleanup()
-	}
+	driver.Start(t, "testdata/red.go", size.X, size.Y)
 
 	// The colors are split in four rectangular sections. Check the corners
 	// of each of the sections. We check the corners left to right, top to
diff --git a/cmd/gogio/js_test.go b/cmd/gogio/js_test.go
index d74bc02..1459e8a 100644
--- a/cmd/gogio/js_test.go
+++ b/cmd/gogio/js_test.go
@@ -30,7 +30,7 @@ type JSTestDriver struct {
 	ctx context.Context
 }
 
-func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) (cleanups []func()) {
+func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) {
 	d.t = t_
 
 	if raceEnabled {
@@ -42,7 +42,7 @@ func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) (cle
 	if err != nil {
 		d.t.Fatal(err)
 	}
-	cleanups = append(cleanups, func() { os.RemoveAll(dir) })
+	d.t.Cleanup(func() { os.RemoveAll(dir) })
 
 	// TODO(mvdan): This is inefficient, as we link the gogio tool every time.
 	// Consider options in the future. On the plus side, this is simple.
@@ -73,13 +73,13 @@ func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) (cle
 	)
 
 	actx, cancel := chromedp.NewExecAllocator(context.Background(), opts...)
-	cleanups = append(cleanups, cancel)
+	d.t.Cleanup(cancel)
 
 	ctx, cancel := chromedp.NewContext(actx,
 		// Send all logf/errf calls to t.Logf
 		chromedp.WithLogf(d.t.Logf),
 	)
-	cleanups = append(cleanups, cancel)
+	d.t.Cleanup(cancel)
 	d.ctx = ctx
 
 	if err := chromedp.Run(ctx); err != nil {
@@ -109,7 +109,7 @@ func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) (cle
 	// Third, serve the app folder, set the browser tab dimensions, and
 	// navigate to the folder.
 	ts := httptest.NewServer(http.FileServer(http.Dir(dir)))
-	cleanups = append(cleanups, ts.Close)
+	d.t.Cleanup(ts.Close)
 
 	if err := chromedp.Run(ctx,
 		chromedp.EmulateViewport(int64(width), int64(height)),
@@ -125,8 +125,6 @@ func (d *JSTestDriver) Start(t_ *testing.T, path string, width, height int) (cle
 	}
 	// TODO(mvdan): synchronize with the app instead
 	time.Sleep(200 * time.Millisecond)
-
-	return cleanups
 }
 
 func (d *JSTestDriver) Screenshot() image.Image {
diff --git a/cmd/gogio/wayland_test.go b/cmd/gogio/wayland_test.go
index 143e5bc..c3f0672 100644
--- a/cmd/gogio/wayland_test.go
+++ b/cmd/gogio/wayland_test.go
@@ -42,7 +42,7 @@ default_border none
 
 var rxSwayReady = regexp.MustCompile(`Running compositor on wayland display '(.*)'`)
 
-func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int) (cleanups []func()) {
+func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int) {
 	d.frameNotifs = make(chan bool, 1)
 	d.t = t_
 
@@ -69,7 +69,7 @@ func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int)
 	if err != nil {
 		d.t.Fatal(err)
 	}
-	cleanups = append(cleanups, func() { os.RemoveAll(dir) })
+	d.t.Cleanup(func() { os.RemoveAll(dir) })
 
 	bin := filepath.Join(dir, "red")
 	flags := []string{"build", "-tags", "nox11", "-o=" + bin}
@@ -100,7 +100,7 @@ func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int)
 	env = append(env, "XDG_RUNTIME_DIR="+d.runtimeDir)
 
 	var wg sync.WaitGroup
-	cleanups = append(cleanups, wg.Wait)
+	d.t.Cleanup(wg.Wait)
 
 	// First, start sway.
 	{
@@ -114,8 +114,8 @@ func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int)
 		if err := cmd.Start(); err != nil {
 			d.t.Fatal(err)
 		}
-		cleanups = append(cleanups, cancel)
-		cleanups = append(cleanups, func() {
+		d.t.Cleanup(cancel)
+		d.t.Cleanup(func() {
 			// Give it a chance to exit gracefully, cleaning up
 			// after itself. After 10ms, the deferred cancel above
 			// will signal an os.Kill.
@@ -163,7 +163,7 @@ func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int)
 		if err := cmd.Start(); err != nil {
 			d.t.Fatal(err)
 		}
-		cleanups = append(cleanups, cancel)
+		d.t.Cleanup(cancel)
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
@@ -186,8 +186,6 @@ func (d *WaylandTestDriver) Start(t_ *testing.T, path string, width, height int)
 
 	// Wait for the gio app to render.
 	<-d.frameNotifs
-
-	return cleanups
 }
 
 func (d *WaylandTestDriver) Screenshot() image.Image {
@@ -216,9 +214,6 @@ func (d *WaylandTestDriver) swaymsg(args ...interface{}) {
 	if out, err := cmd.CombinedOutput(); err != nil {
 		d.t.Errorf("%s", out)
 		d.t.Fatal(err)
-	} else {
-		d.t.Logf("%v", args)
-		d.t.Logf("%s", out)
 	}
 }
 
diff --git a/cmd/gogio/x11_test.go b/cmd/gogio/x11_test.go
index 4aba665..f642b29 100644
--- a/cmd/gogio/x11_test.go
+++ b/cmd/gogio/x11_test.go
@@ -28,7 +28,7 @@ type X11TestDriver struct {
 	display string
 }
 
-func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cleanups []func()) {
+func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) {
 	d.frameNotifs = make(chan bool, 1)
 	d.t = t_
 
@@ -66,7 +66,7 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 	if err != nil {
 		d.t.Fatal(err)
 	}
-	cleanups = append(cleanups, func() { os.RemoveAll(dir) })
+	d.t.Cleanup(func() { os.RemoveAll(dir) })
 
 	bin := filepath.Join(dir, "red")
 	flags := []string{"build", "-tags", "nowayland", "-o=" + bin}
@@ -80,7 +80,7 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 	}
 
 	var wg sync.WaitGroup
-	cleanups = append(cleanups, wg.Wait)
+	d.t.Cleanup(wg.Wait)
 
 	// First, start the X server.
 	{
@@ -92,8 +92,8 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 		if err := cmd.Start(); err != nil {
 			d.t.Fatal(err)
 		}
-		cleanups = append(cleanups, cancel)
-		cleanups = append(cleanups, func() {
+		d.t.Cleanup(cancel)
+		d.t.Cleanup(func() {
 			// Give it a chance to exit gracefully, cleaning up
 			// after itself. After 10ms, the deferred cancel above
 			// will signal an os.Kill.
@@ -141,7 +141,7 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 		if err := cmd.Start(); err != nil {
 			d.t.Fatal(err)
 		}
-		cleanups = append(cleanups, cancel)
+		d.t.Cleanup(cancel)
 		wg.Add(1)
 		go func() {
 			if err := cmd.Wait(); err != nil && ctx.Err() == nil {
@@ -165,8 +165,6 @@ func (d *X11TestDriver) Start(t_ *testing.T, path string, width, height int) (cl
 
 	// Wait for the gio app to render.
 	<-d.frameNotifs
-
-	return cleanups
 }
 
 func (d *X11TestDriver) Screenshot() image.Image {
-- 
2.24.0
Details
Message ID
<BY9RGM01CJH8.3B5WJU5S7FWX0@toolbox>
In-Reply-To
<20191107141519.102318-1-mvdan@mvdan.cc> (view parent)
DKIM signature
pass
Download raw message
Thanks!

remote: Build started: https://builds.sr.ht/~eliasnaur/job/105972 [android.yml]
remote: Build started: https://builds.sr.ht/~eliasnaur/job/105973 [apple.yml]
remote: Build started: https://builds.sr.ht/~eliasnaur/job/105974 [freebsd.yml]
remote: Build started: https://builds.sr.ht/~eliasnaur/job/105975 [linux.yml]
To git.sr.ht:~eliasnaur/gio
   75a58e4..6fedfaf  master -> master