Solve intermittent build errors (#262)

* Solve intermittent build errors
* Eliminate `panic()` in `gui.Screenshot()` function

gui/gui.go

@@ -373,7 +373,6 @@ func (gui *GUI) Close() {
 }
 
 func (gui *GUI) Render() error {
-
 	gui.logger.Debugf("Creating window...")
 	var err error
 	gui.window, err = gui.createWindow()
@@ -470,16 +469,6 @@ func (gui *GUI) Render() error {
 	gl.Disable(gl.DEPTH_TEST)
 	gl.TexParameterf(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST)
 
-	ticker := time.NewTicker(time.Second)
-	defer ticker.Stop()
-
-	go func() {
-		for {
-			<-ticker.C
-			gui.logger.Sync()
-		}
-	}()
-
 	gui.terminal.SetProgram(program)
 
 	latestVersion := ""
@@ -496,7 +485,9 @@ func (gui *GUI) Render() error {
 	showMessage := true
 
 	stop := make(chan struct{})
-	go gui.waker(stop)
+	var waitForWaker sync.WaitGroup
+	waitForWaker.Add(1)
+	go gui.waker(stop, &waitForWaker)
 
 	for !gui.window.ShouldClose() {
 		gui.redraw(true)
@@ -560,9 +551,13 @@ Buffer Size: %d lines
 		}
 	}
 
-	close(stop) // Tell waker to end.
+	gui.logger.Debug("Stopping render...")
+
+	close(stop)         // Tell waker to end...
+	waitForWaker.Wait() // ...and wait it to end
+
+	gui.logger.Debug("Render stopped")
 
-	gui.logger.Debugf("Stopping render...")
 	return nil
 
 }
@@ -571,10 +566,13 @@ Buffer Size: %d lines
 // waking up the main thread when the GUI needs to be
 // redrawn. Limiting is applied on wakeups to avoid excessive CPU
 // usage when the terminal is being updated rapidly.
-func (gui *GUI) waker(stop <-chan struct{}) {
+func (gui *GUI) waker(stop <-chan struct{}, wg *sync.WaitGroup) {
+	defer wg.Done()
+
 	dirty := gui.terminal.Dirty()
 	var nextWake <-chan time.Time
 	var last time.Time
+forLoop:
 	for {
 		select {
 		case <-dirty:
@@ -598,7 +596,7 @@ func (gui *GUI) waker(stop <-chan struct{}) {
 			glfw.PostEmptyEvent()
 			nextWake = nil
 		case <-stop:
-			return
+			break forLoop
 		}
 	}
 }
@@ -869,18 +867,27 @@ func (gui *GUI) SwapBuffers() {
 	gui.window.SwapBuffers()
 }
 
-func (gui *GUI) Screenshot(path string) {
+func (gui *GUI) Screenshot(path string) error {
 	x, y := gui.window.GetPos()
 	w, h := gui.window.GetSize()
 
 	img, err := screenshot.CaptureRect(image.Rectangle{Min: image.Point{X: x, Y: y},
 		Max: image.Point{X: x + w, Y: y + h}})
 	if err != nil {
-		panic(err)
+		return err
+	}
+	file, err := os.Create(path)
+	if err != nil {
+		return err
 	}
-	file, _ := os.Create(path)
 	defer file.Close()
-	png.Encode(file, img)
+	err = png.Encode(file, img)
+	if err != nil {
+		os.Remove(path)
+		return err
+	}
+
+	return nil
 }
 
 func (gui *GUI) windowPosChangeCallback(w *glfw.Window, xpos int, ypos int) {

main.go

@@ -12,6 +12,7 @@ import (
 	"github.com/liamg/aminal/platform"
 	"github.com/liamg/aminal/terminal"
 	"github.com/riywo/loginshell"
+	"time"
 )
 
 type callback func(terminal *terminal.Terminal, g *gui.GUI)
@@ -32,7 +33,17 @@ func initialize(unitTestfunc callback, configOverride *config.Config) {
 		fmt.Printf("Failed to create logger: %s\n", err)
 		os.Exit(1)
 	}
-	defer logger.Sync()
+	ticker := time.NewTicker(time.Second)
+	go func() {
+		for {
+			<-ticker.C
+			logger.Sync()
+		}
+	}()
+	defer func() {
+		ticker.Stop()
+		logger.Sync()
+	}()
 
 	if conf.CPUProfile != "" {
 		logger.Infof("Starting CPU profiling...")
@@ -61,7 +72,7 @@ func initialize(unitTestfunc callback, configOverride *config.Config) {
 	guestProcess, err := pty.CreateGuestProcess(shellStr)
 	if err != nil {
 		pty.Close()
-		logger.Fatalf("Failed to start your shell: %s", err)
+		logger.Fatalf("Failed to start your shell %q: %s", shellStr, err)
 	}
 	defer guestProcess.Close()
 

main_test.go

@@ -11,11 +11,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/carlogit/phash"
 	"github.com/liamg/aminal/config"
 	"github.com/liamg/aminal/gui"
 	"github.com/liamg/aminal/terminal"
-
-	"github.com/carlogit/phash"
 )
 
 var termRef *terminal.Terminal
@@ -47,10 +46,14 @@ func hash(path string) string {
 	return imageHash
 }
 
+func imagesAreEqual(expected string, actual string) int {
+	expectedHash := hash(expected)
+	actualHash := hash(actual)
+	return phash.GetDistance(expectedHash, actualHash)
+}
+
 func compareImages(expected string, actual string) {
-	template := hash(expected)
+	distance := imagesAreEqual(expected, actual)
-	screen := hash(actual)
-	distance := phash.GetDistance(template, screen)
 	if distance != 0 {
 		os.Exit(terminate(fmt.Sprintf("Screenshot \"%s\" doesn't match expected image \"%s\". Distance of hashes difference: %d\n",
 			actual, expected, distance)))
@@ -58,19 +61,55 @@ func compareImages(expected string, actual string) {
 }
 
 func send(terminal *terminal.Terminal, cmd string) {
-	terminal.Write([]byte(cmd))
+	err := terminal.Write([]byte(cmd))
+	if err != nil {
+		panic(err)
+	}
 }
 
 func enter(terminal *terminal.Terminal) {
-	terminal.Write([]byte("\n"))
+	err := terminal.Write([]byte("\n"))
+	if err != nil {
+		panic(err)
+	}
+}
+
+func validateScreen(img string, waitForChange bool) {
+	fmt.Printf("taking screenshot: %s and comparing...", img)
+
+	err := guiRef.Screenshot(img)
+	if err != nil {
+		panic(err)
 	}
 
-func validateScreen(img string) {
-	guiRef.Screenshot(img)
 	compareImages(strings.Join([]string{"vttest/", img}, ""), img)
 
+	fmt.Printf("compare OK\n")
+
 	enter(termRef)
+
+	if waitForChange {
+		fmt.Print("Waiting for screen change...")
+		attempts := 10
+		for {
 			sleep()
+			actualScren := "temp.png"
+			err = guiRef.Screenshot(actualScren)
+			if err != nil {
+				panic(err)
+			}
+			distance := imagesAreEqual(actualScren, img)
+			if distance != 0 {
+				break
+			}
+			fmt.Printf(" %d", attempts)
+			attempts--
+			if attempts <= 0 {
+				break
+			}
+		}
+		fmt.Print("done\n")
+	}
 }
 
 func TestMain(m *testing.M) {
@@ -115,12 +154,12 @@ func TestCursorMovement(t *testing.T) {
 				os.Exit(terminate(fmt.Sprintf("ActiveBuffer doesn't match vttest template vttest/test-cursor-movement-1")))
 			}
 
-			validateScreen("test-cursor-movement-1.png")
+			validateScreen("test-cursor-movement-1.png", true)
-			validateScreen("test-cursor-movement-2.png")
+			validateScreen("test-cursor-movement-2.png", true)
-			validateScreen("test-cursor-movement-3.png")
+			validateScreen("test-cursor-movement-3.png", true)
-			validateScreen("test-cursor-movement-4.png")
+			validateScreen("test-cursor-movement-4.png", true)
-			validateScreen("test-cursor-movement-5.png")
+			validateScreen("test-cursor-movement-5.png", true)
-			validateScreen("test-cursor-movement-6.png")
+			validateScreen("test-cursor-movement-6.png", false)
 
 			g.Close()
 		}
@@ -142,21 +181,21 @@ func TestScreenFeatures(t *testing.T) {
 			send(term, "2\n")
 			sleep()
 
-			validateScreen("test-screen-features-1.png")
+			validateScreen("test-screen-features-1.png", true)
-			validateScreen("test-screen-features-2.png")
+			validateScreen("test-screen-features-2.png", true)
-			validateScreen("test-screen-features-3.png")
+			validateScreen("test-screen-features-3.png", true)
-			validateScreen("test-screen-features-4.png")
+			validateScreen("test-screen-features-4.png", true)
-			validateScreen("test-screen-features-5.png")
+			validateScreen("test-screen-features-5.png", true)
-			validateScreen("test-screen-features-6.png")
+			validateScreen("test-screen-features-6.png", true)
-			validateScreen("test-screen-features-7.png")
+			validateScreen("test-screen-features-7.png", true)
-			validateScreen("test-screen-features-8.png")
+			validateScreen("test-screen-features-8.png", true)
-			validateScreen("test-screen-features-9.png")
+			validateScreen("test-screen-features-9.png", true)
-			validateScreen("test-screen-features-10.png")
+			validateScreen("test-screen-features-10.png", true)
-			validateScreen("test-screen-features-11.png")
+			validateScreen("test-screen-features-11.png", true)
-			validateScreen("test-screen-features-12.png")
+			validateScreen("test-screen-features-12.png", true)
-			validateScreen("test-screen-features-13.png")
+			validateScreen("test-screen-features-13.png", true)
-			validateScreen("test-screen-features-14.png")
+			validateScreen("test-screen-features-14.png", true)
-			validateScreen("test-screen-features-15.png")
+			validateScreen("test-screen-features-15.png", false)
 
 			g.Close()
 		}
@@ -178,10 +217,9 @@ func TestSixel(t *testing.T) {
 			send(term, "clear\n")
 			sleep()
 			send(term, "cat example.sixel\n")
-			sleep(4)
+			sleep(10) // Displaying SIXEL graphics *sometimes* takes long time. Excessive synchronization???
 
-			guiRef.Screenshot("test-sixel.png")
+			validateScreen("test-sixel.png", false)
-			validateScreen("test-sixel.png")
 
 			g.Close()
 		}