Reproduce duplicate connections bug in Mock backend

internal/tui/reproduce_test.go

@@ -0,0 +1,80 @@
+package tui
+
+import (
+	"testing"
+
+	"github.com/shazow/wifitui/wifi"
+	"github.com/shazow/wifitui/wifi/mock"
+)
+
+func TestDuplicateEntriesInList(t *testing.T) {
+	// 1. Setup Backend with duplicates
+	b, err := mock.New()
+	if err != nil {
+		t.Fatalf("failed to create mock backend: %v", err)
+	}
+	mb := b.(*mock.MockBackend)
+	// Configure specific duplicates
+	ssid := "DupNet"
+	mb.VisibleConnections = []wifi.Connection{
+		{SSID: ssid, Strength: 50, IsActive: true},
+		{SSID: ssid, Strength: 80, IsActive: true},
+	}
+	// Ensure no delay for tests
+	mb.ActionSleep = 0
+
+	// 2. Init Model
+	m, err := NewModel(mb)
+	if err != nil {
+		t.Fatalf("NewModel failed: %v", err)
+	}
+
+	// 3. Trigger Scan (Initial)
+	conns, err := mb.BuildNetworkList(true)
+	if err != nil {
+		t.Fatalf("BuildNetworkList failed: %v", err)
+	}
+	msg := scanFinishedMsg(conns)
+	_, _ = m.Update(msg)
+
+	// Helper function to check for duplicates
+	checkForDuplicates := func(label string) {
+		top := m.stack.Top()
+		lm, ok := top.(*ListModel)
+		if !ok {
+			t.Fatalf("[%s] Top component is not ListModel, got %T", label, top)
+		}
+		items := lm.list.Items()
+		count := 0
+		for _, item := range items {
+			ci, ok := item.(connectionItem)
+			if !ok {
+				continue
+			}
+			if ci.SSID == ssid {
+				count++
+			}
+		}
+		if count < 2 {
+			t.Errorf("[%s] Expected at least 2 connections for SSID %q, got %d", label, ssid, count)
+		} else {
+			t.Logf("[%s] Found %d duplicates for SSID %q", label, count, ssid)
+		}
+	}
+
+	// Verify duplicates after first scan
+	checkForDuplicates("First Scan")
+
+	// 4. Trigger Rescan
+	// In the real app, this happens via a timer or keypress. Here we simulate the result.
+	// Since we disabled randomization, the strengths should remain 50 and 80.
+	conns2, err := mb.BuildNetworkList(true)
+	if err != nil {
+		t.Fatalf("BuildNetworkList (Rescan) failed: %v", err)
+	}
+	msg2 := scanFinishedMsg(conns2)
+	_, _ = m.Update(msg2)
+
+	// Verify duplicates persist after rescan
+	checkForDuplicates("Rescan")
+}

wifi/mock/mock.go

@@ -2,7 +2,7 @@ package mock
 
 import (
 	"fmt"
-	"math/rand"
+	//"math/rand"
 	"time"
 
 	"github.com/shazow/wifitui/wifi"
@@ -118,50 +118,70 @@ func (m *MockBackend) BuildNetworkList(shouldScan bool) ([]wifi.Connection, erro
 		return nil, wifi.ErrWirelessDisabled
 	}
 	// For mock, we can re-randomize strengths on each scan
-	if shouldScan {
-		s := rand.NewSource(time.Now().Unix())
-		r := rand.New(s)
-		for i := range m.VisibleConnections {
-			if m.VisibleConnections[i].Strength > 0 {
-				m.VisibleConnections[i].Strength = uint8(r.Intn(70) + 30)
-			}
-		}
-	}
-
-	// Build a unified list of connections, de-duplicating known networks.
-	unified := make(map[string]wifi.Connection)
+	// if shouldScan {
+	// 	s := rand.NewSource(time.Now().Unix())
+	// 	r := rand.New(s)
+	// 	for i := range m.VisibleConnections {
+	// 		if m.VisibleConnections[i].Strength > 0 {
+	// 			m.VisibleConnections[i].Strength = uint8(r.Intn(70) + 30)
+	// 		}
+	// 	}
+	// }
+
+	// Reproduce bug from networkmanager backend:
+	// Iterate through visible connections and append them to the result list,
+	// potentially creating duplicates if multiple APs with the same SSID exist.
+	// We still maintain a map to simulate the "updating best AP" part of the bug,
+	// but we mistakenly append to the result list every time we see a new or better AP.
+
+	processed := make(map[string]wifi.Connection)
+	var result []wifi.Connection
 
-	// Add all visible connections first.
+	// Process visible connections with the buggy logic
 	for _, c := range m.VisibleConnections {
-		unified[c.SSID] = c
-	}
-
-	// Add/overwrite with known connections to ensure they are in the list.
-	for _, kc := range m.KnownConnections {
-		conn := kc.Connection
-		if visibleConn, ok := unified[conn.SSID]; ok {
-			conn.Strength = visibleConn.Strength
+		if existing, ok := processed[c.SSID]; ok {
+			if c.Strength <= existing.Strength {
+				continue
+			}
 		}
-		unified[conn.SSID] = conn
-	}
+		processed[c.SSID] = c
 
-	// Convert map back to a slice for the return value.
-	var result []wifi.Connection
-	for _, c := range unified {
-		// IsActive is now stored on the connection object itself.
-		// We still need to determine IsKnown for networks that might only be in the visible list.
+		// This is the bug: we append to the result list even if we've already added this SSID.
+		// In the real bug, this happened because the append was inside the loop over APs.
+		// We need to ensure we populate IsKnown correctly here too.
+
+		connToAdd := c
 		isKnown := false
 		for _, kc := range m.KnownConnections {
 			if kc.SSID == c.SSID {
 				isKnown = true
+				// Merge known connection details
+				knownConn := kc.Connection
+				connToAdd.IsKnown = true
+				connToAdd.AutoConnect = knownConn.AutoConnect
+				connToAdd.Security = knownConn.Security
+				connToAdd.LastConnected = knownConn.LastConnected
+				// Preserve the visible strength and active status
+				// connToAdd.Strength = c.Strength (already set)
+				// connToAdd.IsActive = c.IsActive (already set)
 				break
 			}
 		}
-		c.IsKnown = isKnown
 		if !isKnown {
-			c.AutoConnect = false
+			connToAdd.IsKnown = false
+			connToAdd.AutoConnect = false
+		}
+
+		result = append(result, connToAdd)
+	}
+
+	// Also ensure known connections that weren't visible are added (if that's desired behavior).
+	// The original mock logic added known connections even if not visible.
+	for _, kc := range m.KnownConnections {
+		if _, ok := processed[kc.SSID]; !ok {
+			result = append(result, kc.Connection)
+			processed[kc.SSID] = kc.Connection
 		}
-		result = append(result, c)
 	}
 
 	return result, nil