test-runner: Print results as soon as they're available

Gives feedback much eariler.

Also move the error messages after the table

Change-Id: If462fd9bbc72ff9428d6ee77c8ec6c9338f7f27e
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54000
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>

tools/src/cmd/test-runner/main.go

@@ -26,7 +26,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync"
 	"unicode/utf8"
 
 	"dawn.googlesource.com/tint/tools/src/fileutils"
@@ -105,7 +104,7 @@ func run() error {
 	}
 
 	// Allow using '/' in the filter on Windows
-	filter = strings.ReplaceAll(filter, "/", string(filepath.Separator));
+	filter = strings.ReplaceAll(filter, "/", string(filepath.Separator))
 
 	// Split the --filter flag up by ',', trimming any whitespace at the start and end
 	globIncludes := strings.Split(filter, ",")
@@ -199,62 +198,51 @@ func run() error {
 	}
 	fmt.Println()
 
-	results := make([]map[outputFormat]*status, len(files))
+	// Build the list of results.
-	jobs := make(chan job, 256)
+	// These hold the chans used to report the job results.
+	results := make([]map[outputFormat]chan status, len(files))
+	for i := range files {
+		fileResults := map[outputFormat]chan status{}
+		for _, format := range formats {
+			fileResults[format] = make(chan status, 1)
+		}
+		results[i] = fileResults
+	}
+
+	pendingJobs := make(chan job, 256)
 
 	// Spawn numCPU job runners...
