Add functional tests, fix unset exit codes on panics (#42)

* Refactor to support functional tests

* Fix unset exit code from uncaught exception

* Fix unset exit code for panic in next tick of event loop

* Fix data race on shadowed "err"

* Poke CI

* Increase test timeout for Windows race detector to complete

* Move test types and helpers to the bottom

* Refactor into table-driven tests

* Remove single-use helper

* Handle errors with testing.T in testParse helper

* Return errors instead of exit codes

* Extract HTTP server, use ctx for startup/shutdown

* Increase test timeout for Windows again

* Handle testParseOther error as assertion failure

* Rename shutdownNoWait to startShutdown for clarity

.github/workflows/ci.yml

@@ -21,6 +21,6 @@ jobs:
     - name: Install cleanenv
       run: go install ./cmd/cleanenv
     - name: Test
-      run: cleanenv -remove-prefix GITHUB_ -remove-prefix JAVA_ -- go test -v -race ./...
+      run: cleanenv -remove-prefix GITHUB_ -remove-prefix JAVA_ -- go test -v -race -timeout 5m ./...
     - name: Install
       run: go install

http_server.go

@@ -0,0 +1,51 @@
+package main
+
+import (
+	"context"
+	"log"
+	"net"
+	"net/http"
+	neturl "net/url"
+	"time"
+)
+
+func startHTTPServer(ctx context.Context, handler http.Handler, logger *log.Logger) (url string, shutdown context.CancelFunc, err error) {
+	// Need to generate a random port every time for tests in parallel to run.
+	l, err := net.Listen("tcp", "localhost:")
+	if err != nil {
+		return "", nil, err
+	}
+
+	server := &http.Server{
+		Handler: handler,
+	}
+	go func() { // serves HTTP
+		err := server.Serve(l)
+		if err != http.ErrServerClosed {
+			logger.Println(err)
+		}
+	}()
+
+	shutdownCtx, startShutdown := context.WithCancel(ctx)
+	shutdownComplete := make(chan struct{}, 1)
+	go func() { // waits for canceled ctx or triggered shutdown, then shuts down HTTP
+		<-shutdownCtx.Done()
+		shutdownTimeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		err := server.Shutdown(shutdownTimeoutCtx)
+		if err != nil {
+			logger.Println(err)
+		}
+		shutdownComplete <- struct{}{}
+	}()
+
+	shutdown = func() {
+		startShutdown()
+		<-shutdownComplete
+	}
+	url = (&neturl.URL{
+		Scheme: "http",
+		Host:   l.Addr().String(),
+	}).String()
+	return url, shutdown, nil
+}

index.html

@@ -89,7 +89,12 @@
 			}).catch((err) => {
 				console.error(err);
 			});
-			await go.run(inst);
+			try {
+				await go.run(inst);
+			} catch(e) {
+				exitCode = 1
+				console.error(e)
+			}
 			document.getElementById("doneButton").disabled = false;
 		})();
 	</script>

main.go

