From 6fedfaf3af4ed51eed58898d7f8d3f685e049223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 7 Nov 2019 14:15:19 +0000 Subject: [PATCH] cmd/gogio: start using testing.T.Cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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í --- 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 fe853ac0..d16d90fe 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 d74bc02f..1459e8ac 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 143e5bc5..c3f06723 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 4aba665b..f642b295 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 {