From ac2ac437fd8c2c27eee606987f4d49193de972b7 Mon Sep 17 00:00:00 2001 From: Gerard Louis Recinto Date: Thu, 10 Sep 2026 01:45:42 -0700 Subject: [PATCH 1/3] fix(ci): point Codecov at the renamed repo instead of the old sop slug go.yml uploaded coverage to slug: SharedCode/sop and the README badge linked the same old project - a leftover from the joltrin rebrand that every other badge in the README already picked up. The badge was pointing at a stale/wrong Codecov project regardless of the actual coverage percentage. --- .github/workflows/go.yml | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index d338ec280..b4b31e364 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -34,7 +34,7 @@ jobs: uses: codecov/codecov-action@v5 with: token: ${{ secrets.CODECOV_TOKEN }} - slug: SharedCode/sop + slug: SharedCode/joltrin perf: if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' diff --git a/README.md b/README.md index 826a3e3d4..5049c8c39 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ [![CI](https://github.com/SharedCode/joltrin/actions/workflows/ci.yml/badge.svg?branch=master)](https://github.com/SharedCode/joltrin/actions/workflows/ci.yml) [![Go Tests](https://github.com/SharedCode/joltrin/actions/workflows/go.yml/badge.svg?event=push&branch=master)](https://github.com/SharedCode/joltrin/actions/workflows/go.yml) [![Release](https://img.shields.io/github/v/release/SharedCode/joltrin)](https://github.com/SharedCode/joltrin/releases) -[![codecov](https://codecov.io/gh/SharedCode/sop/branch/master/graph/badge.svg)](https://app.codecov.io/github/SharedCode/sop) +[![codecov](https://codecov.io/gh/SharedCode/joltrin/branch/master/graph/badge.svg)](https://app.codecov.io/github/SharedCode/joltrin) [![Go Reference](https://pkg.go.dev/badge/github.com/sharedcode/joltrin.svg)](https://pkg.go.dev/github.com/sharedcode/joltrin) [![Go version](https://img.shields.io/github/go-mod/go-version/SharedCode/joltrin)](go.mod) [![License](https://img.shields.io/github/license/SharedCode/joltrin)](LICENSE) From 258f02511d1540c3bd452f10cc948ce2888784a0 Mon Sep 17 00:00:00 2001 From: Gerard Louis Recinto Date: Thu, 10 Sep 2026 01:49:14 -0700 Subject: [PATCH 2/3] test(btree): cover the untested Cursor and btreeWithTransaction delegates Cursor.AddIfNotExist/Upsert/Update/UpdateKey/Remove/FindWithID/ FindInDescendingOrder/UpdateCurrentItem/UpdateCurrentKey/GetCurrentItem/ GetCurrentItemNoLock, and the matching btreeWithTransaction wrapper methods (not-begun, non-writer, delegated-error, and success paths), were previously untested. Raises this repo's core-package coverage from 85.3% to 87.0%. --- btree/btreecursor_additional_test.go | 244 +++++++++++++++++++++++ btree/withtransaction_additional_test.go | 221 ++++++++++++++++++++ 2 files changed, 465 insertions(+) create mode 100644 btree/btreecursor_additional_test.go create mode 100644 btree/withtransaction_additional_test.go diff --git a/btree/btreecursor_additional_test.go b/btree/btreecursor_additional_test.go new file mode 100644 index 000000000..2e621d837 --- /dev/null +++ b/btree/btreecursor_additional_test.go @@ -0,0 +1,244 @@ +package btree + +import ( + "context" + "testing" + + "github.com/sharedcode/joltrin" +) + +// Covers Cursor methods that delegate to the underlying Btree but weren't +// exercised anywhere yet: AddIfNotExist, Upsert, Update, UpdateKey, Remove, +// FindWithID, FindInDescendingOrder, UpdateCurrentItem, UpdateCurrentKey, +// GetCurrentItem, and GetCurrentItemNoLock. + +func TestCursor_AddIfNotExist(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + + if ok, err := c.AddIfNotExist(ctx, 1, "first"); !ok || err != nil { + t.Fatalf("expected first add to succeed, got ok=%v err=%v", ok, err) + } + if ok, err := c.AddIfNotExist(ctx, 1, "second"); ok || err != nil { + t.Fatalf("expected duplicate add to no-op with ok=false, got ok=%v err=%v", ok, err) + } + + if ok, _ := c.Find(ctx, 1, true); !ok { + t.Fatal("Find(1) failed") + } + if v, err := c.GetCurrentValue(ctx); err != nil || v != "first" { + t.Errorf("expected the original value to survive the rejected duplicate add, got %q err=%v", v, err) + } +} + +func TestCursor_Upsert(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + + if ok, err := c.Upsert(ctx, 1, "inserted"); !ok || err != nil { + t.Fatalf("expected insert-path upsert to succeed, got ok=%v err=%v", ok, err) + } + if ok, err := c.Upsert(ctx, 1, "updated"); !ok || err != nil { + t.Fatalf("expected update-path upsert to succeed, got ok=%v err=%v", ok, err) + } + + if ok, _ := c.Find(ctx, 1, true); !ok { + t.Fatal("Find(1) failed") + } + if v, err := c.GetCurrentValue(ctx); err != nil || v != "updated" { + t.Errorf("expected upsert to overwrite the value, got %q err=%v", v, err) + } +} + +func TestCursor_UpdateByKey(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + c.Add(ctx, 1, "before") + + if ok, err := c.Update(ctx, 1, "after"); !ok || err != nil { + t.Fatalf("Update failed: ok=%v err=%v", ok, err) + } + + if ok, _ := c.Find(ctx, 1, true); !ok { + t.Fatal("Find(1) failed") + } + if v, err := c.GetCurrentValue(ctx); err != nil || v != "after" { + t.Errorf("expected updated value, got %q err=%v", v, err) + } +} + +// newTestKeyStructBtree builds a btree keyed on KeyStruct, comparing only +// ID: it lets UpdateKey/UpdateCurrentKey/UpdateCurrentItem change the +// non-comparer Metadata field, which a plain int key (the comparer value +// itself) can never allow - see key_update_test.go. +func newTestKeyStructBtree(t *testing.T) *Btree[KeyStruct, string] { + t.Helper() + store := sop.NewStoreInfo(sop.StoreOptions{SlotLength: 4, IsUnique: true, IsValueDataInNodeSegment: true}) + fnr := &fakeNR[KeyStruct, string]{n: map[sop.UUID]*Node[KeyStruct, string]{}} + si := StoreInterface[KeyStruct, string]{NodeRepository: fnr, ItemActionTracker: fakeIAT[KeyStruct, string]{}} + b, err := New[KeyStruct, string](store, &si, compareKeyStruct) + if err != nil { + t.Fatalf("New failed: %v", err) + } + return b +} + +func TestCursor_UpdateKey(t *testing.T) { + b := newTestKeyStructBtree(t) + ctx := context.Background() + c := NewCursor(b) + key := KeyStruct{ID: 1, Metadata: "initial"} + c.Add(ctx, key, "v") + + newKey := KeyStruct{ID: 1, Metadata: "updated"} + if ok, err := c.UpdateKey(ctx, newKey); !ok || err != nil { + t.Fatalf("UpdateKey failed: ok=%v err=%v", ok, err) + } + + if ok, _ := c.Find(ctx, key, false); !ok { + t.Fatal("Find by ID failed") + } + if got := c.GetCurrentKey().Key; got.Metadata != "updated" { + t.Errorf("expected metadata 'updated', got %q", got.Metadata) + } +} + +func TestCursor_Remove(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + c.Add(ctx, 1, "v") + + if ok, err := c.Remove(ctx, 1); !ok || err != nil { + t.Fatalf("Remove failed: ok=%v err=%v", ok, err) + } + if ok, _ := c.Find(ctx, 1, true); ok { + t.Fatal("expected key 1 to be gone after Remove") + } +} + +func TestCursor_FindWithID(t *testing.T) { + b, _ := newNonUniqueBtree[string]() + ctx := context.Background() + c := NewCursor(b) + c.Add(ctx, 5, "a") + c.Add(ctx, 5, "b") + + // Capture the ID of the second duplicate via the underlying Btree's cursor state. + if ok, err := b.Find(ctx, 5, true); err != nil || !ok { + t.Fatalf("find: %v ok=%v", err, ok) + } + if ok, err := b.Next(ctx); err != nil || !ok { + t.Fatalf("next: %v ok=%v", err, ok) + } + second, _ := b.GetCurrentItem(ctx) + + if ok, err := c.FindWithID(ctx, 5, second.ID); err != nil || !ok { + t.Fatalf("Cursor.FindWithID should locate the specific duplicate: ok=%v err=%v", ok, err) + } + if got := c.GetCurrentKey(); got.ID != second.ID { + t.Errorf("expected cursor positioned on ID %v, got %v", second.ID, got.ID) + } + + if ok, err := c.FindWithID(ctx, 5, sop.NewUUID()); err != nil || ok { + t.Fatalf("expected FindWithID to return (false,nil) for an unknown ID, got ok=%v err=%v", ok, err) + } +} + +func TestCursor_FindInDescendingOrder(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + for i := 1; i <= 5; i++ { + c.Add(ctx, i, "v") + } + + if ok, err := c.FindInDescendingOrder(ctx, 3); err != nil || !ok { + t.Fatalf("FindInDescendingOrder(3) failed: ok=%v err=%v", ok, err) + } + if k := c.GetCurrentKey().Key; k != 3 { + t.Errorf("expected cursor at 3, got %d", k) + } + // Previous still walks toward the next smaller distinct key (see + // TestFindInDescendingOrder_StringKeys: it only reverses duplicate + // insertion order, not distinct-key traversal direction). + if ok, err := c.Previous(ctx); err != nil || !ok { + t.Fatalf("Previous after descending find failed: ok=%v err=%v", ok, err) + } + if k := c.GetCurrentKey().Key; k != 2 { + t.Errorf("expected Previous to move to 2, got %d", k) + } +} + +func TestCursor_UpdateCurrentItem(t *testing.T) { + b := newTestKeyStructBtree(t) + ctx := context.Background() + c := NewCursor(b) + key := KeyStruct{ID: 1, Metadata: "initial"} + c.Add(ctx, key, "v") + c.Find(ctx, key, false) + + newKey := KeyStruct{ID: 1, Metadata: "updated_item"} + if ok, err := c.UpdateCurrentItem(ctx, newKey, "v2"); !ok || err != nil { + t.Fatalf("UpdateCurrentItem failed: ok=%v err=%v", ok, err) + } + item, err := c.GetCurrentItem(ctx) + if err != nil { + t.Fatalf("GetCurrentItem failed: %v", err) + } + if item.Key.Metadata != "updated_item" || item.Value == nil || *item.Value != "v2" { + t.Errorf("unexpected item after UpdateCurrentItem: %+v", item) + } +} + +func TestCursor_UpdateCurrentKey(t *testing.T) { + b := newTestKeyStructBtree(t) + ctx := context.Background() + c := NewCursor(b) + key := KeyStruct{ID: 1, Metadata: "initial"} + c.Add(ctx, key, "v") + c.Find(ctx, key, false) + + newKey := KeyStruct{ID: 1, Metadata: "updated"} + if ok, err := c.UpdateCurrentKey(ctx, newKey); !ok || err != nil { + t.Fatalf("UpdateCurrentKey failed: ok=%v err=%v", ok, err) + } + if got := c.GetCurrentKey().Key; got.Metadata != "updated" { + t.Errorf("expected metadata 'updated', got %q", got.Metadata) + } +} + +func TestCursor_GetCurrentItem(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + c.Add(ctx, 1, "v") + c.First(ctx) + + item, err := c.GetCurrentItem(ctx) + if err != nil { + t.Fatalf("GetCurrentItem failed: %v", err) + } + if item.Key != 1 || item.Value == nil || *item.Value != "v" { + t.Errorf("unexpected item: %+v", item) + } +} + +func TestCursor_GetCurrentItemNoLock(t *testing.T) { + b, _ := newTestBtree[string]() + ctx := context.Background() + c := NewCursor(b) + c.Add(ctx, 1, "v") + c.First(ctx) + + item, err := c.GetCurrentItemNoLock(ctx) + if err != nil { + t.Fatalf("GetCurrentItemNoLock failed: %v", err) + } + if item.Key != 1 || item.Value == nil || *item.Value != "v" { + t.Errorf("unexpected item: %+v", item) + } +} diff --git a/btree/withtransaction_additional_test.go b/btree/withtransaction_additional_test.go new file mode 100644 index 000000000..01356ea7a --- /dev/null +++ b/btree/withtransaction_additional_test.go @@ -0,0 +1,221 @@ +package btree + +import ( + "context" + "errors" + "testing" + + "github.com/sharedcode/joltrin" +) + +// Covers btreeWithTransaction wrapper methods left untested: UpdateKey, +// UpdateCurrentItem, UpdateCurrentKey (delegated-error and success paths), +// and GetCurrentItemNoLock. Follows the same not-begun/non-writer/ +// delegated-error/success shape as the existing Update/Upsert coverage in +// withtransaction_more_test.go. + +func TestWithTransaction_UpdateKey_AllPaths(t *testing.T) { + b := newTestKeyStructBtree(t) + key := KeyStruct{ID: 1, Metadata: "initial"} + if ok, err := b.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + + // Not begun + tx1 := &mockTx{begun: false, mode: sop.ForWriting} + w1 := NewBtreeWithTransaction[KeyStruct, string](tx1, b) + if _, err := w1.UpdateKey(context.Background(), key); !errors.Is(err, errTransHasNotBegunMsg) { + t.Fatalf("not-begun expected errTransHasNotBegunMsg, got %v", err) + } + + // Non-writer + tx2 := &mockTx{begun: true, mode: sop.ForReading} + w2 := NewBtreeWithTransaction[KeyStruct, string](tx2, b) + if _, err := w2.UpdateKey(context.Background(), key); err == nil { + t.Fatal("expected error on non-writer UpdateKey") + } + if tx2.rollbackCount != 1 { + t.Fatalf("expected rollback on non-writer UpdateKey, got %d", tx2.rollbackCount) + } + + // Delegated error + fnrErr := &fakeNR[KeyStruct, string]{n: map[sop.UUID]*Node[KeyStruct, string]{}} + siErr := StoreInterface[KeyStruct, string]{NodeRepository: fnrErr, ItemActionTracker: iatUpdateErr[KeyStruct, string]{}} + bErr, err := New[KeyStruct, string](sop.NewStoreInfo(sop.StoreOptions{SlotLength: 4, IsUnique: true, IsValueDataInNodeSegment: true}), &siErr, compareKeyStruct) + if err != nil { + t.Fatalf("New: %v", err) + } + if ok, err := bErr.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + tx3 := &mockTx{begun: true, mode: sop.ForWriting} + w3 := NewBtreeWithTransaction[KeyStruct, string](tx3, bErr) + if _, err := w3.UpdateKey(context.Background(), key); err == nil { + t.Fatal("expected delegated UpdateKey error") + } + if tx3.rollbackCount != 1 { + t.Fatalf("expected rollback on delegated UpdateKey error, got %d", tx3.rollbackCount) + } + + // Success + tx4 := &mockTx{begun: true, mode: sop.ForWriting} + w4 := NewBtreeWithTransaction[KeyStruct, string](tx4, b) + newKey := KeyStruct{ID: 1, Metadata: "updated"} + if ok, err := w4.UpdateKey(context.Background(), newKey); !ok || err != nil { + t.Fatalf("UpdateKey success expected, ok=%v err=%v", ok, err) + } + if tx4.rollbackCount != 0 { + t.Fatalf("success path should not rollback, got %d", tx4.rollbackCount) + } +} + +func TestWithTransaction_UpdateCurrentItem_AllPaths(t *testing.T) { + b := newTestKeyStructBtree(t) + key := KeyStruct{ID: 1, Metadata: "initial"} + if ok, err := b.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + b.Find(context.Background(), key, false) + + // Not begun + tx1 := &mockTx{begun: false, mode: sop.ForWriting} + w1 := NewBtreeWithTransaction[KeyStruct, string](tx1, b) + if _, err := w1.UpdateCurrentItem(context.Background(), key, "v2"); !errors.Is(err, errTransHasNotBegunMsg) { + t.Fatalf("not-begun expected errTransHasNotBegunMsg, got %v", err) + } + + // Non-writer + tx2 := &mockTx{begun: true, mode: sop.ForReading} + w2 := NewBtreeWithTransaction[KeyStruct, string](tx2, b) + if _, err := w2.UpdateCurrentItem(context.Background(), key, "v2"); err == nil { + t.Fatal("expected error on non-writer UpdateCurrentItem") + } + if tx2.rollbackCount != 1 { + t.Fatalf("expected rollback on non-writer UpdateCurrentItem, got %d", tx2.rollbackCount) + } + + // Delegated error + fnrErr := &fakeNR[KeyStruct, string]{n: map[sop.UUID]*Node[KeyStruct, string]{}} + siErr := StoreInterface[KeyStruct, string]{NodeRepository: fnrErr, ItemActionTracker: iatUpdateErr[KeyStruct, string]{}} + bErr, err := New[KeyStruct, string](sop.NewStoreInfo(sop.StoreOptions{SlotLength: 4, IsUnique: true, IsValueDataInNodeSegment: true}), &siErr, compareKeyStruct) + if err != nil { + t.Fatalf("New: %v", err) + } + if ok, err := bErr.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + bErr.Find(context.Background(), key, false) + tx3 := &mockTx{begun: true, mode: sop.ForWriting} + w3 := NewBtreeWithTransaction[KeyStruct, string](tx3, bErr) + if _, err := w3.UpdateCurrentItem(context.Background(), key, "v2"); err == nil { + t.Fatal("expected delegated UpdateCurrentItem error") + } + if tx3.rollbackCount != 1 { + t.Fatalf("expected rollback on delegated UpdateCurrentItem error, got %d", tx3.rollbackCount) + } + + // Success + tx4 := &mockTx{begun: true, mode: sop.ForWriting} + w4 := NewBtreeWithTransaction[KeyStruct, string](tx4, b) + newKey := KeyStruct{ID: 1, Metadata: "updated_item"} + if ok, err := w4.UpdateCurrentItem(context.Background(), newKey, "v2"); !ok || err != nil { + t.Fatalf("UpdateCurrentItem success expected, ok=%v err=%v", ok, err) + } + if tx4.rollbackCount != 0 { + t.Fatalf("success path should not rollback, got %d", tx4.rollbackCount) + } +} + +func TestWithTransaction_UpdateCurrentKey_DelegatedErrorAndSuccess(t *testing.T) { + b := newTestKeyStructBtree(t) + key := KeyStruct{ID: 1, Metadata: "initial"} + if ok, err := b.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + b.Find(context.Background(), key, false) + + // Delegated error + fnrErr := &fakeNR[KeyStruct, string]{n: map[sop.UUID]*Node[KeyStruct, string]{}} + siErr := StoreInterface[KeyStruct, string]{NodeRepository: fnrErr, ItemActionTracker: iatUpdateErr[KeyStruct, string]{}} + bErr, err := New[KeyStruct, string](sop.NewStoreInfo(sop.StoreOptions{SlotLength: 4, IsUnique: true, IsValueDataInNodeSegment: true}), &siErr, compareKeyStruct) + if err != nil { + t.Fatalf("New: %v", err) + } + if ok, err := bErr.Add(context.Background(), key, "v"); !ok || err != nil { + t.Fatalf("seed add: %v", err) + } + bErr.Find(context.Background(), key, false) + tx1 := &mockTx{begun: true, mode: sop.ForWriting} + w1 := NewBtreeWithTransaction[KeyStruct, string](tx1, bErr) + newKey := KeyStruct{ID: 1, Metadata: "updated"} + if _, err := w1.UpdateCurrentKey(context.Background(), newKey); err == nil { + t.Fatal("expected delegated UpdateCurrentKey error") + } + if tx1.rollbackCount != 1 { + t.Fatalf("expected rollback on delegated UpdateCurrentKey error, got %d", tx1.rollbackCount) + } + + // Success + tx2 := &mockTx{begun: true, mode: sop.ForWriting} + w2 := NewBtreeWithTransaction[KeyStruct, string](tx2, b) + if ok, err := w2.UpdateCurrentKey(context.Background(), newKey); !ok || err != nil { + t.Fatalf("UpdateCurrentKey success expected, ok=%v err=%v", ok, err) + } + if tx2.rollbackCount != 0 { + t.Fatalf("success path should not rollback, got %d", tx2.rollbackCount) + } +} + +func TestWithTransaction_GetCurrentItemNoLock_AllPaths(t *testing.T) { + store := sop.NewStoreInfo(sop.StoreOptions{SlotLength: 4, IsUnique: true}) + fnr := &fakeNRWithErr[int, string]{fakeNR: fakeNR[int, string]{n: map[sop.UUID]*Node[int, string]{}}, errOnGet: true} + si := StoreInterface[int, string]{NodeRepository: fnr, ItemActionTracker: fakeIAT[int, string]{}} + b, _ := New[int, string](store, &si, nil) + b.currentItemRef = currentItemRef{nodeID: sop.NewUUID(), nodeItemIndex: 0} + + // Not begun + tx1 := &mockTx{begun: false, mode: sop.ForReading} + w1 := NewBtreeWithTransaction[int, string](tx1, b) + if _, err := w1.GetCurrentItemNoLock(context.Background()); !errors.Is(err, errTransHasNotBegunMsg) { + t.Fatalf("not-begun expected errTransHasNotBegunMsg, got %v", err) + } + if tx1.rollbackCount != 1 { + t.Fatalf("not-begun should rollback, got %d", tx1.rollbackCount) + } + + // Delegated error (fakeNRWithErr errors on Get) + tx2 := &mockTx{begun: true, mode: sop.ForReading} + w2 := NewBtreeWithTransaction[int, string](tx2, b) + if _, err := w2.GetCurrentItemNoLock(context.Background()); err == nil { + t.Fatal("expected delegated GetCurrentItemNoLock error") + } + if tx2.rollbackCount != 1 { + t.Fatalf("expected rollback on delegated GetCurrentItemNoLock error, got %d", tx2.rollbackCount) + } + + // Success + fnrOK := &fakeNR[int, string]{n: map[sop.UUID]*Node[int, string]{}} + siOK := StoreInterface[int, string]{NodeRepository: fnrOK, ItemActionTracker: fakeIAT[int, string]{}} + bOK, _ := New[int, string](store, &siOK, nil) + root := newNode[int, string](bOK.getSlotLength()) + root.newID(sop.NilUUID) + v := "v" + root.Slots[0] = Item[int, string]{Key: 1, Value: &v, ID: sop.NewUUID()} + root.Count = 1 + bOK.StoreInfo.RootNodeID = root.ID + fnrOK.Add(root) + bOK.setCurrentItemID(root.ID, 0) + + tx3 := &mockTx{begun: true, mode: sop.ForReading} + w3 := NewBtreeWithTransaction[int, string](tx3, bOK) + item, err := w3.GetCurrentItemNoLock(context.Background()) + if err != nil { + t.Fatalf("GetCurrentItemNoLock success expected, got err=%v", err) + } + if item.Key != 1 { + t.Errorf("expected key 1, got %d", item.Key) + } + if tx3.rollbackCount != 0 { + t.Fatalf("success path should not rollback, got %d", tx3.rollbackCount) + } +} From e72849ac044998eccaef90bbee50061326ab1f6a Mon Sep 17 00:00:00 2001 From: Gerard Louis Recinto Date: Thu, 10 Sep 2026 01:51:55 -0700 Subject: [PATCH 3/3] test(common): cover Transaction.GetStores/CommitMaxDuration and the btree cursor openers GetStores and CommitMaxDuration were only ever exercised through their underlying dependency directly, never through the wrapper method itself. CursorOnOpenedBtree and OpenBtreeCursor (both 0%) needed their nil-transaction/not-begun/empty-name/not-found precondition branches plus the already-open and fetch-from-StoreRepository success paths. --- common/accessors_additional_test.go | 48 ++++++ common/managebtree_cursor_additional_test.go | 150 +++++++++++++++++++ 2 files changed, 198 insertions(+) create mode 100644 common/accessors_additional_test.go create mode 100644 common/managebtree_cursor_additional_test.go diff --git a/common/accessors_additional_test.go b/common/accessors_additional_test.go new file mode 100644 index 000000000..dbd6ee0cb --- /dev/null +++ b/common/accessors_additional_test.go @@ -0,0 +1,48 @@ +package common + +import ( + "testing" + "time" + + "github.com/sharedcode/joltrin" + "github.com/sharedcode/joltrin/common/mocks" +) + +// Test_TwoPC_GetStores_And_CommitMaxDuration covers two accessor methods +// that were only ever exercised through their underlying dependency +// (tr.GetStoreRepository().GetAll directly) rather than through the +// Transaction wrapper methods themselves. +func Test_TwoPC_GetStores_And_CommitMaxDuration(t *testing.T) { + tr, err := NewTwoPhaseCommitTransaction(sop.ForReading, 0, mockNodeBlobStore, mockStoreRepository, mockRegistry, mockRedisCache, mocks.NewMockTransactionLog()) + if err != nil { + t.Fatalf("ctor error: %v", err) + } + + if got := tr.CommitMaxDuration(); got <= 0 { + t.Fatalf("expected a positive default CommitMaxDuration, got %v", got) + } + + names, err := tr.GetStores(ctx) + if err != nil { + t.Fatalf("GetStores error: %v", err) + } + // GetStores just forwards to StoreRepository.GetAll; verifying it + // returns without error and matches the repository directly is enough + // to prove the delegation, without depending on other tests' seeded state. + directNames, err := tr.GetStoreRepository().GetAll(ctx) + if err != nil { + t.Fatalf("direct GetAll error: %v", err) + } + if len(names) != len(directNames) { + t.Fatalf("GetStores should forward to StoreRepository.GetAll: got %d names, repository has %d", len(names), len(directNames)) + } + + // CommitMaxDuration with an explicit, non-default value. + tr2, err := NewTwoPhaseCommitTransaction(sop.ForReading, 10*time.Minute, mockNodeBlobStore, mockStoreRepository, mockRegistry, mockRedisCache, mocks.NewMockTransactionLog()) + if err != nil { + t.Fatalf("ctor error: %v", err) + } + if got := tr2.CommitMaxDuration(); got != 10*time.Minute { + t.Fatalf("expected CommitMaxDuration to reflect the configured 10m, got %v", got) + } +} diff --git a/common/managebtree_cursor_additional_test.go b/common/managebtree_cursor_additional_test.go new file mode 100644 index 000000000..a04edd2b6 --- /dev/null +++ b/common/managebtree_cursor_additional_test.go @@ -0,0 +1,150 @@ +package common + +import ( + "cmp" + "testing" + + "github.com/sharedcode/joltrin" +) + +// Covers CursorOnOpenedBtree and OpenBtreeCursor, previously untested (0%). + +func TestCursorOnOpenedBtree_Preconditions(t *testing.T) { + trans, _ := newMockTransaction(t, sop.ForWriting, -1) + trans.Begin(ctx) + + if _, err := CursorOnOpenedBtree[int, string](ctx, "any", nil); err == nil { + t.Fatal("expected error for nil transaction") + } + + notBegun, _ := newMockTransaction(t, sop.ForWriting, -1) + if _, err := CursorOnOpenedBtree[int, string](ctx, "any", notBegun); err == nil { + t.Fatal("expected error for a transaction that has not begun") + } + + if _, err := CursorOnOpenedBtree[int, string](ctx, "", trans); err == nil { + t.Fatal("expected error for empty store name") + } + + if _, err := CursorOnOpenedBtree[int, string](ctx, "never-opened", trans); err == nil { + t.Fatal("expected error when the store was never opened in this transaction") + } +} + +func TestCursorOnOpenedBtree_Success(t *testing.T) { + trans, _ := newMockTransaction(t, sop.ForWriting, -1) + trans.Begin(ctx) + + b3, err := NewBtree[int, string](ctx, sop.StoreOptions{ + Name: "cursorOpened1", + SlotLength: 8, + IsUnique: true, + IsValueDataInNodeSegment: true, + }, trans, cmp.Compare) + if err != nil { + t.Fatalf("NewBtree failed: %v", err) + } + if ok, err := b3.Add(ctx, 1, "v"); !ok || err != nil { + t.Fatalf("seed add failed: ok=%v err=%v", ok, err) + } + + cur, err := CursorOnOpenedBtree[int, string](ctx, "cursorOpened1", trans) + if err != nil { + t.Fatalf("CursorOnOpenedBtree failed: %v", err) + } + if ok, err := cur.Find(ctx, 1, true); !ok || err != nil { + t.Fatalf("cursor Find failed: ok=%v err=%v", ok, err) + } + if v, err := cur.GetCurrentValue(ctx); err != nil || v != "v" { + t.Fatalf("cursor GetCurrentValue got %q err=%v", v, err) + } +} + +func TestOpenBtreeCursor_Preconditions(t *testing.T) { + trans, _ := newMockTransaction(t, sop.ForWriting, -1) + trans.Begin(ctx) + + if _, err := OpenBtreeCursor[int, string](ctx, "any", nil, cmp.Compare); err == nil { + t.Fatal("expected error for nil transaction") + } + + notBegun, _ := newMockTransaction(t, sop.ForWriting, -1) + if _, err := OpenBtreeCursor[int, string](ctx, "any", notBegun, cmp.Compare); err == nil { + t.Fatal("expected error for a transaction that has not begun") + } + + if _, err := OpenBtreeCursor[int, string](ctx, "", trans, cmp.Compare); err == nil { + t.Fatal("expected error for empty store name") + } + + if _, err := OpenBtreeCursor[int, string](ctx, "does-not-exist-anywhere", trans, cmp.Compare); err == nil { + t.Fatal("expected error when the store doesn't exist in this transaction or the repository") + } +} + +func TestOpenBtreeCursor_AlreadyOpenInTransaction(t *testing.T) { + trans, _ := newMockTransaction(t, sop.ForWriting, -1) + trans.Begin(ctx) + + b3, err := NewBtree[int, string](ctx, sop.StoreOptions{ + Name: "cursorOpened2", + SlotLength: 8, + IsUnique: true, + IsValueDataInNodeSegment: true, + }, trans, cmp.Compare) + if err != nil { + t.Fatalf("NewBtree failed: %v", err) + } + if ok, err := b3.Add(ctx, 2, "v2"); !ok || err != nil { + t.Fatalf("seed add failed: ok=%v err=%v", ok, err) + } + + cur, err := OpenBtreeCursor[int, string](ctx, "cursorOpened2", trans, cmp.Compare) + if err != nil { + t.Fatalf("OpenBtreeCursor (already-open path) failed: %v", err) + } + if ok, err := cur.Find(ctx, 2, true); !ok || err != nil { + t.Fatalf("cursor Find failed: ok=%v err=%v", ok, err) + } +} + +func TestOpenBtreeCursor_FetchesFromStoreRepository(t *testing.T) { + // Create the store in one transaction (NewBtree registers it into + // StoreRepository synchronously, not just on commit). + transA, _ := newMockTransaction(t, sop.ForWriting, -1) + transA.Begin(ctx) + b3, err := NewBtree[int, string](ctx, sop.StoreOptions{ + Name: "cursorOpened3", + SlotLength: 8, + IsUnique: true, + IsValueDataInNodeSegment: true, + }, transA, cmp.Compare) + if err != nil { + t.Fatalf("NewBtree failed: %v", err) + } + if ok, err := b3.Add(ctx, 3, "v3"); !ok || err != nil { + t.Fatalf("seed add failed: ok=%v err=%v", ok, err) + } + + // A fresh transaction has not opened "cursorOpened3" yet, so + // OpenBtreeCursor must take the StoreRepository-fetch branch. transA + // never committed, so transB sees the store definition but none of + // transA's uncommitted node data - verify the returned cursor is a + // working handle on this fresh transaction's own view of the store, + // not that it somehow sees transA's in-flight writes. + transB, _ := newMockTransaction(t, sop.ForWriting, -1) + transB.Begin(ctx) + cur, err := OpenBtreeCursor[int, string](ctx, "cursorOpened3", transB, cmp.Compare) + if err != nil { + t.Fatalf("OpenBtreeCursor (fetch-from-repository path) failed: %v", err) + } + if ok, err := cur.Add(ctx, 30, "v30"); !ok || err != nil { + t.Fatalf("cursor Add on fetched store failed: ok=%v err=%v", ok, err) + } + if ok, err := cur.Find(ctx, 30, true); !ok || err != nil { + t.Fatalf("cursor Find failed: ok=%v err=%v", ok, err) + } + if v, err := cur.GetCurrentValue(ctx); err != nil || v != "v30" { + t.Fatalf("cursor GetCurrentValue got %q err=%v", v, err) + } +}