fix(lab): a failed run stops the Lab's sweep, a run cancelled while starting is cancelled on the server, and diff rows keep their order

cli/tui/lab.go

@@ -4,8 +4,10 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -213,16 +215,7 @@ func (m LabModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 					// Fire the server-side cancel as a command. It used to run
 					// synchronously HERE, inside Update — freezing the entire
 					// TUI for up to the 30s client timeout.
-					var cancelCmd tea.Cmd
-					if id := m.runTestID; id != "" {
-						c := m.client
-						cancelCmd = func() tea.Msg {
-							ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-							defer cancel()
-							_ = c.CancelTest(ctx, id)
-							return nil
-						}
-					}
+					cancelCmd := m.cancelRun(m.runTestID)
 					m.running = false
 					m.runTestID = ""
 					m.sweepActive = false
@@ -395,7 +388,9 @@ func (m LabModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
 
 	case labTestStartedMsg:
 		if !m.running {
-			return m, nil // cancelled while the test was being created
+			// Cancelled while the test was being created. The run the API
+			// started anyway used to be dropped here, and ran on unwatched.
+			return m, m.cancelRun(msg.run.ID)
 		}
 		m.runTestID = msg.run.ID
 		m.lastTestReq = msg.req
@@ -991,8 +986,9 @@ func (m LabModel) viewDiff(width int) string {
 			paramSepW = 10
 		}
 		sb.WriteString(dimStyle.Render("  "+strings.Repeat("─", paramSepW)) + "\n")
-		for key, aVal := range a.Params {
-			bVal := b.Params[key]
+		// In key order: ranging over the map reordered the rows every frame.
+		for _, key := range slices.Sorted(maps.Keys(a.Params)) {
+			aVal, bVal := a.Params[key], b.Params[key]
 			if aVal != bVal {
 				sb.WriteString(fmt.Sprintf("  %-*s %s → %s\n",
 					colW,
@@ -1044,6 +1040,20 @@ func (m LabModel) currentParams() map[string]string {
 	return params
 }
 
+// cancelRun cancels run id on the server, as a command: nil for no run.
+func (m LabModel) cancelRun(id string) tea.Cmd {
+	if id == "" {
+		return nil
+	}
+	c := m.client
+	return func() tea.Msg {
+		ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+		defer cancel()
+		_ = c.CancelTest(ctx, id)
+		return nil
+	}
+}
+
 // runTest and retryTest run as tea.Cmd goroutines against a copy of the
 // model, so they must not assign model fields — anything the UI needs back
 // (run ID, request) travels in labTestStartedMsg and is applied in Update.
@@ -1112,7 +1122,12 @@ func (m LabModel) pollTestOnce(testID string, deadline time.Time, maxWait time.D
 		}
 
 		status := strings.ToUpper(updated.Status)
-		if status == "DONE" || status == "COMPLETED" || status == "FAILED" || status == "ERROR" {
+		// A run that failed measured nothing. Taken as a result, it became a
+		// 0 rec/s iteration, and a sweep went on to its next, doomed step.
+		if status == "FAILED" || status == "ERROR" {
+			return labTestDoneMsg{testID: testID, err: fmt.Errorf("test %s ended %s", testID, status)}
+		}
+		if status == "DONE" || status == "COMPLETED" {
 			summary, _ := m.client.ReportSummary(context.Background(), testID)
 			return labTestDoneMsg{testID: testID, run: updated, summary: summary}
 		}

cli/tui/lab_test.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/bmscomp/kates/cli/client"
 	tea "github.com/charmbracelet/bubbletea"
@@ -312,21 +313,86 @@ func TestLabTestStartedMsg(t *testing.T) {
 		}
 	})
 
-	t.Run("dropped when the run was cancelled during creation", func(t *testing.T) {
-		m := NewLab(nil, "")
+	// x while CreateTest was in flight dropped the run the API then started,
+	// and it ran on with nothing watching it.
+	t.Run("cancelled on the server when the run was cancelled during creation", func(t *testing.T) {
+		var cancels []string
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			cancels = append(cancels, r.Method+" "+r.URL.Path)
+			w.Header().Set("Content-Type", "application/json")
+			_, _ = w.Write([]byte(`{}`))
+		}))
+		t.Cleanup(srv.Close)
+		m := NewLab(client.New(srv.URL), "")
 		m, cmd := updateLab(t, m, msg)
 		if m.runTestID != "" {
 			t.Errorf("runTestID = %q, want empty", m.runTestID)
 		}
 		if m.lastTestReq != nil {
 			t.Error("lastTestReq set from a dropped message")
 		}
-		if cmd != nil {
-			t.Error("cmd = non-nil, want nil")
+		if cmd == nil {
+			t.Fatal("cmd = nil, want the server-side cancel of run-42")
+		}
+		cmd()
+		if strings.Join(cancels, ",") != "POST /api/tests/run-42/cancel" {
+			t.Errorf("requests = %q, want the cancel of run-42", cancels)
 		}
 	})
 }
 
+// A FAILED or ERROR run came back as a result: a sweep recorded a 0 rec/s
+// iteration and launched its next step against the same failure.
+func TestLabFailedRunStopsTheSweep(t *testing.T) {
+	for _, status := range []string{"FAILED", "ERROR"} {
+		t.Run(status, func(t *testing.T) {
+			t.Parallel()
+			m := newRunningLab(t, newLabServerClient(t, `{"id":"t1","status":"`+status+`"}`))
+			m.sweepActive = true
+			m.sweepParam = 1
+			m.params[1].Current = 0
+
+			done := m.pollTestOnce("t1", time.Now().Add(time.Minute), time.Minute)()
+			if d, ok := done.(labTestDoneMsg); !ok || d.err == nil {
+				t.Fatalf("poll = %#v, want a labTestDoneMsg with an error", done)
+			}
+			m, cmd := updateLab(t, m, done)
+			if len(m.iterations) != 0 || m.sweepActive || m.running || cmd != nil {
+				t.Errorf("iterations %d, sweep %v, running %v, cmd %v: want no iteration and the sweep stopped",
+					len(m.iterations), m.sweepActive, m.running, cmd != nil)
+			}
+			if !strings.Contains(m.status, status) {
+				t.Errorf("status %q does not say the run ended %s", m.status, status)
+			}
+		})
+	}
+}
+
+// The parameter rows of the diff came from ranging over a map, so they changed
+// order from one frame to the next.
+func TestLabDiffParameterRowsKeepTheirOrder(t *testing.T) {
+	m := NewLab(nil, "")
+	m.width, m.height = 160, 50
+	m.iterations = []labIteration{
+		{Number: 1, Params: map[string]string{"acks": "1", "batchSize": "16384", "compression": "lz4", "lingerMs": "0", "producers": "4"}},
+		{Number: 2, Params: map[string]string{"acks": "all", "batchSize": "65536", "compression": "zstd", "lingerMs": "5", "producers": "8"}},
+	}
+	first := m.viewDiff(160)
+	for range 20 {
+		if got := m.viewDiff(160); got != first {
+			t.Fatalf("the diff changed between frames:\n%s\n---\n%s", first, got)
+		}
+	}
+	last := -1
+	for _, key := range []string{"acks", "batchSize", "compression", "lingerMs", "producers"} {
+		at := strings.Index(first, key)
+		if at < last {
+			t.Errorf("%s is not after the keys before it in sorted order:\n%s", key, first)
+		}
+		last = at
+	}
+}
+
 func TestLabTestDoneMsg(t *testing.T) {
 	run := &client.TestRun{
 		ID: "t1",