added a ton of test rigor and fixed linting

.golangci.yml

@@ -1,51 +1,113 @@
-linters:
+run:
   tests: true
+  timeout: 10m
+
+linters:
   disable-all: true
   enable:
-    - bodyclose
-    - depguard
-    - dogsled
-    - dupl
+    - exportloopref
     - errcheck
+    - gci
     - goconst
     - gocritic
     - gofumpt
     - gosec
     - gosimple
     - govet
     - ineffassign
-    - lll
     - misspell
     - nakedret
-    - prealloc
     - staticcheck
+    - thelper
+    - typecheck
     - stylecheck
+    - revive
     - typecheck
+    - tenv
     - unconvert
+    # Prefer unparam over revive's unused param. It is more thorough in its checking.
     - unparam
     - unused
-    - nolintlint
+    - misspell
 
 issues:
+  exclude-rules:
+    - text: 'differs only by capitalization to method'
+      linters:
+        - revive
+    - text: 'Use of weak random number generator'
+      linters:
+        - gosec
+    - linters:
+        - staticcheck
+      text: "SA1019:" # silence errors on usage of deprecated funcs
+
+  max-issues-per-linter: 10000
+  max-same-issues: 10000
+
 linters-settings:
-  errcheck:
-    check-blank: true
-  depguard:
+  gci:
+    sections:
+      - standard # Standard section: captures all standard packages.
+      - default # Default section: contains all imports that could not be matched to another section type.
+      - blank # blank imports
+      - dot # dot imports
+      - prefix(github.com/cometbft/cometbft-db)
+    custom-order: true
+  revive:
+    enable-all-rules: true
+    # Do NOT whine about the following, full explanation found in:
+    # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#description-of-available-rules
     rules:
-      main:
-        files:
-          - $all
-          - "!$test"
-        allow:
-          - $gostd
-          - github.com/cometbft
-          - github.com/syndtr/goleveldb/leveldb
-          - github.com/google/btree
-      test:
-        files:
-          - $test
-        allow:
-          - $gostd
-          - github.com/cometbft
-          - github.com/syndtr/goleveldb/leveldb
-          - github.com/stretchr/testify
+      - name: use-any
+        disabled: true
+      - name: if-return
+        disabled: true
+      - name: max-public-structs
+        disabled: true
+      - name: cognitive-complexity
+        disabled: true
+      - name: argument-limit
+        disabled: true
+      - name: cyclomatic
+        disabled: true
+      - name: file-header
+        disabled: true
+      - name: function-length
+        disabled: true
+      - name: function-result-limit
+        disabled: true
+      - name: line-length-limit
+        disabled: true
+      - name: flag-parameter
+        disabled: true
+      - name: add-constant
+        disabled: true
+      - name: empty-lines
+        disabled: true
+      - name: banned-characters
+        disabled: true
+      - name: deep-exit
+        disabled: true
+      - name: confusing-results
+        disabled: true
+      - name: unused-parameter
+        disabled: true
+      - name: modifies-value-receiver
+        disabled: true
+      - name: early-return
+        disabled: true
+      - name: confusing-naming
+        disabled: true
+      - name: defer
+        disabled: true
+      # Disabled in favour of unparam.
+      - name: unused-parameter
+        disabled: true
+      - name: unhandled-error
+        disabled: false
+        arguments:
+          - 'fmt.Printf'
+          - 'fmt.Print'
+          - 'fmt.Println'
+          - 'myFunction'

backend_test.go