@@ -3,19 +3,18 @@ package main
 import (
 	"bytes"
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
-	"net"
-	"net/http"
 	"os"
+	"os/signal"
 	"path"
 	"runtime"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/chromedp/cdproto/inspector"
 	"github.com/chromedp/cdproto/profiler"
@@ -24,71 +23,64 @@ import (
 	"github.com/chromedp/chromedp"
 )
 
-var (
-	cpuProfile      *string
-	coverageProfile *string
-)
-
 func main() {
-	// NOTE: Since `os.Exit` will cause the process to exit, this defer
-	// must be at the bottom of the defer stack to allow all other defer calls to
-	// be called first.
-	exitCode := 0
+	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
+	defer cancel()
+	err := run(ctx, os.Args, os.Stderr, flag.CommandLine)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(1)
+	}
+}
+
+func run(ctx context.Context, args []string, errOutput io.Writer, flagSet *flag.FlagSet) (returnedErr error) {
+	logger := log.New(errOutput, "[wasmbrowsertest]: ", log.LstdFlags|log.Lshortfile)
 	defer func() {
-		if exitCode != 0 {
-			os.Exit(exitCode)
+		r := recover()
+		if r != nil {
+			returnedErr = fmt.Errorf("Panicked: %+v", r)
 		}
 	}()
 
-	logger := log.New(os.Stderr, "[wasmbrowsertest]: ", log.LstdFlags|log.Lshortfile)
-	if len(os.Args) < 2 {
-		logger.Fatal("Please pass a wasm file as a parameter")
+	if len(args) < 2 {
+		return errors.New("Please pass a wasm file as a parameter")
 	}
 
-	cpuProfile = flag.String("test.cpuprofile", "", "")
-	coverageProfile = flag.String("test.coverprofile", "", "")
+	cpuProfile := flagSet.String("test.cpuprofile", "", "")
+	coverageProfile := flagSet.String("test.coverprofile", "", "")
 
-	wasmFile := os.Args[1]
+	wasmFile := args[1]
 	ext := path.Ext(wasmFile)
 	// net/http code does not take js/wasm path if it is a .test binary.
 	if ext == ".test" {
 		wasmFile = strings.Replace(wasmFile, ext, ".wasm", -1)
-		err := copyFile(os.Args[1], wasmFile)
+		err := copyFile(args[1], wasmFile)
 		if err != nil {
-			logger.Fatal(err)
+			return err
 		}
 		defer os.Remove(wasmFile)
-		os.Args[1] = wasmFile
+		args[1] = wasmFile
 	}
 
-	passon := gentleParse(flag.CommandLine, os.Args[2:])
+	passon, err := gentleParse(flagSet, args[2:])
+	if err != nil {
+		return err
+	}
 	passon = append([]string{wasmFile}, passon...)
 	if *coverageProfile != "" {
 		passon = append(passon, "-test.coverprofile="+*coverageProfile)
 	}
 
-	// Need to generate a random port every time for tests in parallel to run.
-	l, err := net.Listen("tcp", "localhost:")
-	if err != nil {
-		logger.Fatal(err)
-	}
-	tcpL, ok := l.(*net.TCPListener)
-	if !ok {
-		logger.Fatal("net.Listen did not return a TCPListener")
-	}
-	_, port, err := net.SplitHostPort(tcpL.Addr().String())
-	if err != nil {
-		logger.Fatal(err)
-	}
-
 	// Setup web server.
 	handler, err := NewWASMServer(wasmFile, passon, *coverageProfile, logger)
 	if err != nil {
-		logger.Fatal(err)
+		return err
 	}
-	httpServer := &http.Server{
-		Handler: handler,
+	url, shutdownHTTPServer, err := startHTTPServer(ctx, handler, logger)
+	if err != nil {
+		return err
 	}
+	defer shutdownHTTPServer()
 
 	opts := chromedp.DefaultExecAllocatorOptions[:]
 	if os.Getenv("WASM_HEADLESS") == "off" {
@@ -105,7 +97,7 @@ func main() {
 	}
 
 	// create chrome instance
-	allocCtx, cancelAllocCtx := chromedp.NewExecAllocator(context.Background(), opts...)
+	allocCtx, cancelAllocCtx := chromedp.NewExecAllocator(ctx, opts...)
 	defer cancelAllocCtx()
 	ctx, cancelCtx := chromedp.NewContext(allocCtx)
 	defer cancelCtx()
@@ -114,17 +106,10 @@ func main() {
 		handleEvent(ctx, ev, logger)
 	})
 
-	done := make(chan struct{})
-	go func() {
-		err = httpServer.Serve(l)
-		if err != http.ErrServerClosed {
-			logger.Println(err)
-		}
-		done <- struct{}{}
-	}()
+	var exitCode int
 	var coverageProfileContents string
 	tasks := []chromedp.Action{
-		chromedp.Navigate(`http://localhost:` + port),
+		chromedp.Navigate(url),
 		chromedp.WaitEnabled(`#doneButton`),
 		chromedp.Evaluate(`exitCode;`, &exitCode),
 		chromedp.Evaluate(`coverageProfileContents;`, &coverageProfileContents),
@@ -167,20 +152,13 @@ func main() {
 
 	err = chromedp.Run(ctx, tasks...)
 	if err != nil {
-		logger.Println(err)
+		// Browser did not exit cleanly. Likely failed with an uncaught error.
+		return err
 	}
 	if exitCode != 0 {
-		exitCode = 1
+		return fmt.Errorf("exit with status %d", exitCode)
 	}
-	// create a timeout
-	ctx, cancelHTTPCtx := context.WithTimeout(ctx, 5*time.Second)
-	defer cancelHTTPCtx()
-	// Close shop
-	err = httpServer.Shutdown(ctx)
-	if err != nil {
-		logger.Println(err)
-	}
-	<-done
+	return nil
 }
 
 func copyFile(src, dst string) error {