Skip to content

test: add test examples for prefixdb.go #22752

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tac0turtle merged 31 commits into from
Dec 7, 2024
Merged

test: add test examples for prefixdb.go #22752

Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6bf5fbd
refact(math/):refact ApproxRoot for readality
xujiangyu Oct 15, 2024
687b732
Merge branch 'cosmos:main' into main
xujiangyu Oct 15, 2024
32d9110
Merge branch 'cosmos:main' into main
xujiangyu Oct 16, 2024
6097cc0
Merge branch 'cosmos:main' into main
xujiangyu Oct 24, 2024
3ff753f
test:(math/dec): add test case for ApproxRoot()
heren-ke Oct 24, 2024
c825214
Merge branch 'main' into main
xujiangyu Oct 25, 2024
fa71970
Merge branch 'cosmos:main' into main
xujiangyu Oct 25, 2024
3851378
Merge branch 'main' into main
xujiangyu Oct 26, 2024
a2955ef
Merge branch 'main' into main
xujiangyu Oct 28, 2024
ccf8c42
Merge branch 'main' into main
xujiangyu Oct 28, 2024
0cc52f2
Merge branch 'cosmos:main' into main
xujiangyu Oct 28, 2024
e854dbc
Merge branch 'cosmos:main' into main
xujiangyu Oct 29, 2024
c2d1698
Merge branch 'cosmos:main' into main
xujiangyu Oct 29, 2024
18c263c
test: fix test case for ApproxRoot
heren-ke Oct 30, 2024
075ebbb
Merge branch 'cosmos:main' into main
xujiangyu Oct 31, 2024
d152a6b
Merge branch 'cosmos:main' into main
xujiangyu Nov 7, 2024
402149b
Merge branch 'cosmos:main' into main
xujiangyu Nov 19, 2024
1c569e2
Merge branch 'cosmos:main' into main
xujiangyu Nov 21, 2024
8917ceb
Merge branch 'cosmos:main' into main
xujiangyu Nov 22, 2024
a51ebf6
Merge branch 'cosmos:main' into main
xujiangyu Dec 3, 2024
42c75b6
Merge branch 'cosmos:main' into main
xujiangyu Dec 4, 2024
86e7052
test: add test cases for prefixdb.go
heren-ke Dec 4, 2024
579aebc
Merge branch 'cosmos:main' into main
xujiangyu Dec 4, 2024
39f67a7
Merge branch 'cosmos:main' into main
xujiangyu Dec 5, 2024
8b6f1ca
Merge branch 'cosmos:main' into main
xujiangyu Dec 5, 2024
b88d678
Merge branch 'main' into main
xujiangyu Dec 5, 2024
aa87378
Merge branch 'cosmos:main' into main
xujiangyu Dec 6, 2024
d83d4f8
Merge branch 'main' into main
xujiangyu Dec 6, 2024
7026bc9
Merge branch 'cosmos:main' into main
xujiangyu Dec 6, 2024
e2cae82
format prefixdb_test.go
heren-ke Dec 6, 2024
245dafa
Merge branch 'main' into main
xujiangyu Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions store/db/prefixdb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package db_test

import (
"testing"

"github.com/stretchr/testify/require"
Copy link
Contributor

@julienrbrt julienrbrt Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make lint-fix

Copy link
Contributor Author

@xujiangyu xujiangyu Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, it was done with go fmt

"cosmossdk.io/store/db"
"cosmossdk.io/store/mock"
"go.uber.org/mock/gomock"
)