@@ -12,7 +12,7 @@ import (
 
 // Register a test backend for PrefixDB as well, with some unrelated junk data
 func init() {
-	//nolint: errcheck
+	//nolint: errcheck, revive // probably should check errors?
 	registerDBCreator("prefixdb", func(name, dir string) (DB, error) {
 		mdb := NewMemDB()
 		mdb.Set([]byte("a"), []byte{1})
@@ -33,6 +33,7 @@ func cleanupDBDir(dir, name string) {
 }
 
 func testBackendGetSetDelete(t *testing.T, backend BackendType) {
+	t.Helper()
 	// Default
 	dirname, err := os.MkdirTemp("", fmt.Sprintf("test_backend_%s_", backend))
 	require.Nil(t, err)
@@ -161,6 +162,8 @@ func TestDBIterator(t *testing.T) {
 }
 
 func testDBIterator(t *testing.T, backend BackendType) {
+	t.Helper()
+
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
 	db, err := NewDB(name, backend, dir)
@@ -317,6 +320,8 @@ func testDBIterator(t *testing.T, backend BackendType) {
 }
 
 func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) {
+	t.Helper()
+
 	var list []int64
 	for itr.Valid() {
 		key := itr.Key()
@@ -335,6 +340,8 @@ func TestDBBatch(t *testing.T) {
 }
 
 func testDBBatch(t *testing.T, backend BackendType) {
+	t.Helper()
+
 	name := fmt.Sprintf("test_%x", randStr(12))
 	dir := os.TempDir()
 	db, err := NewDB(name, backend, dir)
@@ -396,8 +403,12 @@ func testDBBatch(t *testing.T, backend BackendType) {
 
 	// it should be possible to close an empty batch, and to re-close a closed batch
 	batch = db.NewBatch()
-	batch.Close()
-	batch.Close()
+	if err := batch.Close(); err != nil {
+		require.NoError(t, err)
+	}
+	if err := batch.Close(); err != nil {
+		require.NoError(t, err)
+	}
 
 	// all other operations on a closed batch should error
 	require.Error(t, batch.Set([]byte("a"), []byte{9}))
@@ -407,6 +418,7 @@ func testDBBatch(t *testing.T, backend BackendType) {
 }
 
 func assertKeyValues(t *testing.T, db DB, expect map[string][]byte) {
+	t.Helper()
 	iter, err := db.Iterator(nil, nil)
 	require.NoError(t, err)
 	defer iter.Close()

common_test.go

@@ -11,38 +11,44 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-//----------------------------------------
+// ----------------------------------------
 // Helper functions.
 
 func checkValue(t *testing.T, db DB, key []byte, valueWanted []byte) {
+	t.Helper()
 	valueGot, err := db.Get(key)
 	assert.NoError(t, err)
 	assert.Equal(t, valueWanted, valueGot)
 }
 
 func checkValid(t *testing.T, itr Iterator, expected bool) {
+	t.Helper()
 	valid := itr.Valid()
 	require.Equal(t, expected, valid)
 }
 
 func checkNext(t *testing.T, itr Iterator, expected bool) {
+	t.Helper()
 	itr.Next()
 	// assert.NoError(t, err) TODO: look at fixing this
 	valid := itr.Valid()
 	require.Equal(t, expected, valid)
 }
 
 func checkNextPanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Next() }, "checkNextPanics expected an error but didn't")
 }
 
 func checkDomain(t *testing.T, itr Iterator, start, end []byte) {
+	t.Helper()
 	ds, de := itr.Domain()
 	assert.Equal(t, start, ds, "checkDomain domain start incorrect")
 	assert.Equal(t, end, de, "checkDomain domain end incorrect")
 }
 
 func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) {
+	t.Helper()
 	v := itr.Value()
 
 	k := itr.Key()
@@ -52,21 +58,25 @@ func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) {
 }
 
 func checkInvalid(t *testing.T, itr Iterator) {
+	t.Helper()
 	checkValid(t, itr, false)
 	checkKeyPanics(t, itr)
 	checkValuePanics(t, itr)
 	checkNextPanics(t, itr)
 }
 
 func checkKeyPanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Key() }, "checkKeyPanics expected panic but didn't")
 }
 
 func checkValuePanics(t *testing.T, itr Iterator) {
+	t.Helper()
 	assert.Panics(t, func() { itr.Value() })
 }
 
 func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
+	t.Helper()
 	dirname, err := os.MkdirTemp("", "db_common_test")
 	require.NoError(t, err)
 	db, err = NewDB("testdb", backend, dirname)
@@ -75,6 +85,7 @@ func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
 }
 
 func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
+	b.Helper()
 	b.StopTimer()
 
 	rangeSize := int64(10000)
@@ -83,8 +94,8 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
 	}
 
 	for i := int64(0); i < dbSize; i++ {
-		bytes := int642Bytes(i)
-		err := db.Set(bytes, bytes)
+		int64bytes := int642Bytes(i)
+		err := db.Set(int64bytes, int64bytes)
 		if err != nil {
 			// require.NoError() is very expensive (according to profiler), so check manually
 			b.Fatal(b, err)
@@ -101,12 +112,14 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) {
 		for ; iter.Valid(); iter.Next() {
 			count++
 		}
-		iter.Close()
+		err = iter.Close()
+		require.NoError(b, err)
 		require.EqualValues(b, rangeSize, count)
 	}
 }
 
 func benchmarkRandomReadsWrites(b *testing.B, db DB) {
+	b.Helper()
 	b.StopTimer()
 
 	// create dummy data

goleveldb_test.go

@@ -17,7 +17,8 @@ func TestGoLevelDBNewGoLevelDB(t *testing.T) {
 	require.Nil(t, err)
 	_, err = NewGoLevelDB(name, "")
 	require.NotNil(t, err)
-	wr1.Close() // Close the db to release the lock
+	err = wr1.Close() // Close the db to release the lock
+	require.Nil(t, err)
 
 	// Test we can open the db twice for reading only
 	ro1, err := NewGoLevelDBWithOpts(name, "", &opt.Options{ReadOnly: true})
@@ -35,7 +36,8 @@ func BenchmarkGoLevelDBRandomReadsWrites(b *testing.B) {
 		b.Fatal(err)
 	}
 	defer func() {
-		db.Close()
+		err = db.Close()
+		require.NoError(b, err)
 		cleanupDBDir("", name)
 	}()
 

memdb.go

@@ -137,7 +137,7 @@ func (db *MemDB) DeleteSync(key []byte) error {
 }
 
 // Close implements DB.
-func (db *MemDB) Close() error {
+func (*MemDB) Close() error {
 	// Close is a noop since for an in-memory database, we don't have a destination to flush
 	// contents to nor do we want any data loss on invoking Close().
 	return nil
@@ -149,7 +149,10 @@ func (db *MemDB) Print() error {
 	defer db.mtx.RUnlock()
 
 	db.btree.Ascend(func(i btree.Item) bool {
-		item := i.(*item)
+		item, ok := i.(*item)
+		if !ok {
+			return false // or handle the error as appropriate
+		}
 		fmt.Printf("[%X]:\t[%X]\n", item.key, item.value)
 		return true
 	})

memdb_iterator.go

@@ -56,7 +56,10 @@ func newMemDBIteratorMtxChoice(db *MemDB, start []byte, end []byte, reverse bool
 			abortLessThan []byte
 		)
 		visitor := func(i btree.Item) bool {
-			item := i.(*item)
+			item, ok := i.(*item)
+			if !ok {
+				return false // or handle the error as appropriate
+			}
 			if skipEqual != nil && bytes.Equal(item.key, skipEqual) {
 				skipEqual = nil
 				return true
@@ -105,7 +108,7 @@ func newMemDBIteratorMtxChoice(db *MemDB, start []byte, end []byte, reverse bool
 // Close implements Iterator.
 func (i *memDBIterator) Close() error {
 	i.cancel()
-	for range i.ch { // drain channel
+	for range i.ch { //nolint:revive // drain channel
 	}
 	i.item = nil
 	return nil
@@ -134,7 +137,7 @@ func (i *memDBIterator) Next() {
 }
 
 // Error implements Iterator.
-func (i *memDBIterator) Error() error {
+func (*memDBIterator) Error() error {
 	return nil // famous last words
 }