Fix tests, as they weren't actually checking DeviceLists

state/device_data_table_test.go

@@ -1,8 +1,6 @@
 package state
 
 import (
-	"bytes"
-	"encoding/json"
 	"reflect"
 	"testing"
 
@@ -12,7 +10,7 @@ import (
 func assertVal(t *testing.T, msg string, got, want interface{}) {
 	t.Helper()
 	if !reflect.DeepEqual(got, want) {
-		t.Errorf("%s: got %v want %v", msg, got, want)
+		t.Errorf("%s: got\n%#v want\n%#v", msg, got, want)
 	}
 }
 
@@ -23,54 +21,7 @@ func assertDeviceData(t *testing.T, g, w internal.DeviceData) {
 	assertVal(t, "FallbackKeyTypes", g.FallbackKeyTypes, w.FallbackKeyTypes)
 	assertVal(t, "OTKCounts", g.OTKCounts, w.OTKCounts)
 	assertVal(t, "ChangedBits", g.ChangedBits, w.ChangedBits)
-}
-
-func BenchmarkReflectDeepEqual(b *testing.B) {
-	newList := map[string]int{"abc": 1, "cde": 1, "fgh": 1, "ijk": 1, "lmn": 1}
-	d1 := internal.DeviceData{}
-	d2 := internal.DeviceData{
-		DeviceLists: internal.DeviceLists{New: newList},
-		DeviceID:    "abc",
-		UserID:      "abc",
-		ChangedBits: 0,
-		OTKCounts:   map[string]int{"ed25519": 50},
-	}
-
-	for i := 0; i < b.N; i++ {
-		if reflect.DeepEqual(d1, d2) {
-			b.Fatal("structs are equal")
-		}
-	}
-}
-
-func BenchmarkJSONMarhsalBytesEqual(b *testing.B) {
-	newList := map[string]int{"abc": 1, "cde": 1, "fgh": 1, "ijk": 1, "lmn": 1}
-	d1 := internal.DeviceData{}
-	d2 := internal.DeviceData{
-		DeviceLists: internal.DeviceLists{New: newList},
-		DeviceID:    "abc",
-		UserID:      "abc",
-		ChangedBits: 0,
-		OTKCounts:   map[string]int{"ed25519": 50},
-	}
-
-	data1, err := json.Marshal(d1)
-	if err != nil {
-		b.Fatal(err)
-	}
-
-	// Reset the timer, so we actually just test what is
-	// executed after we fetched the data from the database.
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		data2, err := json.Marshal(d2)
-		if err != nil {
-			b.Fatal(err)
-		}
-		if bytes.Equal(data1, data2) {
-			b.Fatal("bytes are equal")
-		}
-	}
+	assertVal(t, "DeviceLists", g.DeviceLists, w.DeviceLists)
 }
 
 func TestDeviceDataTableSwaps(t *testing.T) {
@@ -126,7 +77,8 @@ func TestDeviceDataTableSwaps(t *testing.T) {
 		},
 		FallbackKeyTypes: []string{"foobar"},
 		DeviceLists: internal.DeviceLists{
-			New: internal.ToDeviceListChangesMap([]string{"alice", "bob"}, nil),
+			New:  internal.ToDeviceListChangesMap([]string{"alice", "bob"}, nil),
+			Sent: map[string]int{},
 		},
 	}
 	want.SetFallbackKeysChanged()
@@ -137,36 +89,41 @@ func TestDeviceDataTableSwaps(t *testing.T) {
 		assertNoError(t, err)
 		assertDeviceData(t, *got, want)
 	}
-	// now swap-er-roo
+	// now swap-er-roo, at this point we still expect the "old" data,
+	// as it is the first time we swap
 	got, err := table.Select(userID, deviceID, true)
 	assertNoError(t, err)
+	assertDeviceData(t, *got, want)
+
+	// changed bits were reset when we swapped
 	want2 := want
 	want2.DeviceLists = internal.DeviceLists{
-		Sent: internal.ToDeviceListChangesMap([]string{"alice"}, nil),
-		New:  nil,
+		Sent: internal.ToDeviceListChangesMap([]string{"alice", "bob"}, nil),
+		New:  map[string]int{},
 	}
-	assertDeviceData(t, *got, want2)
-
-	// changed bits were reset when we swapped
 	want2.ChangedBits = 0
 	want.ChangedBits = 0
 
-	// this is permanent, read-only views show this too
-	got, err = table.Select(userID, deviceID, false)
+	// this is permanent, read-only views show this too.
+	// Since we have swapped previously, we now expect New to be empty
+	// and Sent to be set. Swap again to clear Sent.
+	got, err = table.Select(userID, deviceID, true)
 	assertNoError(t, err)
 	assertDeviceData(t, *got, want2)
 
-	// another swap causes sent to be cleared out
-	got, err = table.Select(userID, deviceID, true)
+	// We now expect empty DeviceLists, as we swapped twice.
+	got, err = table.Select(userID, deviceID, false)
 	assertNoError(t, err)
 	want3 := want2
 	want3.DeviceLists = internal.DeviceLists{
-		Sent: nil,
-		New:  nil,
+		Sent: map[string]int{},
+		New:  map[string]int{},
 	}
 	assertDeviceData(t, *got, want3)
 
 	// get back the original state
+	//err = table.DeleteDevice(userID, deviceID)
+	assertNoError(t, err)
 	for _, dd := range deltas {
 		err = table.Upsert(&dd)
 		assertNoError(t, err)
@@ -178,6 +135,7 @@ func TestDeviceDataTableSwaps(t *testing.T) {
 	assertDeviceData(t, *got, want)
 
 	// swap once then add once so both sent and new are populated
+	// Moves Alice and Bob to Sent
 	_, err = table.Select(userID, deviceID, true)
 	assertNoError(t, err)
 	err = table.Upsert(&internal.DeviceData{
@@ -193,14 +151,16 @@ func TestDeviceDataTableSwaps(t *testing.T) {
 
 	want4 := want
 	want4.DeviceLists = internal.DeviceLists{
-		Sent: internal.ToDeviceListChangesMap([]string{"alice"}, nil),
+		Sent: internal.ToDeviceListChangesMap([]string{"alice", "bob"}, nil),
 		New:  internal.ToDeviceListChangesMap([]string{"bob"}, []string{"charlie"}),
 	}
+	// Without swapping, we expect Alice and Bob in Sent, and Bob and Charlie in New
 	got, err = table.Select(userID, deviceID, false)
 	assertNoError(t, err)
 	assertDeviceData(t, *got, want4)
 
 	// another append then consume
+	// This results in dave to be added to Sent
 	err = table.Upsert(&internal.DeviceData{
 		UserID:   userID,
 		DeviceID: deviceID,
@@ -213,8 +173,18 @@ func TestDeviceDataTableSwaps(t *testing.T) {
 	assertNoError(t, err)
 	want5 := want4
 	want5.DeviceLists = internal.DeviceLists{
-		Sent: internal.ToDeviceListChangesMap([]string{"bob", "dave"}, []string{"charlie", "dave"}),
-		New:  nil,
+		Sent: internal.ToDeviceListChangesMap([]string{"alice", "bob"}, nil),
+		New:  internal.ToDeviceListChangesMap([]string{"bob"}, []string{"charlie", "dave"}),
+	}
+	assertDeviceData(t, *got, want5)
+
+	// Swapping again clears New
+	got, err = table.Select(userID, deviceID, true)
+	assertNoError(t, err)
+	want5 = want4
+	want5.DeviceLists = internal.DeviceLists{
+		Sent: internal.ToDeviceListChangesMap([]string{"bob"}, []string{"charlie", "dave"}),
+		New:  map[string]int{},
 	}
 	assertDeviceData(t, *got, want5)
 
@@ -240,11 +210,13 @@ func TestDeviceDataTableBitset(t *testing.T) {
 			"foo": 100,
 			"bar": 92,
 		},
+		DeviceLists: internal.DeviceLists{New: map[string]int{}, Sent: map[string]int{}},
 	}
 	fallbakKeyUpdate := internal.DeviceData{
 		UserID:           userID,
 		DeviceID:         deviceID,
 		FallbackKeyTypes: []string{"foo", "bar"},
+		DeviceLists:      internal.DeviceLists{New: map[string]int{}, Sent: map[string]int{}},
 	}
 	bothUpdate := internal.DeviceData{
 		UserID:           userID,
@@ -253,6 +225,7 @@ func TestDeviceDataTableBitset(t *testing.T) {
 		OTKCounts: map[string]int{
 			"both": 100,
 		},
+		DeviceLists: internal.DeviceLists{New: map[string]int{}, Sent: map[string]int{}},
 	}
 
 	err := table.Upsert(&otkUpdate)