func TestPrefixDB(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()

mockDB := mock.NewMockKVStoreWithBatch(mockCtrl)
prefix := []byte("test:")
pdb := db.NewPrefixDB(mockDB, prefix)

key := []byte("key1")
value := []byte("value1")
mockDB.EXPECT().Set(gomock.Eq(append(prefix, key...)), gomock.Eq(value)).Return(nil)

err := pdb.Set(key, value)
require.NoError(t, err)

mockDB.EXPECT().Get(gomock.Eq(append(prefix, key...))).Return(value, nil)

returnedValue, err := pdb.Get(key)
require.NoError(t, err)
require.Equal(t, value, returnedValue)

Comment on lines +20 to +32
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test cases for error scenarios and edge cases

The current test coverage for Set/Get operations could be enhanced by adding:

  1. Error scenarios from the underlying DB
  2. Edge cases with nil/empty values
  3. Concurrent access scenarios
  4. Large key/value pairs

Example test cases to add:

// Test error propagation
mockDB.EXPECT().Set(gomock.Any(), gomock.Any()).Return(errors.New("db error"))
err := pdb.Set(key, value)
require.Error(t, err)

// Test nil value
mockDB.EXPECT().Set(gomock.Any(), nil).Return(nil)
err = pdb.Set(key, nil)
require.NoError(t, err)

mockDB.EXPECT().Has(gomock.Eq(append(prefix, key...))).Return(true, nil)

has, err := pdb.Has(key)
require.NoError(t, err)
require.True(t, has)

mockDB.EXPECT().Delete(gomock.Eq(append(prefix, key...))).Return(nil)

err = pdb.Delete(key)
require.NoError(t, err)

mockDB.EXPECT().Has(gomock.Eq(append(prefix, key...))).Return(false, nil)

has, err = pdb.Has(key)
require.NoError(t, err)
require.False(t, has)
}
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance Has/Delete operations test coverage

The test coverage for Has/Delete operations should include:

  1. Error scenarios from the underlying DB
  2. Multiple key operations
  3. Delete of non-existent keys

Example test cases to add:

// Test Has error propagation
mockDB.EXPECT().Has(gomock.Any()).Return(false, errors.New("db error"))
_, err = pdb.Has(key)
require.Error(t, err)

// Test Delete non-existent key
mockDB.EXPECT().Delete(gomock.Any()).Return(nil)
err = pdb.Delete([]byte("non-existent-key"))
require.NoError(t, err)
🧰 Tools
🪛 golangci-lint (1.62.2)

49-49: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order

(gci)

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Test coverage needs significant improvement

The current test file is missing critical test coverage for required interface methods:

  1. Iterator and ReverseIterator methods are completely untested, despite being core requirements of the KVStore interface
  2. Batch operations through the KVStoreWithBatch interface are not covered
  3. No tests for edge cases like empty prefix, nil keys, or concurrent access scenarios

Required test additions:

  • Add tests for forward and reverse iteration over prefixed keys
  • Include batch operation tests as required by KVStoreWithBatch interface
  • Add tests for concurrent access safety (the struct has a mutex)
  • Cover edge cases with empty/nil inputs
🔗 Analysis chain

Verify test coverage with additional scenarios

While the test provides good basic coverage, consider adding:

  1. Batch operations testing
  2. Iterator testing if supported by PrefixDB
  3. Performance tests for large datasets
  4. Recovery from panic scenarios

Run the following script to check test coverage:

Let me gather more information about the interface methods and implementations to provide a comprehensive assessment.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check test coverage for prefixdb package

# Search for PrefixDB methods that might need testing
ast-grep --pattern 'type PrefixDB struct {
  $$$
}'

# Search for interface methods that should be tested
ast-grep --pattern 'type DB interface {
  $$$
}'

Length of output: 547

Script:

#!/bin/bash
# Search for all methods implemented by PrefixDB
ast-grep --pattern 'func (p *PrefixDB) $_($$$) $$$'

# Search for KVStoreWithBatch interface definition
ast-grep --pattern 'type KVStoreWithBatch interface {
  $$$
}'

# Search for any existing iterator or batch related tests
rg -l "Iterator|Batch" store/db/prefixdb_test.go

Length of output: 500

Script:

#!/bin/bash
# Let's check the KVStore interface methods
ast-grep --pattern 'type KVStore interface {
  $$$
}'

# Check PrefixDB implementation for iterator and batch methods
rg -A 5 "func \(p \*PrefixDB\)" store/db/prefixdb.go

# Check if there are any other test files for PrefixDB
fd prefixdb.*test

Length of output: 1513

🧰 Tools
🪛 golangci-lint (1.62.2)

6-6: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order

(gci)

9-9: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order

(gci)

49-49: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order

(gci)

8-8: File is not gofumpt-ed with -extra

(gofumpt)

Loading