-	wg := sync.WaitGroup{}
-	wg.Add(numCPU)
 	for cpu := 0; cpu < numCPU; cpu++ {
 		go func() {
-			defer wg.Done()
+			for job := range pendingJobs {
-			for job := range jobs {
 				job.run(dir, exe, dxcPath, xcrunPath, generateExpected, generateSkip)
 			}
 		}()
 	}
 
 	// Issue the jobs...
-	for i, file := range files { // For each test file...
+	go func() {
-		file := filepath.Join(dir, file)
+		for i, file := range files { // For each test file...
-		fileResults := map[outputFormat]*status{}
+			file := filepath.Join(dir, file)
-		for _, format := range formats { // For each output format...
+			for _, format := range formats { // For each output format...
-			result := &status{}
+				pendingJobs <- job{
-			jobs <- job{
+					file:   file,
-				file:   file,
+					format: format,
-				format: format,
+					result: results[i][format],
-				result: result,
+				}
-			}
-			fileResults[format] = result
-		}
-		results[i] = fileResults
-	}
-
-	// Wait for the jobs to all finish...
-	close(jobs)
-	wg.Wait()
-
-	// Time to print the outputs
-
-	// Start by printing the error message for any file x format combinations
-	// that failed...
-	for i, file := range files {
-		results := results[i]
-		for _, format := range formats {
-			if err := results[format].err; err != nil {
-				color.Set(color.FgBlue)
-				fmt.Printf("%s ", file)
-				color.Set(color.FgCyan)
-				fmt.Printf("%s ", format)
-				color.Set(color.FgRed)
-				fmt.Println("FAIL")
-				color.Unset()
-				fmt.Println(indent(err.Error(), 4))
 			}
 		}
+		close(pendingJobs)
+	}()
+
+	type failure struct {
+		file   string
+		format outputFormat
+		err    error
 	}
 
-	// Now print the table of file x format
+	// Print the table of file x format
+	failures := []failure{}
 	numTests, numPass, numSkip, numFail := 0, 0, 0, 0
 	filenameFmt := columnFormat(maxStringLen(files), false)
 
@@ -285,8 +273,13 @@ func run() error {
 		fmt.Printf(" ┃ ")
 		for _, format := range formats {
 			formatFmt := columnFormat(formatWidth(format), true)
-			result := results[format]
+			result := <-results[format]
 			numTests++
+			if err := result.err; err != nil {
+				failures = append(failures, failure{
+					file: file, format: format, err: err,
+				})
+			}
 			switch result.code {
 			case pass:
 				color.Set(color.FgGreen)
@@ -310,6 +303,20 @@ func run() error {
 	}
 	fmt.Println()
 
+	for _, f := range failures {
+		color.Set(color.FgBlue)
+		fmt.Printf("%s ", f.file)
+		color.Set(color.FgCyan)
+		fmt.Printf("%s ", f.format)
+		color.Set(color.FgRed)
+		fmt.Println("FAIL")
+		color.Unset()
+		fmt.Println(indent(f.err.Error(), 4))
+	}
+	if len(failures) > 0 {
+		fmt.Println()
+	}
+
 	fmt.Printf("%d tests run", numTests)
 	if numPass > 0 {
 		fmt.Printf(", ")
@@ -362,95 +369,93 @@ type status struct {
 type job struct {
 	file   string
 	format outputFormat
-	result *status
+	result chan status
 }
 
 func (j job) run(wd, exe, dxcPath, xcrunPath string, generateExpected, generateSkip bool) {
-	// Is there an expected output?
+	j.result <- func() status {
-	expected := loadExpectedFile(j.file, j.format)
+		// Is there an expected output?
-	skipped := false
+		expected := loadExpectedFile(j.file, j.format)
-	if strings.HasPrefix(expected, "SKIP") { // Special SKIP token
+		skipped := false
-		skipped = true
+		if strings.HasPrefix(expected, "SKIP") { // Special SKIP token
-	}
+			skipped = true
+		}
 
-	expected = strings.ReplaceAll(expected, "\r\n", "\n")
+		expected = strings.ReplaceAll(expected, "\r\n", "\n")
 
-	file, err := filepath.Rel(wd, j.file)
+		file, err := filepath.Rel(wd, j.file)
-	if err != nil {
+		if err != nil {
-		file = j.file
+			file = j.file
-	}
+		}
 
-	// Make relative paths use forward slash separators (on Windows) so that paths in tint
+		// Make relative paths use forward slash separators (on Windows) so that paths in tint
-	// output match expected output that contain errors
+		// output match expected output that contain errors
-	file = strings.ReplaceAll(file, `\`, `/`)
+		file = strings.ReplaceAll(file, `\`, `/`)
 
-	args := []string{
+		args := []string{
-		file,
+			file,
-		"--format", string(j.format),
+			"--format", string(j.format),
-	}
+		}
 
-	// Can we validate?
+		// Can we validate?
-	validate := false
+		validate := false
-	switch j.format {
+		switch j.format {
-	case spvasm:
+		case spvasm:
-		args = append(args, "--validate") // spirv-val is statically linked, always available
+			args = append(args, "--validate") // spirv-val is statically linked, always available
-		validate = true
-	case hlsl:
-		if dxcPath != "" {
-			args = append(args, "--dxc", dxcPath)
 			validate = true
-		}
+		case hlsl:
-	case msl:
+			if dxcPath != "" {
-		if xcrunPath != "" {
+				args = append(args, "--dxc", dxcPath)
-			args = append(args, "--xcrun", xcrunPath)
+				validate = true
-			validate = true
+			}
-		}
+		case msl:
-	}
+			if xcrunPath != "" {
-
+				args = append(args, "--xcrun", xcrunPath)
-	// Invoke the compiler...
+				validate = true
-	ok, out := invoke(wd, exe, args...)
+			}
-	out = strings.ReplaceAll(out, "\r\n", "\n")
-	matched := expected == "" || expected == out
-
-	if ok && generateExpected && (validate || !skipped) {
-		saveExpectedFile(j.file, j.format, out)
-		matched = true
-	}
-
-	switch {
-	case ok && matched:
-		// Test passed
-		*j.result = status{code: pass}
-		return
-
-		//       --- Below this point the test has failed ---
-
-	case skipped:
-		if generateSkip {
-			saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
-		}
-		*j.result = status{code: skip}
-		return
-
-	case !ok:
-		// Compiler returned non-zero exit code
-		if generateSkip {
-			saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
-		}
-		err := fmt.Errorf("%s", out)
-		*j.result = status{code: fail, err: err}
-		return
-
-	default:
-		// Compiler returned zero exit code, or output was not as expected
-		if generateSkip {
-			saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
 		}
 
-		// Expected output did not match
+		// Invoke the compiler...
-		dmp := diffmatchpatch.New()
+		ok, out := invoke(wd, exe, args...)
-		diff := dmp.DiffPrettyText(dmp.DiffMain(expected, out, true))
+		out = strings.ReplaceAll(out, "\r\n", "\n")
-		err := fmt.Errorf(`Output was not as expected
+		matched := expected == "" || expected == out
+
+		if ok && generateExpected && (validate || !skipped) {
+			saveExpectedFile(j.file, j.format, out)
+			matched = true
+		}
+
+		switch {
+		case ok && matched:
+			// Test passed
+			return status{code: pass}
+
+			//       --- Below this point the test has failed ---
+
+		case skipped:
+			if generateSkip {
+				saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
+			}
+			return status{code: skip}
+
+		case !ok:
+			// Compiler returned non-zero exit code
+			if generateSkip {
+				saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
+			}
+			err := fmt.Errorf("%s", out)
+			return status{code: fail, err: err}
+
+		default:
+			// Compiler returned zero exit code, or output was not as expected
+			if generateSkip {
+				saveExpectedFile(j.file, j.format, "SKIP: FAILED\n\n"+out)
+			}
+
+			// Expected output did not match
+			dmp := diffmatchpatch.New()
+			diff := dmp.DiffPrettyText(dmp.DiffMain(expected, out, true))
+			err := fmt.Errorf(`Output was not as expected
 
 --------------------------------------------------------------------------------
 -- Expected:                                                                  --
@@ -466,10 +471,10 @@ func (j job) run(wd, exe, dxcPath, xcrunPath string, generateExpected, generateS
 -- Diff:                                                                      --
 --------------------------------------------------------------------------------
 %s`,
-			expected, out, diff)
+				expected, out, diff)
-		*j.result = status{code: fail, err: err}
+			return status{code: fail, err: err}
-		return
+		}
-	}
+	}()
 }
 
 // loadExpectedFile loads the expected output file for the test file at 'path'