Overnight/test harness 2026 04 20

internal/engine/persist_no_sess_test.go

@@ -13,8 +13,12 @@ import (
 // arm at the top of persistAdd, persistState, upgradeMagnetSession,
 // and RestoreSession (engine.go:1325, 1363, 1407, 1460). New()
 // always populates e.sess, but the explicit guards future-proof
-// callers that might construct an engine without one. Reach the
-// arms via the internal entry points after force-clearing e.sess.
+// callers that might construct an engine without one.
+//
+// Each function tests `if e.sess == nil { return }` BEFORE touching
+// the *Handle argument, so we can safely pass nil. This avoids
+// AddMagnet/AddInfoHash, both of which spawn upgradeMagnetSession
+// goroutines that race with the e.sess = nil write under -race.
 func TestPersistFnsNoSessReturnEarly(t *testing.T) {
 	t.Parallel()
 	cfg := config.Default()
@@ -34,27 +38,19 @@ func TestPersistFnsNoSessReturnEarly(t *testing.T) {
 	}
 	defer eng.Close()
 
-	const magnet = "magnet:?xt=urn:btih:1111111111111111111111111111111111111111"
-	h, err := eng.AddMagnet(magnet)
-	if err != nil {
-		t.Fatalf("AddMagnet: %v", err)
-	}
-
-	// Force-clear sess so the nil-sess guards short-circuit.
+	// Force-clear sess so the nil-sess guards short-circuit. No
+	// goroutines reading e.sess exist yet because we haven't
+	// added any magnets/infohashes.
+	eng.mu.Lock()
 	eng.sess = nil
+	eng.mu.Unlock()
 
-	// persistAdd: void return; success means it didn't panic.
-	eng.persistAdd(h, "magnet", magnet, "")
-
-	// persistState: same.
-	eng.persistState(h)
-
-	// RestoreSession: returns nil on nil-sess.
+	// All four functions check `if e.sess == nil` first; passing
+	// nil h is safe because they short-circuit before touching it.
+	eng.persistAdd(nil, "magnet", "magnet:?xt=urn:btih:00", "")
+	eng.persistState(nil)
 	if err := eng.RestoreSession(); err != nil {
 		t.Errorf("RestoreSession with nil sess = %v, want nil", err)
 	}
-
-	// upgradeMagnetSession is a goroutine entrypoint that returns
-	// immediately on nil-sess. Calling synchronously is safe.
-	eng.upgradeMagnetSession(h)
+	eng.upgradeMagnetSession(nil)
 }

internal/gui/app_loops_canceled_ctx_test.go

@@ -0,0 +1,31 @@
+package gui
+
+import (
+	"context"
+	"testing"
+)
+
+// TestTitleLoopReturnsOnContextCancel covers titleLoop's
+// `case <-ctx.Done(): return` arm at app.go:337-338. With an
+// already-canceled context the select picks ctx.Done before
+// any tick fires; a.daemon.Eng.* is never dereferenced, so a
+// daemon-less App with just a version field is sufficient.
+func TestTitleLoopReturnsOnContextCancel(t *testing.T) {
+	t.Parallel()
+	a := &App{version: "test"}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	a.titleLoop(ctx)
+}
+
+// TestNotificationLoopReturnsOnContextCancel covers
+// notificationLoop's `case <-ctx.Done(): return` arm at
+// app.go:372-373. Same canceled-ctx pattern; the daemon stays
+// untouched.
+func TestNotificationLoopReturnsOnContextCancel(t *testing.T) {
+	t.Parallel()
+	a := &App{lastNotified: make(map[string]bool)}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	a.notificationLoop(ctx)
+}

internal/gui/app_loops_tick_test.go

@@ -0,0 +1,50 @@
+package gui
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"fyne.io/fyne/v2/test"
+	"fyne.io/fyne/v2/widget"
+)
+
+// TestTitleLoopTickAndNotificationLoopTick covers titleLoop's
+// tick.C arm at app.go:339-360 and notificationLoop's tick.C
+// arm at app.go:374-385 against a real daemon. Both loops fetch
+// TorrentSnapshots under their respective tick cadences (2s for
+// title, 3s for notification). We let them tick once then
+// cancel. Combined into one test to keep total wall time below
+// ~3.5s.
+func TestTitleLoopTickAndNotificationLoopTick(t *testing.T) {
+	app := test.NewApp()
+	defer app.Quit()
+	w := app.NewWindow("title-test")
+	defer w.Close()
+	w.SetContent(widget.NewLabel("anchor"))
+
+	d := newTestDaemon(t)
+	// Add a torrent so titleLoop's totalDown/totalUp aggregation
+	// has rows to walk and notificationLoop's snapshot range
+	// loop body fires.
+	addTestTorrent(t, d.Eng)
+
+	a := &App{
+		fyne:         app,
+		win:          w,
+		daemon:       d,
+		version:      "test",
+		lastNotified: make(map[string]bool),
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	go a.titleLoop(ctx)
+	go a.notificationLoop(ctx)
+
+	// Wait long enough for titleLoop (2s) and notificationLoop
+	// (3s) to fire at least once each.
+	time.Sleep(3200 * time.Millisecond)
+	cancel()
+	// Brief drain so goroutines exit before the test returns.
+	time.Sleep(50 * time.Millisecond)
+}

internal/gui/app_methods_test.go

@@ -0,0 +1,75 @@
+package gui
+
+import (
+	"context"
+	"testing"
+
+	"fyne.io/fyne/v2/container"
+	"fyne.io/fyne/v2/test"
+	"fyne.io/fyne/v2/widget"
+)
+
+// TestAppSelectTabDispatchesByName covers SelectTab's switch on
+// each accepted tab name (downloads/search/status/companion/
+// settings, both lowercase and capitalised). An App with a
+// pre-populated tabs container is enough — no daemon needed.
+func TestAppSelectTabDispatchesByName(t *testing.T) {
+	app := test.NewApp()
+	defer app.Quit()
+
+	tabs := container.NewAppTabs(
+		container.NewTabItem("Downloads", widget.NewLabel("0")),
+		container.NewTabItem("Search", widget.NewLabel("1")),
+		container.NewTabItem("Status", widget.NewLabel("2")),
+		container.NewTabItem("Companion", widget.NewLabel("3")),
+		container.NewTabItem("Settings", widget.NewLabel("4")),
+	)
+	a := &App{tabs: tabs}
+
+	cases := []struct {
+		name string
+		want int
+	}{
+		{"downloads", 0}, {"Downloads", 0},
+		{"search", 1}, {"Search", 1},
+		{"status", 2}, {"Status", 2},
+		{"companion", 3}, {"Companion", 3},
+		{"settings", 4}, {"Settings", 4},
+	}
+	for _, c := range cases {
+		a.SelectTab(c.name)
+		if got := tabs.SelectedIndex(); got != c.want {
+			t.Errorf("SelectTab(%q) → index %d, want %d", c.name, got, c.want)
+		}
+	}
+
+	// Unknown name should leave the selection where it was.
+	a.SelectTab("unknown")
+	if got := tabs.SelectedIndex(); got != 4 {
+		t.Errorf("SelectTab('unknown') changed selection to %d, want 4 (unchanged)", got)
+	}
+}
+
+// TestAppSelectTabNilTabsShortCircuits covers SelectTab's
+// `if a.tabs == nil { return }` arm at app.go:393-395.
+func TestAppSelectTabNilTabsShortCircuits(t *testing.T) {
+	t.Parallel()
+	a := &App{}
+	a.SelectTab("downloads") // must not panic
+}
+
+// TestAppCleanupCallsCancel covers Cleanup's `a.cancel()` call.
+// Construct an App with a real context and verify the context is
+// canceled after Cleanup returns.
+func TestAppCleanupCallsCancel(t *testing.T) {
+	t.Parallel()
+	ctx, cancel := context.WithCancel(context.Background())
+	a := &App{cancel: cancel}
+	a.Cleanup()
+	select {
+	case <-ctx.Done():
+		// good
+	default:
+		t.Error("Cleanup did not cancel the context")
+	}
+}

internal/gui/apply_settings_validation_test.go

@@ -0,0 +1,93 @@
+package gui
+
+import (
+	"testing"
+
+	"fyne.io/fyne/v2/test"
+	"fyne.io/fyne/v2/widget"
+)
+
+// TestApplyQueueSettingsInvalidInput covers applyQueueSettings'
+// validation arm at settings.go:117-121. A non-numeric input
+// trips strconv.Atoi err and the function calls dialog.ShowError
+// + return before reaching st.d.Eng. Requires only a content
+// label and an anchor window for st.win() to resolve.
+func TestApplyQueueSettingsInvalidInput(t *testing.T) {
+	app := test.NewApp()
+	defer app.Quit()
+	w := app.NewWindow("anchor")
+	defer w.Close()
+	w.SetContent(widget.NewLabel("anchor"))
+
+	st := &settingsTab{
+		content:        widget.NewLabel("settings"),
+		maxActiveEntry: widget.NewEntry(),
+	}
+	st.maxActiveEntry.SetText("not a number")
+	st.applyQueueSettings()
+
+	// Negative-number arm — Atoi parses but n < 0 trips the
+	// `n < 0` half of the same condition.
+	st.maxActiveEntry.SetText("-5")
+	st.applyQueueSettings()
+}
+
+// TestApplyQueueSettingsZeroAndEmpty covers applyQueueSettings'
+// `if s == ""` (empty input → s = "0") arm and `if n == 0`
+// (label = "unlimited") arm at settings.go:114-128. Both are
+// daemon-touching arms — they call SetMaxActiveDownloads — so
+// the test requires a real daemon.
+func TestApplyQueueSettingsZeroAndEmpty(t *testing.T) {
+	app := test.NewApp()
+	defer app.Quit()
+	w := app.NewWindow("anchor")
+	defer w.Close()
+	w.SetContent(widget.NewLabel("anchor"))
+
+	d := newTestDaemon(t)
+	st := newSettingsTab(d)
+
+	// Empty input: TrimSpace returns "", trips the `s == ""`
+	// fallback to "0", then `n == 0` triggers the "unlimited"
+	// label arm.
+	st.maxActiveEntry.SetText("")
+	st.applyQueueSettings()
+
+	// Explicit "0" — already triggers the "unlimited" arm; left
+	// here as a redundant check that the empty-string handling
+	// converges on the same path.
+	st.maxActiveEntry.SetText("0")
+	st.applyQueueSettings()
+}
+
+// TestApplyRateLimitsInvalidInput covers applyRateLimits'
+// upload-err and download-err validation arms at
+// settings.go:139-148. Each malformed input trips parseKiB and
+// causes ShowError + return before any daemon call.
+func TestApplyRateLimitsInvalidInput(t *testing.T) {
+	app := test.NewApp()
+	defer app.Quit()
+	w := app.NewWindow("anchor")
+	defer w.Close()
+	w.SetContent(widget.NewLabel("anchor"))
+
+	// Upload err arm — bad upload, anything in download.
+	stUp := &settingsTab{
+		content:       widget.NewLabel("settings"),
+		uploadEntry:   widget.NewEntry(),
+		downloadEntry: widget.NewEntry(),
+	}
+	stUp.uploadEntry.SetText("abc")
+	stUp.downloadEntry.SetText("100")
+	stUp.applyRateLimits()
+
+	// Download err arm — valid upload, bad download.
+	stDown := &settingsTab{
+		content:       widget.NewLabel("settings"),
+		uploadEntry:   widget.NewEntry(),
+		downloadEntry: widget.NewEntry(),
+	}
+	stDown.uploadEntry.SetText("100")
+	stDown.downloadEntry.SetText("xyz")
+	stDown.applyRateLimits()
+}