dots-v2: fix jitteriness, again

This had some previous fixes:
* gotestyourself#368
* gotestyourself#354

These do not fully fix the problem, only mask the issues.

The real issues arise when there are 100s of packages to display, but
only so much screen space. As each one bounces around, the text is
entirely unreadable. The previous approaches was to more effieciently
update the lines to make it more incremental.

The approach here is different - simple but extremely effective. Instead
of trying to constantly write out the entire set of lines/packages under
test, on each event, we keep track of a active and completed set of
packages.

Once a package is completed, we write it out one last time, and then
never overwrite it. The active set is treated as we do today.

This means if we have 8 core machine and 1000 packages, we are only
updating 8 lines at a time, which removes all jitteriness.

internal/dotwriter/writer.go

@@ -5,28 +5,33 @@ terminal.
 package dotwriter
 
 import (
-	"bytes"
 	"io"
 )
 
-// ESC is the ASCII code for escape character
-const ESC = 27
-
 // Writer buffers writes until Flush is called. Flush clears previously written
 // lines before writing new lines from the buffer.
 // The main logic is platform specific, see the related files.
 type Writer struct {
-	out       io.Writer
-	buf       bytes.Buffer
-	lineCount int
+	out             io.Writer
+	inProgressLines int
 }
 
 // New returns a new Writer
 func New(out io.Writer) *Writer {
 	return &Writer{out: out}
 }
 
