From c71790b3a1258f7633d4208b492b5e7bee228526 Mon Sep 17 00:00:00 2001 From: "kayos@tcp.direct" Date: Sat, 8 Jan 2022 21:50:58 -0800 Subject: [PATCH] Testing: yet another edge case bug fixed --- bitcask/bitcask.go | 22 ++++--- bitcask/bitcask_search_test.go | 107 ++++++++++++++++++++++----------- bitcask/bitcask_test.go | 43 +++++++++++-- bitcask/errors.go | 5 +- 4 files changed, 126 insertions(+), 51 deletions(-) diff --git a/bitcask/bitcask.go b/bitcask/bitcask.go index 313f425..777c44a 100644 --- a/bitcask/bitcask.go +++ b/bitcask/bitcask.go @@ -1,7 +1,6 @@ package bitcask import ( - "errors" "strings" "sync" @@ -42,7 +41,7 @@ func (db *DB) Init(bucketName string) error { db.mu.Lock() defer db.mu.Unlock() if _, ok := db.store["bucketName"]; ok { - return errors.New("bucket already exists") + return errStoreExists } path := db.Path() if !strings.HasSuffix("/", db.Path()) { @@ -73,7 +72,11 @@ func (db *DB) With(bucketName string) Store { func (db *DB) Close(bucketName string) error { db.mu.Lock() defer db.mu.Unlock() - err := db.store[bucketName].Close() + st, ok := db.store[bucketName] + if !ok { + return errBogusStore + } + err := st.Close() if err != nil { return err } @@ -98,11 +101,14 @@ const ( dsync ) -// WithAll performs an action on all bitcask stores that we have open. -// In the case of an error, WithAll will continue and return a compound form of any errors that occurred. +// withAll performs an action on all bitcask stores that we have open. +// In the case of an error, withAll will continue and return a compound form of any errors that occurred. // For now this is just for Close and Sync, thusly it does a hard lock on the Keeper. -func (db *DB) WithAll(action withAllAction) error { +func (db *DB) withAll(action withAllAction) error { var errs []error + if len(db.store) < 1 { + return errNoStores + } for name, store := range db.store { var err error if store.Bitcask == nil { @@ -141,10 +147,10 @@ func (db *DB) SyncAndCloseAll() error { // CloseAll closes all bitcask datastores. func (db *DB) CloseAll() error { - return db.WithAll(dclose) + return db.withAll(dclose) } // SyncAll syncs all bitcask datastores. func (db *DB) SyncAll() error { - return db.WithAll(dsync) + return db.withAll(dsync) } diff --git a/bitcask/bitcask_search_test.go b/bitcask/bitcask_search_test.go index c70340b..2490741 100644 --- a/bitcask/bitcask_search_test.go +++ b/bitcask/bitcask_search_test.go @@ -104,53 +104,88 @@ func Test_Search(t *testing.T) { addJunk(db, storename, one, two, three, four, five, t, true) - t.Logf("executing search for %s", needle) - results, err := db.With(storename).Search(needle) - if err != nil { - t.Errorf("failed to search: %e", err) - } - var keys = []int{one, two, three, four, five} - var needed = len(keys) - for _, kv := range results { - keyint, err := strconv.Atoi(kv.Key.String()) + // For coverage + db.store["yeet"] = Store{Bitcask: nil} + + t.Run("BasicSearch", func(t *testing.T) { + + t.Logf("executing search for %s", needle) + + results, err := db.With(storename).Search(needle) if err != nil { - t.Fatalf("failed to convert Key to int: %e", err) + t.Errorf("failed to search: %e", err) } - for _, k := range keys { - if keyint == k { - needed-- + var keys = []int{one, two, three, four, five} + var needed = len(keys) + for _, kv := range results { + keyint, err := strconv.Atoi(kv.Key.String()) + if err != nil { + t.Fatalf("failed to convert Key to int: %e", err) } + for _, k := range keys { + if keyint == k { + needed-- + } + } + keys = append(keys, keyint) + t.Logf("Found Key: %s, Value: %s", kv.Key.String(), kv.Value.String()) } - keys = append(keys, keyint) - t.Logf("Found Key: %s, Value: %s", kv.Key.String(), kv.Value.String()) - } - if needed != 0 { - t.Errorf("Needed %d results, got %d", len(keys), len(keys)-needed) - } + if needed != 0 { + t.Errorf("Needed %d results, got %d", len(keys), len(keys)-needed) + } + }) + + t.Run("NoResultsSearch", func(t *testing.T) { + bogus := c.RandStr(55) + t.Logf("executing search for %s", bogus) + + results, err := db.With(storename).Search(bogus) + if err != nil { + t.Errorf("failed to search: %e", err) + } + if len(results) > 0 { + t.Errorf("[FAIL] got %d results, wanted 0", len(results)) + } + }) } func Test_ValueExists(t *testing.T) { var storename = "test_value_exists" var db = setupTest(storename, t) - needles := addJunk(db, storename, c.RNG(100), c.RNG(100), c.RNG(100), c.RNG(100), c.RNG(100), t, true) + t.Run("ValueExists", func(t *testing.T) { + needles := addJunk(db, storename, c.RNG(100), c.RNG(100), c.RNG(100), c.RNG(100), c.RNG(100), t, true) - for _, needle := range needles { - if k, ok := db.With(storename).ValueExists(needle); !ok { - t.Errorf("[FAIL] store should have contained a value %s somewhere, it did not.", string(needle)) - } else { - t.Logf("[SUCCESS] successfully located value: %s, at key: %s", string(k), string(needle)) + for _, needle := range needles { + if k, exists := db.With(storename).ValueExists(needle); !exists { + t.Errorf("[FAIL] store should have contained a value %s somewhere, it did not.", string(needle)) + } else { + t.Logf("[SUCCESS] successfully located value: %s, at key: %s", string(k), string(needle)) + } } - } + }) - for n := 0; n != 5; n++ { - garbage := c.RandStr(55) + t.Run("ValueShouldNotExist", func(t *testing.T) { + for n := 0; n != 5; n++ { + garbage := c.RandStr(55) + if _, exists := db.With(storename).ValueExists([]byte(garbage)); exists { + t.Errorf("[FAIL] store should have not contained value %v, but it did", []byte(garbage)) + } else { + t.Logf("[SUCCESS] store succeeded in not having random value %s", garbage) + } + } + }) + + t.Run("ValueExistNilBitcask", func(t *testing.T) { + db.store["asdb"] = Store{Bitcask: nil} + garbage := "yeet" if _, exists := db.With(storename).ValueExists([]byte(garbage)); exists { - t.Errorf("[FAIL] store should have not contained value %v, but it did", []byte(garbage)) + t.Errorf("[FAIL] store should have not contained value %v, should have been nil", []byte(garbage)) } else { - t.Logf("[SUCCESS] store succeeded in not having random value %s", garbage) + t.Log("[SUCCESS] store succeeded in being nil") } - } + + }) } func Test_PrefixScan(t *testing.T) { @@ -160,23 +195,23 @@ func Test_PrefixScan(t *testing.T) { addJunk(db, storename, c.RNG(5), c.RNG(5), c.RNG(5), c.RNG(5), c.RNG(5), t, false) var needles = []KeyValue{ - KeyValue{ + { Key: Key{b: []byte("user:Fuckhole")}, Value: Value{b: []byte(c.RandStr(55))}, }, - KeyValue{ + { Key: Key{b: []byte("user:Johnson")}, Value: Value{b: []byte(c.RandStr(55))}, }, - KeyValue{ + { Key: Key{b: []byte("user:Jackson")}, Value: Value{b: []byte(c.RandStr(55))}, }, - KeyValue{ + { Key: Key{b: []byte("user:Frackhole")}, Value: Value{b: []byte(c.RandStr(55))}, }, - KeyValue{ + { Key: Key{b: []byte("user:Baboshka")}, Value: Value{b: []byte(c.RandStr(55))}, }, diff --git a/bitcask/bitcask_test.go b/bitcask/bitcask_test.go index 96a4a17..a827cc1 100644 --- a/bitcask/bitcask_test.go +++ b/bitcask/bitcask_test.go @@ -39,13 +39,14 @@ func TestDB_Init(t *testing.T) { var db = newTestDB(t) type args struct { - bucketName string + storeName string } type test struct { name string fields *DB args args wantErr bool + specErr error } tests := []test{ @@ -56,10 +57,11 @@ func TestDB_Init(t *testing.T) { wantErr: false, }, { - name: "bucketExists", + name: "storeExists", fields: db, args: args{"simple"}, wantErr: true, + specErr: errStoreExists, }, { name: "newBucket", @@ -71,9 +73,13 @@ func TestDB_Init(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := db.Init(tt.args.bucketName); (err != nil) != tt.wantErr { + err := db.Init(tt.args.storeName) + if (err != nil) != tt.wantErr { t.Errorf("[FAIL] Init() error = %v, wantErr %v", err, tt.wantErr) } + if (err != nil) != tt.wantErr && tt.specErr != nil && err != tt.specErr { + t.Errorf("[FAIL] wanted error %e, got error %e", tt.specErr, err) + } }) } @@ -96,9 +102,9 @@ func TestDB_Init(t *testing.T) { }) t.Run("withBucketDoesntExist", func(t *testing.T) { if nope := db.With("asdfqwerty"); nope.Bitcask != nil { - t.Errorf("[FAIL] got non nil result for nonexistent bucket: %T, %v", nope, nope) + t.Errorf("[FAIL] got non nil result for nonexistent store: %T, %v", nope, nope) } - t.Logf("[SUCCESS] got nil Value for bucket that doesn't exist") + t.Logf("[SUCCESS] got nil Value for store that doesn't exist") }) t.Run("syncAllShouldFail", func(t *testing.T) { db.store["wtf"] = Store{} @@ -187,4 +193,31 @@ func Test_Close(t *testing.T) { } t.Logf("[SUCCESS] Confirmed that all stores have been closed") }) + + t.Run("CantCloseBogusStore", func(t *testing.T) { + err := db.Close(c.RandStr(55)) + if err != errBogusStore { + t.Errorf("[FAIL] got err %e, wanted err %e", err, errBogusStore) + } + }) +} + +func Test_withAll(t *testing.T) { + var db = newTestDB(t) + t.Run("withAllNoStores", func(t *testing.T) { + err := db.withAll(121) + if err != errNoStores { + t.Errorf("[FAIL] got err %e, wanted err %e", err, errBogusStore) + } + }) + t.Run("withAllBogusAction", func(t *testing.T) { + err := db.Init("asdf") + if err != nil { + t.Errorf("[FAIL] unexpected error: %e", err) + } + wAllErr := db.withAll(121) + if wAllErr != errUnknownAction { + t.Errorf("[FAIL] wanted error %e, got error %e", errUnknownAction, err) + } + }) } diff --git a/bitcask/errors.go b/bitcask/errors.go index 46b665f..c915ae1 100644 --- a/bitcask/errors.go +++ b/bitcask/errors.go @@ -7,13 +7,14 @@ import ( var errUnknownAction = errors.New("unknown action") var errBogusStore = errors.New("bogus store backend") - +var errStoreExists = errors.New("store name already exists") +var errNoStores = errors.New("no stores initialized") func namedErr(name string, err error) error { if err == nil { return nil } - return errors.New(name+": "+err.Error()) + return errors.New(name + ": " + err.Error()) } func compoundErrors(errs []error) error {