From 5696d6d7eae163adf01fba9d90f0bd9fdbbf0bbb Mon Sep 17 00:00:00 2001
From: Martin Hutchinson <mhutchinson@gmail.com>
Date: Mon, 4 Dec 2023 16:06:12 +0000
Subject: [PATCH] Test both revisioned and unrevisioned subtrees

---
 storage/mysql/storage_test.go            | 103 ++++++++++++++---------
 storage/testonly/admin_storage_tester.go |  10 +++
 2 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/storage/mysql/storage_test.go b/storage/mysql/storage_test.go
index 13479c1445..5aa0d0e867 100644
--- a/storage/mysql/storage_test.go
+++ b/storage/mysql/storage_test.go
@@ -58,11 +58,11 @@ func TestNodeRoundTrip(t *testing.T) {
 		{desc: "store-all-read-all", store: nodes, read: nodeIDs, want: nodes},
 		{desc: "store-all-read-none", store: nodes, read: nil, want: nil},
 	} {
-		t.Run(tc.desc, func(t *testing.T) {
+		testbody := func(treeDef *trillian.Tree) {
 			ctx := context.Background()
 			cleanTestDB(DB)
 			as := NewAdminStorage(DB)
-			tree := mustCreateTree(ctx, t, as, storageto.LogTree)
+			tree := mustCreateTree(ctx, t, as, treeDef)
 			s := NewLogStorage(DB, nil)
 
 			const writeRev = int64(100)
@@ -86,6 +86,12 @@ func TestNodeRoundTrip(t *testing.T) {
 				}
 				return nil
 			})
+		}
+		t.Run(tc.desc+"-revisions", func(t *testing.T) {
+			testbody(storageto.LogTree)
+		})
+		t.Run(tc.desc+"-norevisions", func(t *testing.T) {
+			testbody(storageto.RevisionlessLogTree)
 		})
 	}
 }
@@ -93,50 +99,67 @@ func TestNodeRoundTrip(t *testing.T) {
 // This test ensures that node writes cross subtree boundaries so this edge case in the subtree
 // cache gets exercised. Any tree size > 256 will do this.
 func TestLogNodeRoundTripMultiSubtree(t *testing.T) {
-	ctx := context.Background()
-	cleanTestDB(DB)
-	as := NewAdminStorage(DB)
-	tree := mustCreateTree(ctx, t, as, storageto.LogTree)
-	s := NewLogStorage(DB, nil)
-
-	const writeRev = int64(100)
-	const size = 871
-	nodesToStore, err := createLogNodesForTreeAtSize(t, size, writeRev)
-	if err != nil {
-		t.Fatalf("failed to create test tree: %v", err)
-	}
-	nodeIDsToRead := make([]compact.NodeID, len(nodesToStore))
-	for i := range nodesToStore {
-		nodeIDsToRead[i] = nodesToStore[i].ID
+	testCases := []struct {
+		desc string
+		tree *trillian.Tree
+	}{
+		{
+			desc: "Revisions",
+			tree: storageto.LogTree,
+		},
+		{
+			desc: "No revisions",
+			tree: storageto.RevisionlessLogTree,
+		},
 	}
+	for _, tC := range testCases {
+		t.Run(tC.desc, func(t *testing.T) {
+			ctx := context.Background()
+			cleanTestDB(DB)
+			as := NewAdminStorage(DB)
+			tree := mustCreateTree(ctx, t, as, tC.tree)
+			s := NewLogStorage(DB, nil)
 
-	{
-		runLogTX(s, tree, t, func(ctx context.Context, tx storage.LogTreeTX) error {
-			forceWriteRevision(writeRev, tx)
-			if err := tx.SetMerkleNodes(ctx, nodesToStore); err != nil {
-				t.Fatalf("Failed to store nodes: %s", err)
+			const writeRev = int64(100)
+			const size = 871
+			nodesToStore, err := createLogNodesForTreeAtSize(t, size, writeRev)
+			if err != nil {
+				t.Fatalf("failed to create test tree: %v", err)
+			}
+			nodeIDsToRead := make([]compact.NodeID, len(nodesToStore))
+			for i := range nodesToStore {
+				nodeIDsToRead[i] = nodesToStore[i].ID
 			}
-			return storeLogRoot(ctx, tx, uint64(size), uint64(writeRev), []byte{1, 2, 3})
-		})
-	}
 
-	{
-		runLogTX(s, tree, t, func(ctx context.Context, tx storage.LogTreeTX) error {
-			readNodes, err := tx.GetMerkleNodes(ctx, nodeIDsToRead)
-			if err != nil {
-				t.Fatalf("Failed to retrieve nodes: %s", err)
+			{
+				runLogTX(s, tree, t, func(ctx context.Context, tx storage.LogTreeTX) error {
+					forceWriteRevision(writeRev, tx)
+					if err := tx.SetMerkleNodes(ctx, nodesToStore); err != nil {
+						t.Fatalf("Failed to store nodes: %s", err)
+					}
+					return storeLogRoot(ctx, tx, uint64(size), uint64(writeRev), []byte{1, 2, 3})
+				})
 			}
-			if err := nodesAreEqual(readNodes, nodesToStore); err != nil {
-				missing, extra := diffNodes(readNodes, nodesToStore)
-				for _, n := range missing {
-					t.Errorf("Missing: %v", n.ID)
-				}
-				for _, n := range extra {
-					t.Errorf("Extra  : %v", n.ID)
-				}
-				t.Fatalf("Read back different nodes from the ones stored: %s", err)
+
+			{
+				runLogTX(s, tree, t, func(ctx context.Context, tx storage.LogTreeTX) error {
+					readNodes, err := tx.GetMerkleNodes(ctx, nodeIDsToRead)
+					if err != nil {
+						t.Fatalf("Failed to retrieve nodes: %s", err)
+					}
+					if err := nodesAreEqual(readNodes, nodesToStore); err != nil {
+						missing, extra := diffNodes(readNodes, nodesToStore)
+						for _, n := range missing {
+							t.Errorf("Missing: %v", n.ID)
+						}
+						for _, n := range extra {
+							t.Errorf("Extra  : %v", n.ID)
+						}
+						t.Fatalf("Read back different nodes from the ones stored: %s", err)
+					}
+					return nil
+				})
 			}
-			return nil
 		})
 	}
 }
diff --git a/storage/testonly/admin_storage_tester.go b/storage/testonly/admin_storage_tester.go
index c1b9e161cd..e88e5607c5 100644
--- a/storage/testonly/admin_storage_tester.go
+++ b/storage/testonly/admin_storage_tester.go
@@ -34,6 +34,16 @@ import (
 var (
 	// LogTree is a valid, LOG-type trillian.Tree for tests.
 	LogTree = &trillian.Tree{
+		TreeState:       trillian.TreeState_ACTIVE,
+		TreeType:        trillian.TreeType_LOG,
+		DisplayName:     "Llamas Log",
+		Description:     "Registry of publicly-owned llamas",
+		MaxRootDuration: durationpb.New(0 * time.Millisecond),
+	}
+
+	// LogTree is a valid, LOG-type trillian.Tree for tests.
+	// This tree is configured not to write revisions for each subtree.
+	RevisionlessLogTree = &trillian.Tree{
 		TreeState:       trillian.TreeState_ACTIVE,
 		TreeType:        trillian.TreeType_LOG,
 		DisplayName:     "Llamas Log",