-// Write saves buf to a buffer
-func (w *Writer) Write(buf []byte) (int, error) {
-	return w.buf.Write(buf)
+func (w *Writer) Write(persistent []string, progressing []string) {
+	defer w.hideCursor()()
+	// Move up to the top of our last output.
+	up := w.inProgressLines
+	w.up(up)
+	for _, lines := range [][]string{persistent, progressing} {
+		for _, l := range lines {
+			w.write([]byte(l))
+			w.clearRest()
+			w.write([]byte{'\n'})
+		}
+	}
+	w.inProgressLines = len(progressing)
 }

internal/dotwriter/writer_posix.go

@@ -4,45 +4,20 @@
 package dotwriter
 
 import (
-	"bytes"
 	"fmt"
 )
 
+// ESC is the ASCII code for escape character
+const ESC = 27
+
 // hide cursor
 var hide = fmt.Sprintf("%c[?25l", ESC)
 
 // show cursor
 var show = fmt.Sprintf("%c[?25h", ESC)
 
-// Flush the buffer, writing all buffered lines to out
-func (w *Writer) Flush() error {
-	if w.buf.Len() == 0 {
-		return nil
-	}
-	// Hide cursor during write to avoid it moving around the screen
-	defer w.hideCursor()()
-
-	// Move up to the top of our last output.
-	w.up(w.lineCount)
-	lines := bytes.Split(w.buf.Bytes(), []byte{'\n'})
-	w.lineCount = len(lines) - 1 // Record how many lines we will write for the next Flush()
-	for i, line := range lines {
-		// For each line, write the contents and clear everything else on the line
-		_, err := w.out.Write(line)
-		if err != nil {
-			return err
-		}
-		w.clearRest()
-		// Add a newline if this isn't the last line
-		if i != len(lines)-1 {
-			_, err := w.out.Write([]byte{'\n'})
-			if err != nil {
-				return err
-			}
-		}
-	}
-	w.buf.Reset()
-	return nil
+func (w *Writer) write(b []byte) {
+	_, _ = w.out.Write(b)
 }
 
 func (w *Writer) up(count int) {

internal/dotwriter/writer_windows.go

@@ -42,8 +42,8 @@ func (w *Writer) Flush() error {
 	if w.buf.Len() == 0 {
 		return nil
 	}
-	w.clearLines(w.lineCount)
-	w.lineCount = bytes.Count(w.buf.Bytes(), []byte{'\n'})
+	w.clearLines(w.inProgressLines)
+	w.inProgressLines = bytes.Count(w.buf.Bytes(), []byte{'\n'})
 	_, err := w.out.Write(w.buf.Bytes())
 	w.buf.Reset()
 	return err

testjson/dotformat.go

@@ -2,6 +2,7 @@ package testjson
 
 import (
 	"bufio"
+	"bytes"
 	"fmt"
 	"io"
 	"os"
@@ -56,6 +57,7 @@ type dotLine struct {
 	runes      int
 	builder    *strings.Builder
 	lastUpdate time.Time
+	terminal   bool
 }
 
 func (l *dotLine) update(dot string) {
@@ -96,6 +98,8 @@ func (d *dotFormatter) Format(event TestEvent, exec *Execution) error {
 	}
 	line := d.pkgs[event.Package]
 	line.lastUpdate = event.Time
+	epkg := exec.Package(event.Package)
+	line.terminal = epkg.action.IsTerminal() || epkg.skipped
 
 	if !event.PackageEvent() {
 		line.update(fmtDot(event))
@@ -105,29 +109,46 @@ func (d *dotFormatter) Format(event TestEvent, exec *Execution) error {
 		return nil
 	}
 
-	// Add an empty header to work around incorrect line counting
-	fmt.Fprint(d.writer, "\n\n")
-
-	sort.Slice(d.order, d.orderByLastUpdated)
+	persistent, progressing := []string{}, []string{}
+	sort.SliceStable(d.order, d.orderByLastUpdated)
 	for _, pkg := range d.order {
-		if d.opts.HideEmptyPackages && exec.Package(pkg).IsEmpty() {
+		p := exec.Package(pkg)
+		if d.opts.HideEmptyPackages && p.IsEmpty() {
 			continue
 		}
-
 		line := d.pkgs[pkg]
+		if line.terminal && pkg != event.Package {
+			// The package is done already, and the event is not for this package.
+			// This means we have already persistently emitted the package once; skip it
+			continue
+		}
+
 		pkgname := RelativePackagePath(pkg) + " "
-		prefix := fmtDotElapsed(exec.Package(pkg))
+		prefix := fmtDotElapsed(p)
 		line.checkWidth(len(prefix+pkgname), d.termWidth)
-		fmt.Fprintf(d.writer, prefix+pkgname+line.builder.String()+"\n")
+		lines := strings.Split(prefix+pkgname+line.builder.String(), "\n")
+
+		if line.terminal {
+			// This should happen exactly once per package, and any future times we filter our the line above
+			// Persist it so we permanently write the final line output
+			persistent = append(persistent, lines...)
+		} else {
+			progressing = append(progressing, lines...)
+		}
 	}
-	PrintSummary(d.writer, exec, SummarizeNone)
-	return d.writer.Flush()
+	buf := &bytes.Buffer{}
+	PrintSummary(buf, exec, SummarizeNone)
+	progressing = append(progressing, strings.Split(buf.String(), "\n")...)
+	d.writer.Write(persistent, progressing)
+	return nil
 }
 
 // orderByLastUpdated so that the most recently updated packages move to the
 // bottom of the list, leaving completed package in the same order at the top.
 func (d *dotFormatter) orderByLastUpdated(i, j int) bool {
-	return d.pkgs[d.order[i]].lastUpdate.Before(d.pkgs[d.order[j]].lastUpdate)
+	iterm := d.pkgs[d.order[i]].terminal
+	jterm := d.pkgs[d.order[j]].terminal
+	return iterm && !jterm
 }
 
 func fmtDotElapsed(p *Package) string {
@@ -139,6 +160,8 @@ func fmtDotElapsed(p *Package) string {
 	switch {
 	case p.cached:
 		return f("🖴 ")
+	case p.skipped:
+		return f("↷")
 	case elapsed <= 0:
 		return f("")
 	case elapsed >= time.Hour:

testjson/execution.go

@@ -109,6 +109,7 @@ type Package struct {
 	// output caused by a test timeout. This is necessary to work around a race
 	// condition in test2json. See https://github.com/golang/go/issues/57305.
 	testTimeoutPanicInTest string
+	skipped                bool
 }
 
 // Result returns if the package passed, failed, or was skipped because there
@@ -371,6 +372,8 @@ func (p *Package) addEvent(event TestEvent) {
 	case ActionPass, ActionFail:
 		p.action = event.Action
 		p.elapsed = elapsedDuration(event.Elapsed)
+	case ActionSkip:
+		p.skipped = true
 	case ActionOutput:
 		if coverage, ok := isCoverageOutput(event.Output); ok {
 			p.coverage = coverage