From 283896e4c923478327cb6420649c5ebb8632446b Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Fri, 19 Jan 2024 14:17:17 -0500 Subject: [PATCH 1/7] Replace `closeFn` with `t.Cleanup` --- database/benchmark_database.go | 20 ++--- database/rpcdb/db_test.go | 40 +++++----- database/test_database.go | 76 +++++++++---------- .../gvalidators/validator_state_test.go | 22 ++---- vms/platformvm/warp/gwarp/signer_test.go | 20 ++--- vms/platformvm/warp/test_signer.go | 6 +- 6 files changed, 88 insertions(+), 96 deletions(-) diff --git a/database/benchmark_database.go b/database/benchmark_database.go index b27eec902caa..9161f05194a8 100644 --- a/database/benchmark_database.go +++ b/database/benchmark_database.go @@ -15,16 +15,16 @@ import ( var ( // Benchmarks is a list of all database benchmarks - Benchmarks = []func(b *testing.B, db Database, name string, keys, values [][]byte){ - BenchmarkGet, - BenchmarkPut, - BenchmarkDelete, - BenchmarkBatchPut, - BenchmarkBatchDelete, - BenchmarkBatchWrite, - BenchmarkParallelGet, - BenchmarkParallelPut, - BenchmarkParallelDelete, + Benchmarks = map[string]func(b *testing.B, db Database, name string, keys, values [][]byte){ + "BenchmarkGet": BenchmarkGet, + "BenchmarkPut": BenchmarkPut, + "BenchmarkDelete": BenchmarkDelete, + "BenchmarkBatchPut": BenchmarkBatchPut, + "BenchmarkBatchDelete": BenchmarkBatchDelete, + "BenchmarkBatchWrite": BenchmarkBatchWrite, + "BenchmarkParallelGet": BenchmarkParallelGet, + "BenchmarkParallelPut": BenchmarkParallelPut, + "BenchmarkParallelDelete": BenchmarkParallelDelete, } // BenchmarkSizes to use with each benchmark BenchmarkSizes = [][]int{ diff --git a/database/rpcdb/db_test.go b/database/rpcdb/db_test.go index 99323f8bd8c1..59dafb90a96d 100644 --- a/database/rpcdb/db_test.go +++ b/database/rpcdb/db_test.go @@ -18,9 +18,8 @@ import ( ) type testDatabase struct { - client *DatabaseClient - server *memdb.Database - closeFn func() + client *DatabaseClient + server *memdb.Database } func setupDB(t testing.TB) *testDatabase { @@ -44,51 +43,48 @@ func setupDB(t testing.TB) *testDatabase { require.NoError(err) db.client = NewClient(rpcdbpb.NewDatabaseClient(conn)) - db.closeFn = func() { + + t.Cleanup(func() { serverCloser.Stop() - _ = conn.Close() - _ = listener.Close() - } + require.NoError(conn.Close()) + require.NoError(listener.Close()) + }) + return db } func TestInterface(t *testing.T) { - for _, test := range database.Tests { - db := setupDB(t) - test(t, db.client) - - db.closeFn() + for name, f := range database.Tests { + t.Run(name, func(t *testing.T) { + db := setupDB(t) + f(t, db.client) + }) } } func FuzzKeyValue(f *testing.F) { db := setupDB(f) database.FuzzKeyValue(f, db.client) - - db.closeFn() } func FuzzNewIteratorWithPrefix(f *testing.F) { db := setupDB(f) database.FuzzNewIteratorWithPrefix(f, db.client) - - db.closeFn() } func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { db := setupDB(f) database.FuzzNewIteratorWithStartAndPrefix(f, db.client) - - db.closeFn() } func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := setupDB(b) - bench(b, db.client, "rpcdb", keys, values) - db.closeFn() + for name, bench := range database.Benchmarks { + b.Run(name, func(b *testing.B) { + db := setupDB(b) + bench(b, db.client, "rpcdb", keys, values) + }) } } } diff --git a/database/test_database.go b/database/test_database.go index 1ce32bb54257..d8fe3013a968 100644 --- a/database/test_database.go +++ b/database/test_database.go @@ -24,44 +24,44 @@ import ( ) // Tests is a list of all database tests -var Tests = []func(t *testing.T, db Database){ - TestSimpleKeyValue, - TestOverwriteKeyValue, - TestEmptyKey, - TestKeyEmptyValue, - TestSimpleKeyValueClosed, - TestNewBatchClosed, - TestBatchPut, - TestBatchDelete, - TestBatchReset, - TestBatchReuse, - TestBatchRewrite, - TestBatchReplay, - TestBatchReplayPropagateError, - TestBatchInner, - TestBatchLargeSize, - TestIteratorSnapshot, - TestIterator, - TestIteratorStart, - TestIteratorPrefix, - TestIteratorStartPrefix, - TestIteratorMemorySafety, - TestIteratorClosed, - TestIteratorError, - TestIteratorErrorAfterRelease, - TestCompactNoPanic, - TestMemorySafetyDatabase, - TestMemorySafetyBatch, - TestAtomicClear, - TestClear, - TestAtomicClearPrefix, - TestClearPrefix, - TestModifyValueAfterPut, - TestModifyValueAfterBatchPut, - TestModifyValueAfterBatchPutReplay, - TestConcurrentBatches, - TestManySmallConcurrentKVPairBatches, - TestPutGetEmpty, +var Tests = map[string]func(t *testing.T, db Database){ + "TestSimpleKeyValue": TestSimpleKeyValue, + "TestOverwriteKeyValue": TestOverwriteKeyValue, + "TestEmptyKey": TestEmptyKey, + "TestKeyEmptyValue": TestKeyEmptyValue, + "TestSimpleKeyValueClosed": TestSimpleKeyValueClosed, + "TestNewBatchClosed": TestNewBatchClosed, + "TestBatchPut": TestBatchPut, + "TestBatchDelete": TestBatchDelete, + "TestBatchReset": TestBatchReset, + "TestBatchReuse": TestBatchReuse, + "TestBatchRewrite": TestBatchRewrite, + "TestBatchReplay": TestBatchReplay, + "TestBatchReplayPropagateError": TestBatchReplayPropagateError, + "TestBatchInner": TestBatchInner, + "TestBatchLargeSize": TestBatchLargeSize, + "TestIteratorSnapshot": TestIteratorSnapshot, + "TestIterator": TestIterator, + "TestIteratorStart": TestIteratorStart, + "TestIteratorPrefix": TestIteratorPrefix, + "TestIteratorStartPrefix": TestIteratorStartPrefix, + "TestIteratorMemorySafety": TestIteratorMemorySafety, + "TestIteratorClosed": TestIteratorClosed, + "TestIteratorError": TestIteratorError, + "TestIteratorErrorAfterRelease": TestIteratorErrorAfterRelease, + "TestCompactNoPanic": TestCompactNoPanic, + "TestMemorySafetyDatabase": TestMemorySafetyDatabase, + "TestMemorySafetyBatch": TestMemorySafetyBatch, + "TestAtomicClear": TestAtomicClear, + "TestClear": TestClear, + "TestAtomicClearPrefix": TestAtomicClearPrefix, + "TestClearPrefix": TestClearPrefix, + "TestModifyValueAfterPut": TestModifyValueAfterPut, + "TestModifyValueAfterBatchPut": TestModifyValueAfterBatchPut, + "TestModifyValueAfterBatchPutReplay": TestModifyValueAfterBatchPutReplay, + "TestConcurrentBatches": TestConcurrentBatches, + "TestManySmallConcurrentKVPairBatches": TestManySmallConcurrentKVPairBatches, + "TestPutGetEmpty": TestPutGetEmpty, } // TestSimpleKeyValue tests to make sure that simple Put + Get + Delete + Has diff --git a/snow/validators/gvalidators/validator_state_test.go b/snow/validators/gvalidators/validator_state_test.go index 9b6e692d8645..12360822d5fa 100644 --- a/snow/validators/gvalidators/validator_state_test.go +++ b/snow/validators/gvalidators/validator_state_test.go @@ -24,9 +24,8 @@ import ( var errCustom = errors.New("custom") type testState struct { - client *Client - server *validators.MockState - closeFn func() + client *Client + server *validators.MockState } func setupState(t testing.TB, ctrl *gomock.Controller) *testState { @@ -52,11 +51,13 @@ func setupState(t testing.TB, ctrl *gomock.Controller) *testState { require.NoError(err) state.client = NewClient(pb.NewValidatorStateClient(conn)) - state.closeFn = func() { + + t.Cleanup(func() { serverCloser.Stop() - _ = conn.Close() - _ = listener.Close() - } + require.NoError(conn.Close()) + require.NoError(listener.Close()) + }) + return state } @@ -65,7 +66,6 @@ func TestGetMinimumHeight(t *testing.T) { ctrl := gomock.NewController(t) state := setupState(t, ctrl) - defer state.closeFn() // Happy path expectedHeight := uint64(1337) @@ -88,7 +88,6 @@ func TestGetCurrentHeight(t *testing.T) { ctrl := gomock.NewController(t) state := setupState(t, ctrl) - defer state.closeFn() // Happy path expectedHeight := uint64(1337) @@ -111,7 +110,6 @@ func TestGetSubnetID(t *testing.T) { ctrl := gomock.NewController(t) state := setupState(t, ctrl) - defer state.closeFn() // Happy path chainID := ids.GenerateTestID() @@ -135,7 +133,6 @@ func TestGetValidatorSet(t *testing.T) { ctrl := gomock.NewController(t) state := setupState(t, ctrl) - defer state.closeFn() // Happy path sk0, err := bls.NewSecretKey() @@ -209,9 +206,6 @@ func benchmarkGetValidatorSet(b *testing.B, vs map[ids.NodeID]*validators.GetVal require := require.New(b) ctrl := gomock.NewController(b) state := setupState(b, ctrl) - defer func() { - state.closeFn() - }() height := uint64(1337) subnetID := ids.GenerateTestID() diff --git a/vms/platformvm/warp/gwarp/signer_test.go b/vms/platformvm/warp/gwarp/signer_test.go index 306067dc883d..52f11880c9b8 100644 --- a/vms/platformvm/warp/gwarp/signer_test.go +++ b/vms/platformvm/warp/gwarp/signer_test.go @@ -23,7 +23,6 @@ type testSigner struct { sk *bls.SecretKey networkID uint32 chainID ids.ID - closeFn func() } func setupSigner(t testing.TB) *testSigner { @@ -55,18 +54,21 @@ func setupSigner(t testing.TB) *testSigner { require.NoError(err) s.client = NewClient(pb.NewSignerClient(conn)) - s.closeFn = func() { + + t.Cleanup(func() { serverCloser.Stop() - _ = conn.Close() - _ = listener.Close() - } + require.NoError(conn.Close()) + require.NoError(listener.Close()) + }) + return s } func TestInterface(t *testing.T) { - for _, test := range warp.SignerTests { - s := setupSigner(t) - test(t, s.client, s.sk, s.networkID, s.chainID) - s.closeFn() + for name, f := range warp.SignerTests { + t.Run(name, func(t *testing.T) { + s := setupSigner(t) + f(t, s.client, s.sk, s.networkID, s.chainID) + }) } } diff --git a/vms/platformvm/warp/test_signer.go b/vms/platformvm/warp/test_signer.go index c17b15b215e2..2cbc7bdbb4b3 100644 --- a/vms/platformvm/warp/test_signer.go +++ b/vms/platformvm/warp/test_signer.go @@ -14,9 +14,9 @@ import ( ) // SignerTests is a list of all signer tests -var SignerTests = []func(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID){ - TestSignerWrongChainID, - TestSignerVerifies, +var SignerTests = map[string]func(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID){ + "TestSignerWrongChainID": TestSignerWrongChainID, + "TestSignerVerifies": TestSignerVerifies, } // Test that using a random SourceChainID results in an error From 2dfd32219424c101d0e8f0712d805736ece69c24 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:38:54 -0500 Subject: [PATCH 2/7] wip --- database/benchmark_database.go | 18 ++--- database/rpcdb/db_test.go | 4 +- database/test_database.go | 74 +++++++++---------- .../gvalidators/validator_state_test.go | 4 +- vms/platformvm/warp/gwarp/signer_test.go | 12 ++- vms/platformvm/warp/test_signer.go | 8 +- 6 files changed, 59 insertions(+), 61 deletions(-) diff --git a/database/benchmark_database.go b/database/benchmark_database.go index 9161f05194a8..9e1a04c464a7 100644 --- a/database/benchmark_database.go +++ b/database/benchmark_database.go @@ -16,15 +16,15 @@ import ( var ( // Benchmarks is a list of all database benchmarks Benchmarks = map[string]func(b *testing.B, db Database, name string, keys, values [][]byte){ - "BenchmarkGet": BenchmarkGet, - "BenchmarkPut": BenchmarkPut, - "BenchmarkDelete": BenchmarkDelete, - "BenchmarkBatchPut": BenchmarkBatchPut, - "BenchmarkBatchDelete": BenchmarkBatchDelete, - "BenchmarkBatchWrite": BenchmarkBatchWrite, - "BenchmarkParallelGet": BenchmarkParallelGet, - "BenchmarkParallelPut": BenchmarkParallelPut, - "BenchmarkParallelDelete": BenchmarkParallelDelete, + "Get": BenchmarkGet, + "Put": BenchmarkPut, + "Delete": BenchmarkDelete, + "BatchPut": BenchmarkBatchPut, + "BatchDelete": BenchmarkBatchDelete, + "BatchWrite": BenchmarkBatchWrite, + "ParallelGet": BenchmarkParallelGet, + "ParallelPut": BenchmarkParallelPut, + "ParallelDelete": BenchmarkParallelDelete, } // BenchmarkSizes to use with each benchmark BenchmarkSizes = [][]int{ diff --git a/database/rpcdb/db_test.go b/database/rpcdb/db_test.go index 59dafb90a96d..57913a64dc13 100644 --- a/database/rpcdb/db_test.go +++ b/database/rpcdb/db_test.go @@ -46,8 +46,8 @@ func setupDB(t testing.TB) *testDatabase { t.Cleanup(func() { serverCloser.Stop() - require.NoError(conn.Close()) - require.NoError(listener.Close()) + _ = conn.Close() + _ = listener.Close() }) return db diff --git a/database/test_database.go b/database/test_database.go index d8fe3013a968..792b038012cb 100644 --- a/database/test_database.go +++ b/database/test_database.go @@ -25,43 +25,43 @@ import ( // Tests is a list of all database tests var Tests = map[string]func(t *testing.T, db Database){ - "TestSimpleKeyValue": TestSimpleKeyValue, - "TestOverwriteKeyValue": TestOverwriteKeyValue, - "TestEmptyKey": TestEmptyKey, - "TestKeyEmptyValue": TestKeyEmptyValue, - "TestSimpleKeyValueClosed": TestSimpleKeyValueClosed, - "TestNewBatchClosed": TestNewBatchClosed, - "TestBatchPut": TestBatchPut, - "TestBatchDelete": TestBatchDelete, - "TestBatchReset": TestBatchReset, - "TestBatchReuse": TestBatchReuse, - "TestBatchRewrite": TestBatchRewrite, - "TestBatchReplay": TestBatchReplay, - "TestBatchReplayPropagateError": TestBatchReplayPropagateError, - "TestBatchInner": TestBatchInner, - "TestBatchLargeSize": TestBatchLargeSize, - "TestIteratorSnapshot": TestIteratorSnapshot, - "TestIterator": TestIterator, - "TestIteratorStart": TestIteratorStart, - "TestIteratorPrefix": TestIteratorPrefix, - "TestIteratorStartPrefix": TestIteratorStartPrefix, - "TestIteratorMemorySafety": TestIteratorMemorySafety, - "TestIteratorClosed": TestIteratorClosed, - "TestIteratorError": TestIteratorError, - "TestIteratorErrorAfterRelease": TestIteratorErrorAfterRelease, - "TestCompactNoPanic": TestCompactNoPanic, - "TestMemorySafetyDatabase": TestMemorySafetyDatabase, - "TestMemorySafetyBatch": TestMemorySafetyBatch, - "TestAtomicClear": TestAtomicClear, - "TestClear": TestClear, - "TestAtomicClearPrefix": TestAtomicClearPrefix, - "TestClearPrefix": TestClearPrefix, - "TestModifyValueAfterPut": TestModifyValueAfterPut, - "TestModifyValueAfterBatchPut": TestModifyValueAfterBatchPut, - "TestModifyValueAfterBatchPutReplay": TestModifyValueAfterBatchPutReplay, - "TestConcurrentBatches": TestConcurrentBatches, - "TestManySmallConcurrentKVPairBatches": TestManySmallConcurrentKVPairBatches, - "TestPutGetEmpty": TestPutGetEmpty, + "SimpleKeyValue": TestSimpleKeyValue, + "OverwriteKeyValue": TestOverwriteKeyValue, + "EmptyKey": TestEmptyKey, + "KeyEmptyValue": TestKeyEmptyValue, + "SimpleKeyValueClosed": TestSimpleKeyValueClosed, + "NewBatchClosed": TestNewBatchClosed, + "BatchPut": TestBatchPut, + "BatchDelete": TestBatchDelete, + "BatchReset": TestBatchReset, + "BatchReuse": TestBatchReuse, + "BatchRewrite": TestBatchRewrite, + "BatchReplay": TestBatchReplay, + "BatchReplayPropagateError": TestBatchReplayPropagateError, + "BatchInner": TestBatchInner, + "BatchLargeSize": TestBatchLargeSize, + "IteratorSnapshot": TestIteratorSnapshot, + "Iterator": TestIterator, + "IteratorStart": TestIteratorStart, + "IteratorPrefix": TestIteratorPrefix, + "IteratorStartPrefix": TestIteratorStartPrefix, + "IteratorMemorySafety": TestIteratorMemorySafety, + "IteratorClosed": TestIteratorClosed, + "IteratorError": TestIteratorError, + "IteratorErrorAfterRelease": TestIteratorErrorAfterRelease, + "CompactNoPanic": TestCompactNoPanic, + "MemorySafetyDatabase": TestMemorySafetyDatabase, + "MemorySafetyBatch": TestMemorySafetyBatch, + "AtomicClear": TestAtomicClear, + "Clear": TestClear, + "AtomicClearPrefix": TestAtomicClearPrefix, + "ClearPrefix": TestClearPrefix, + "ModifyValueAfterPut": TestModifyValueAfterPut, + "ModifyValueAfterBatchPut": TestModifyValueAfterBatchPut, + "ModifyValueAfterBatchPutReplay": TestModifyValueAfterBatchPutReplay, + "ConcurrentBatches": TestConcurrentBatches, + "ManySmallConcurrentKVPairBatches": TestManySmallConcurrentKVPairBatches, + "PutGetEmpty": TestPutGetEmpty, } // TestSimpleKeyValue tests to make sure that simple Put + Get + Delete + Has diff --git a/snow/validators/gvalidators/validator_state_test.go b/snow/validators/gvalidators/validator_state_test.go index 12360822d5fa..0dbf9ebe8952 100644 --- a/snow/validators/gvalidators/validator_state_test.go +++ b/snow/validators/gvalidators/validator_state_test.go @@ -54,8 +54,8 @@ func setupState(t testing.TB, ctrl *gomock.Controller) *testState { t.Cleanup(func() { serverCloser.Stop() - require.NoError(conn.Close()) - require.NoError(listener.Close()) + _ = conn.Close() + _ = listener.Close() }) return state diff --git a/vms/platformvm/warp/gwarp/signer_test.go b/vms/platformvm/warp/gwarp/signer_test.go index 52f11880c9b8..876eb2edb74e 100644 --- a/vms/platformvm/warp/gwarp/signer_test.go +++ b/vms/platformvm/warp/gwarp/signer_test.go @@ -57,18 +57,16 @@ func setupSigner(t testing.TB) *testSigner { t.Cleanup(func() { serverCloser.Stop() - require.NoError(conn.Close()) - require.NoError(listener.Close()) + _ = conn.Close() + _ = listener.Close() }) return s } func TestInterface(t *testing.T) { - for name, f := range warp.SignerTests { - t.Run(name, func(t *testing.T) { - s := setupSigner(t) - f(t, s.client, s.sk, s.networkID, s.chainID) - }) + for _, test := range warp.SignerTests { + s := setupSigner(t) + test(t, s.client, s.sk, s.networkID, s.chainID) } } diff --git a/vms/platformvm/warp/test_signer.go b/vms/platformvm/warp/test_signer.go index 2cbc7bdbb4b3..e3b82ace8a3a 100644 --- a/vms/platformvm/warp/test_signer.go +++ b/vms/platformvm/warp/test_signer.go @@ -15,12 +15,12 @@ import ( // SignerTests is a list of all signer tests var SignerTests = map[string]func(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID){ - "TestSignerWrongChainID": TestSignerWrongChainID, - "TestSignerVerifies": TestSignerVerifies, + "WrongChainID": TestWrongChainID, + "Verifies": TestVerifies, } // Test that using a random SourceChainID results in an error -func TestSignerWrongChainID(t *testing.T, s Signer, _ *bls.SecretKey, _ uint32, _ ids.ID) { +func TestWrongChainID(t *testing.T, s Signer, _ *bls.SecretKey, _ uint32, _ ids.ID) { require := require.New(t) msg, err := NewUnsignedMessage( @@ -52,7 +52,7 @@ func TestSignerWrongNetworkID(t *testing.T, s Signer, _ *bls.SecretKey, networkI } // Test that a signature generated with the signer verifies correctly -func TestSignerVerifies(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID) { +func TestVerifies(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID) { require := require.New(t) msg, err := NewUnsignedMessage( From f69c69db1c3fc8fd58cbea9ba33f92d995c38fe3 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:41:07 -0500 Subject: [PATCH 3/7] reduce diff --- database/rpcdb/db_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/database/rpcdb/db_test.go b/database/rpcdb/db_test.go index 57913a64dc13..926c92e6540e 100644 --- a/database/rpcdb/db_test.go +++ b/database/rpcdb/db_test.go @@ -54,11 +54,9 @@ func setupDB(t testing.TB) *testDatabase { } func TestInterface(t *testing.T) { - for name, f := range database.Tests { - t.Run(name, func(t *testing.T) { - db := setupDB(t) - f(t, db.client) - }) + for _, test := range database.Tests { + db := setupDB(t) + test(t, db.client) } } @@ -80,11 +78,9 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for name, bench := range database.Benchmarks { - b.Run(name, func(b *testing.B) { - db := setupDB(b) - bench(b, db.client, "rpcdb", keys, values) - }) + for _, bench := range database.Benchmarks { + db := setupDB(b) + bench(b, db.client, "rpcdb", keys, values) } } } From d8b42314c629b5eebf9e25e547a959780b503940 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 14:57:30 -0500 Subject: [PATCH 4/7] Cleanup warp signer tests --- vms/platformvm/warp/gwarp/signer_test.go | 10 ++++++---- vms/platformvm/warp/signer_test.go | 14 ++++++++------ vms/platformvm/warp/test_signer.go | 13 +++++++------ 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/vms/platformvm/warp/gwarp/signer_test.go b/vms/platformvm/warp/gwarp/signer_test.go index 306067dc883d..938d0ed07fa5 100644 --- a/vms/platformvm/warp/gwarp/signer_test.go +++ b/vms/platformvm/warp/gwarp/signer_test.go @@ -64,9 +64,11 @@ func setupSigner(t testing.TB) *testSigner { } func TestInterface(t *testing.T) { - for _, test := range warp.SignerTests { - s := setupSigner(t) - test(t, s.client, s.sk, s.networkID, s.chainID) - s.closeFn() + for name, test := range warp.SignerTests { + t.Run(name, func(t *testing.T) { + s := setupSigner(t) + test(t, s.client, s.sk, s.networkID, s.chainID) + s.closeFn() + }) } } diff --git a/vms/platformvm/warp/signer_test.go b/vms/platformvm/warp/signer_test.go index 1bc177872d87..84b51f6574fa 100644 --- a/vms/platformvm/warp/signer_test.go +++ b/vms/platformvm/warp/signer_test.go @@ -14,13 +14,15 @@ import ( ) func TestSigner(t *testing.T) { - for _, test := range SignerTests { - sk, err := bls.NewSecretKey() - require.NoError(t, err) + for name, test := range SignerTests { + t.Run(name, func(t *testing.T) { + sk, err := bls.NewSecretKey() + require.NoError(t, err) - chainID := ids.GenerateTestID() - s := NewSigner(sk, constants.UnitTestID, chainID) + chainID := ids.GenerateTestID() + s := NewSigner(sk, constants.UnitTestID, chainID) - test(t, s, sk, constants.UnitTestID, chainID) + test(t, s, sk, constants.UnitTestID, chainID) + }) } } diff --git a/vms/platformvm/warp/test_signer.go b/vms/platformvm/warp/test_signer.go index c17b15b215e2..e30423edf1ed 100644 --- a/vms/platformvm/warp/test_signer.go +++ b/vms/platformvm/warp/test_signer.go @@ -14,13 +14,14 @@ import ( ) // SignerTests is a list of all signer tests -var SignerTests = []func(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID){ - TestSignerWrongChainID, - TestSignerVerifies, +var SignerTests = map[string]func(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID){ + "WrongChainID": TestWrongChainID, + "WrongNetworkID": TestWrongNetworkID, + "Verifies": TestVerifies, } // Test that using a random SourceChainID results in an error -func TestSignerWrongChainID(t *testing.T, s Signer, _ *bls.SecretKey, _ uint32, _ ids.ID) { +func TestWrongChainID(t *testing.T, s Signer, _ *bls.SecretKey, _ uint32, _ ids.ID) { require := require.New(t) msg, err := NewUnsignedMessage( @@ -36,7 +37,7 @@ func TestSignerWrongChainID(t *testing.T, s Signer, _ *bls.SecretKey, _ uint32, } // Test that using a different networkID results in an error -func TestSignerWrongNetworkID(t *testing.T, s Signer, _ *bls.SecretKey, networkID uint32, blockchainID ids.ID) { +func TestWrongNetworkID(t *testing.T, s Signer, _ *bls.SecretKey, networkID uint32, blockchainID ids.ID) { require := require.New(t) msg, err := NewUnsignedMessage( @@ -52,7 +53,7 @@ func TestSignerWrongNetworkID(t *testing.T, s Signer, _ *bls.SecretKey, networkI } // Test that a signature generated with the signer verifies correctly -func TestSignerVerifies(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID) { +func TestVerifies(t *testing.T, s Signer, sk *bls.SecretKey, networkID uint32, chainID ids.ID) { require := require.New(t) msg, err := NewUnsignedMessage( From 3f9f7675a22bc648a215ad13b34d9d48c42dd499 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:25:15 -0500 Subject: [PATCH 5/7] Cleanup database benchmarks --- database/benchmark_database.go | 200 +++++++++++++++------------------ database/encdb/db_test.go | 7 +- database/leveldb/db_test.go | 15 ++- database/memdb/db_test.go | 9 +- database/meterdb/db_test.go | 7 +- database/pebble/db_test.go | 11 +- database/prefixdb/db_test.go | 9 +- database/rpcdb/db_test.go | 11 +- database/versiondb/db_test.go | 13 ++- x/merkledb/db_test.go | 10 +- 10 files changed, 149 insertions(+), 143 deletions(-) diff --git a/database/benchmark_database.go b/database/benchmark_database.go index b27eec902caa..80ca32dd069c 100644 --- a/database/benchmark_database.go +++ b/database/benchmark_database.go @@ -4,7 +4,6 @@ package database import ( - "fmt" "math/rand" "testing" @@ -15,16 +14,16 @@ import ( var ( // Benchmarks is a list of all database benchmarks - Benchmarks = []func(b *testing.B, db Database, name string, keys, values [][]byte){ - BenchmarkGet, - BenchmarkPut, - BenchmarkDelete, - BenchmarkBatchPut, - BenchmarkBatchDelete, - BenchmarkBatchWrite, - BenchmarkParallelGet, - BenchmarkParallelPut, - BenchmarkParallelDelete, + Benchmarks = map[string]func(b *testing.B, db Database, keys, values [][]byte){ + "Get": BenchmarkGet, + "Put": BenchmarkPut, + "Delete": BenchmarkDelete, + "BatchPut": BenchmarkBatchPut, + "BatchDelete": BenchmarkBatchDelete, + "BatchWrite": BenchmarkBatchWrite, + "ParallelGet": BenchmarkParallelGet, + "ParallelPut": BenchmarkParallelPut, + "ParallelDelete": BenchmarkParallelDelete, } // BenchmarkSizes to use with each benchmark BenchmarkSizes = [][]int{ @@ -56,169 +55,150 @@ func SetupBenchmark(b *testing.B, count int, keySize, valueSize int) ([][]byte, } // BenchmarkGet measures the time it takes to get an operation from a database. -func BenchmarkGet(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkGet(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.get", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - require := require.New(b) + require := require.New(b) - for i, key := range keys { - value := values[i] - require.NoError(db.Put(key, value)) - } + for i, key := range keys { + value := values[i] + require.NoError(db.Put(key, value)) + } - b.ResetTimer() + b.ResetTimer() - // Reads b.N values from the db - for i := 0; i < b.N; i++ { - _, err := db.Get(keys[i%count]) - require.NoError(err) - } - }) + // Reads b.N values from the db + for i := 0; i < b.N; i++ { + _, err := db.Get(keys[i%count]) + require.NoError(err) + } } // BenchmarkPut measures the time it takes to write an operation to a database. -func BenchmarkPut(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkPut(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.put", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - // Writes b.N values to the db - for i := 0; i < b.N; i++ { - require.NoError(b, db.Put(keys[i%count], values[i%count])) - } - }) + // Writes b.N values to the db + for i := 0; i < b.N; i++ { + require.NoError(b, db.Put(keys[i%count], values[i%count])) + } } // BenchmarkDelete measures the time it takes to delete a (k, v) from a database. -func BenchmarkDelete(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkDelete(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.delete", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - require := require.New(b) + require := require.New(b) - // Writes random values of size _size_ to the database - for i, key := range keys { - value := values[i] - require.NoError(db.Put(key, value)) - } + // Writes random values of size _size_ to the database + for i, key := range keys { + value := values[i] + require.NoError(db.Put(key, value)) + } - b.ResetTimer() + b.ResetTimer() - // Deletes b.N values from the db - for i := 0; i < b.N; i++ { - require.NoError(db.Delete(keys[i%count])) - } - }) + // Deletes b.N values from the db + for i := 0; i < b.N; i++ { + require.NoError(db.Delete(keys[i%count])) + } } // BenchmarkBatchPut measures the time it takes to batch put. -func BenchmarkBatchPut(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkBatchPut(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_batch.put", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - batch := db.NewBatch() - for i := 0; i < b.N; i++ { - require.NoError(b, batch.Put(keys[i%count], values[i%count])) - } - }) + batch := db.NewBatch() + for i := 0; i < b.N; i++ { + require.NoError(b, batch.Put(keys[i%count], values[i%count])) + } } // BenchmarkBatchDelete measures the time it takes to batch delete. -func BenchmarkBatchDelete(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkBatchDelete(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_batch.delete", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - batch := db.NewBatch() - for i := 0; i < b.N; i++ { - require.NoError(b, batch.Delete(keys[i%count])) - } - }) + batch := db.NewBatch() + for i := 0; i < b.N; i++ { + require.NoError(b, batch.Delete(keys[i%count])) + } } // BenchmarkBatchWrite measures the time it takes to batch write. -func BenchmarkBatchWrite(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkBatchWrite(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) - count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_batch.write", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - require := require.New(b) + require := require.New(b) - batch := db.NewBatch() - for i, key := range keys { - value := values[i] - require.NoError(batch.Put(key, value)) - } + batch := db.NewBatch() + for i, key := range keys { + value := values[i] + require.NoError(batch.Put(key, value)) + } - b.ResetTimer() + b.ResetTimer() - for i := 0; i < b.N; i++ { - require.NoError(batch.Write()) - } - }) + for i := 0; i < b.N; i++ { + require.NoError(batch.Write()) + } } // BenchmarkParallelGet measures the time it takes to read in parallel. -func BenchmarkParallelGet(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkParallelGet(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.get_parallel", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - require := require.New(b) + require := require.New(b) - for i, key := range keys { - value := values[i] - require.NoError(db.Put(key, value)) - } + for i, key := range keys { + value := values[i] + require.NoError(db.Put(key, value)) + } - b.ResetTimer() + b.ResetTimer() - b.RunParallel(func(pb *testing.PB) { - for i := 0; pb.Next(); i++ { - _, err := db.Get(keys[i%count]) - require.NoError(err) - } - }) + b.RunParallel(func(pb *testing.PB) { + for i := 0; pb.Next(); i++ { + _, err := db.Get(keys[i%count]) + require.NoError(err) + } }) } // BenchmarkParallelPut measures the time it takes to write to the db in parallel. -func BenchmarkParallelPut(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkParallelPut(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.put_parallel", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - b.RunParallel(func(pb *testing.PB) { - // Write N values to the db - for i := 0; pb.Next(); i++ { - require.NoError(b, db.Put(keys[i%count], values[i%count])) - } - }) + b.RunParallel(func(pb *testing.PB) { + // Write N values to the db + for i := 0; pb.Next(); i++ { + require.NoError(b, db.Put(keys[i%count], values[i%count])) + } }) } // BenchmarkParallelDelete measures the time it takes to delete a (k, v) from the db. -func BenchmarkParallelDelete(b *testing.B, db Database, name string, keys, values [][]byte) { +func BenchmarkParallelDelete(b *testing.B, db Database, keys, values [][]byte) { require.NotEmpty(b, keys) count := len(keys) - b.Run(fmt.Sprintf("%s_%d_pairs_%d_keys_%d_values_db.delete_parallel", name, count, len(keys[0]), len(values[0])), func(b *testing.B) { - require := require.New(b) - for i, key := range keys { - value := values[i] - require.NoError(db.Put(key, value)) + require := require.New(b) + for i, key := range keys { + value := values[i] + require.NoError(db.Put(key, value)) + } + b.ResetTimer() + + b.RunParallel(func(pb *testing.PB) { + // Deletes b.N values from the db + for i := 0; pb.Next(); i++ { + require.NoError(db.Delete(keys[i%count])) } - b.ResetTimer() - - b.RunParallel(func(pb *testing.PB) { - // Deletes b.N values from the db - for i := 0; pb.Next(); i++ { - require.NoError(db.Delete(keys[i%count])) - } - }) }) } diff --git a/database/encdb/db_test.go b/database/encdb/db_test.go index 871460a90a41..d659bd3ba455 100644 --- a/database/encdb/db_test.go +++ b/database/encdb/db_test.go @@ -4,6 +4,7 @@ package encdb import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -46,8 +47,10 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - bench(b, newDB(b), "encdb", keys, values) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("encdb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + bench(b, newDB(b), keys, values) + }) } } } diff --git a/database/leveldb/db_test.go b/database/leveldb/db_test.go index bf70739ce222..748958086d7c 100644 --- a/database/leveldb/db_test.go +++ b/database/leveldb/db_test.go @@ -4,6 +4,7 @@ package leveldb import ( + "fmt" "testing" "github.com/prometheus/client_golang/prometheus" @@ -57,14 +58,16 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := newDB(b) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("leveldb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + db := newDB(b) - bench(b, db, "leveldb", keys, values) + bench(b, db, keys, values) - // The database may have been closed by the test, so we don't care if it - // errors here. - _ = db.Close() + // The database may have been closed by the test, so we don't care if it + // errors here. + _ = db.Close() + }) } } } diff --git a/database/memdb/db_test.go b/database/memdb/db_test.go index 21c97909c7fb..7408409f13fd 100644 --- a/database/memdb/db_test.go +++ b/database/memdb/db_test.go @@ -4,6 +4,7 @@ package memdb import ( + "fmt" "testing" "github.com/ava-labs/avalanchego/database" @@ -30,9 +31,11 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := New() - bench(b, db, "memdb", keys, values) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("memdb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + db := New() + bench(b, db, keys, values) + }) } } } diff --git a/database/meterdb/db_test.go b/database/meterdb/db_test.go index cd36a6fb34f2..f11944b39df5 100644 --- a/database/meterdb/db_test.go +++ b/database/meterdb/db_test.go @@ -4,6 +4,7 @@ package meterdb import ( + "fmt" "testing" "github.com/prometheus/client_golang/prometheus" @@ -46,8 +47,10 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - bench(b, newDB(b), "meterdb", keys, values) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("meterdb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + bench(b, newDB(b), keys, values) + }) } } } diff --git a/database/pebble/db_test.go b/database/pebble/db_test.go index 2ea3c354f3d8..357f5db58119 100644 --- a/database/pebble/db_test.go +++ b/database/pebble/db_test.go @@ -4,6 +4,7 @@ package pebble import ( + "fmt" "testing" "github.com/prometheus/client_golang/prometheus" @@ -50,10 +51,12 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := newDB(b) - bench(b, db, "pebble", keys, values) - _ = db.Close() + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("pebble_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + db := newDB(b) + bench(b, db, keys, values) + _ = db.Close() + }) } } } diff --git a/database/prefixdb/db_test.go b/database/prefixdb/db_test.go index 109d8c4c9aba..7cc1ee25b878 100644 --- a/database/prefixdb/db_test.go +++ b/database/prefixdb/db_test.go @@ -4,6 +4,7 @@ package prefixdb import ( + "fmt" "testing" "github.com/ava-labs/avalanchego/database" @@ -37,9 +38,11 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := New([]byte("hello"), memdb.New()) - bench(b, db, "prefixdb", keys, values) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("prefixdb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + db := New([]byte("hello"), memdb.New()) + bench(b, db, keys, values) + }) } } } diff --git a/database/rpcdb/db_test.go b/database/rpcdb/db_test.go index 99323f8bd8c1..6fe2ea02125c 100644 --- a/database/rpcdb/db_test.go +++ b/database/rpcdb/db_test.go @@ -5,6 +5,7 @@ package rpcdb import ( "context" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -85,10 +86,12 @@ func FuzzNewIteratorWithStartAndPrefix(f *testing.F) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - db := setupDB(b) - bench(b, db.client, "rpcdb", keys, values) - db.closeFn() + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("rpcdb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + db := setupDB(b) + bench(b, db.client, keys, values) + db.closeFn() + }) } } } diff --git a/database/versiondb/db_test.go b/database/versiondb/db_test.go index c3093a9ee843..fb94eae01fd6 100644 --- a/database/versiondb/db_test.go +++ b/database/versiondb/db_test.go @@ -4,6 +4,7 @@ package versiondb import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -299,11 +300,13 @@ func TestSetDatabaseClosed(t *testing.T) { func BenchmarkInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) - for _, bench := range database.Benchmarks { - baseDB := memdb.New() - db := New(baseDB) - bench(b, db, "versiondb", keys, values) - _ = db.Close() + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("versiondb_%d_pairs_%d_keys_%d_values_%s", size[0], size[1], size[2], name), func(b *testing.B) { + baseDB := memdb.New() + db := New(baseDB) + bench(b, db, keys, values) + _ = db.Close() + }) } } } diff --git a/x/merkledb/db_test.go b/x/merkledb/db_test.go index 244858aeccc8..6329f0e4be97 100644 --- a/x/merkledb/db_test.go +++ b/x/merkledb/db_test.go @@ -111,10 +111,12 @@ func Benchmark_MerkleDB_DBInterface(b *testing.B) { for _, size := range database.BenchmarkSizes { keys, values := database.SetupBenchmark(b, size[0], size[1], size[2]) for _, bf := range validBranchFactors { - for _, bench := range database.Benchmarks { - db, err := getBasicDBWithBranchFactor(bf) - require.NoError(b, err) - bench(b, db, fmt.Sprintf("merkledb_%d", bf), keys, values) + for name, bench := range database.Benchmarks { + b.Run(fmt.Sprintf("merkledb_%d_%d_pairs_%d_keys_%d_values_%s", bf, size[0], size[1], size[2], name), func(b *testing.B) { + db, err := getBasicDBWithBranchFactor(bf) + require.NoError(b, err) + bench(b, db, keys, values) + }) } } } From 452bf22857d835763031d545cb6107c2e3682398 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:33:40 -0500 Subject: [PATCH 6/7] nit --- database/benchmark_database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/benchmark_database.go b/database/benchmark_database.go index 80ca32dd069c..43af10db1c2b 100644 --- a/database/benchmark_database.go +++ b/database/benchmark_database.go @@ -119,7 +119,7 @@ func BenchmarkBatchPut(b *testing.B, db Database, keys, values [][]byte) { } // BenchmarkBatchDelete measures the time it takes to batch delete. -func BenchmarkBatchDelete(b *testing.B, db Database, keys, values [][]byte) { +func BenchmarkBatchDelete(b *testing.B, db Database, keys, _ [][]byte) { require.NotEmpty(b, keys) count := len(keys) From 5af621f422189c34646874668e951d28e556c2d9 Mon Sep 17 00:00:00 2001 From: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com> Date: Tue, 23 Jan 2024 15:39:54 -0500 Subject: [PATCH 7/7] Cleanup database tests --- database/corruptabledb/db_test.go | 16 +++---- database/encdb/db_test.go | 14 +++--- database/leveldb/db_test.go | 14 +++--- database/memdb/db_test.go | 6 ++- database/meterdb/db_test.go | 14 +++--- database/pebble/db_test.go | 10 ++-- database/prefixdb/db_test.go | 18 ++++---- database/rpcdb/db_test.go | 10 ++-- database/test_database.go | 76 +++++++++++++++---------------- database/versiondb/db_test.go | 8 ++-- x/merkledb/db_test.go | 10 ++-- 11 files changed, 107 insertions(+), 89 deletions(-) diff --git a/database/corruptabledb/db_test.go b/database/corruptabledb/db_test.go index 566a6b084e19..5c7a48a64c4d 100644 --- a/database/corruptabledb/db_test.go +++ b/database/corruptabledb/db_test.go @@ -18,19 +18,19 @@ import ( var errTest = errors.New("non-nil error") -func TestInterface(t *testing.T) { - for _, test := range database.Tests { - baseDB := memdb.New() - db := New(baseDB) - test(t, db) - } -} - func newDB() *Database { baseDB := memdb.New() return New(baseDB) } +func TestInterface(t *testing.T) { + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + test(t, newDB()) + }) + } +} + func FuzzKeyValue(f *testing.F) { database.FuzzKeyValue(f, newDB()) } diff --git a/database/encdb/db_test.go b/database/encdb/db_test.go index d659bd3ba455..b3dfdfed68e7 100644 --- a/database/encdb/db_test.go +++ b/database/encdb/db_test.go @@ -16,12 +16,14 @@ import ( const testPassword = "lol totally a secure password" //nolint:gosec func TestInterface(t *testing.T) { - for _, test := range database.Tests { - unencryptedDB := memdb.New() - db, err := New([]byte(testPassword), unencryptedDB) - require.NoError(t, err) - - test(t, db) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + unencryptedDB := memdb.New() + db, err := New([]byte(testPassword), unencryptedDB) + require.NoError(t, err) + + test(t, db) + }) } } diff --git a/database/leveldb/db_test.go b/database/leveldb/db_test.go index 748958086d7c..ad8d60cbbc4f 100644 --- a/database/leveldb/db_test.go +++ b/database/leveldb/db_test.go @@ -16,14 +16,16 @@ import ( ) func TestInterface(t *testing.T) { - for _, test := range database.Tests { - folder := t.TempDir() - db, err := New(folder, nil, logging.NoLog{}, "", prometheus.NewRegistry()) - require.NoError(t, err) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + folder := t.TempDir() + db, err := New(folder, nil, logging.NoLog{}, "", prometheus.NewRegistry()) + require.NoError(t, err) - test(t, db) + test(t, db) - _ = db.Close() + _ = db.Close() + }) } } diff --git a/database/memdb/db_test.go b/database/memdb/db_test.go index 7408409f13fd..90dc459f3602 100644 --- a/database/memdb/db_test.go +++ b/database/memdb/db_test.go @@ -11,8 +11,10 @@ import ( ) func TestInterface(t *testing.T) { - for _, test := range database.Tests { - test(t, New()) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + test(t, New()) + }) } } diff --git a/database/meterdb/db_test.go b/database/meterdb/db_test.go index f11944b39df5..eee3f1c23c01 100644 --- a/database/meterdb/db_test.go +++ b/database/meterdb/db_test.go @@ -16,12 +16,14 @@ import ( ) func TestInterface(t *testing.T) { - for _, test := range database.Tests { - baseDB := memdb.New() - db, err := New("", prometheus.NewRegistry(), baseDB) - require.NoError(t, err) - - test(t, db) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + baseDB := memdb.New() + db, err := New("", prometheus.NewRegistry(), baseDB) + require.NoError(t, err) + + test(t, db) + }) } } diff --git a/database/pebble/db_test.go b/database/pebble/db_test.go index 357f5db58119..5c8650dbf183 100644 --- a/database/pebble/db_test.go +++ b/database/pebble/db_test.go @@ -23,10 +23,12 @@ func newDB(t testing.TB) *Database { } func TestInterface(t *testing.T) { - for _, test := range database.Tests { - db := newDB(t) - test(t, db) - _ = db.Close() + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + db := newDB(t) + test(t, db) + _ = db.Close() + }) } } diff --git a/database/prefixdb/db_test.go b/database/prefixdb/db_test.go index 7cc1ee25b878..f928d2f635a4 100644 --- a/database/prefixdb/db_test.go +++ b/database/prefixdb/db_test.go @@ -12,14 +12,16 @@ import ( ) func TestInterface(t *testing.T) { - for _, test := range database.Tests { - db := memdb.New() - test(t, New([]byte("hello"), db)) - test(t, New([]byte("world"), db)) - test(t, New([]byte("wor"), New([]byte("ld"), db))) - test(t, New([]byte("ld"), New([]byte("wor"), db))) - test(t, NewNested([]byte("wor"), New([]byte("ld"), db))) - test(t, NewNested([]byte("ld"), New([]byte("wor"), db))) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + db := memdb.New() + test(t, New([]byte("hello"), db)) + test(t, New([]byte("world"), db)) + test(t, New([]byte("wor"), New([]byte("ld"), db))) + test(t, New([]byte("ld"), New([]byte("wor"), db))) + test(t, NewNested([]byte("wor"), New([]byte("ld"), db))) + test(t, NewNested([]byte("ld"), New([]byte("wor"), db))) + }) } } diff --git a/database/rpcdb/db_test.go b/database/rpcdb/db_test.go index 6fe2ea02125c..a309ea01298b 100644 --- a/database/rpcdb/db_test.go +++ b/database/rpcdb/db_test.go @@ -54,11 +54,13 @@ func setupDB(t testing.TB) *testDatabase { } func TestInterface(t *testing.T) { - for _, test := range database.Tests { - db := setupDB(t) - test(t, db.client) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + db := setupDB(t) + test(t, db.client) - db.closeFn() + db.closeFn() + }) } } diff --git a/database/test_database.go b/database/test_database.go index 1ce32bb54257..792b038012cb 100644 --- a/database/test_database.go +++ b/database/test_database.go @@ -24,44 +24,44 @@ import ( ) // Tests is a list of all database tests -var Tests = []func(t *testing.T, db Database){ - TestSimpleKeyValue, - TestOverwriteKeyValue, - TestEmptyKey, - TestKeyEmptyValue, - TestSimpleKeyValueClosed, - TestNewBatchClosed, - TestBatchPut, - TestBatchDelete, - TestBatchReset, - TestBatchReuse, - TestBatchRewrite, - TestBatchReplay, - TestBatchReplayPropagateError, - TestBatchInner, - TestBatchLargeSize, - TestIteratorSnapshot, - TestIterator, - TestIteratorStart, - TestIteratorPrefix, - TestIteratorStartPrefix, - TestIteratorMemorySafety, - TestIteratorClosed, - TestIteratorError, - TestIteratorErrorAfterRelease, - TestCompactNoPanic, - TestMemorySafetyDatabase, - TestMemorySafetyBatch, - TestAtomicClear, - TestClear, - TestAtomicClearPrefix, - TestClearPrefix, - TestModifyValueAfterPut, - TestModifyValueAfterBatchPut, - TestModifyValueAfterBatchPutReplay, - TestConcurrentBatches, - TestManySmallConcurrentKVPairBatches, - TestPutGetEmpty, +var Tests = map[string]func(t *testing.T, db Database){ + "SimpleKeyValue": TestSimpleKeyValue, + "OverwriteKeyValue": TestOverwriteKeyValue, + "EmptyKey": TestEmptyKey, + "KeyEmptyValue": TestKeyEmptyValue, + "SimpleKeyValueClosed": TestSimpleKeyValueClosed, + "NewBatchClosed": TestNewBatchClosed, + "BatchPut": TestBatchPut, + "BatchDelete": TestBatchDelete, + "BatchReset": TestBatchReset, + "BatchReuse": TestBatchReuse, + "BatchRewrite": TestBatchRewrite, + "BatchReplay": TestBatchReplay, + "BatchReplayPropagateError": TestBatchReplayPropagateError, + "BatchInner": TestBatchInner, + "BatchLargeSize": TestBatchLargeSize, + "IteratorSnapshot": TestIteratorSnapshot, + "Iterator": TestIterator, + "IteratorStart": TestIteratorStart, + "IteratorPrefix": TestIteratorPrefix, + "IteratorStartPrefix": TestIteratorStartPrefix, + "IteratorMemorySafety": TestIteratorMemorySafety, + "IteratorClosed": TestIteratorClosed, + "IteratorError": TestIteratorError, + "IteratorErrorAfterRelease": TestIteratorErrorAfterRelease, + "CompactNoPanic": TestCompactNoPanic, + "MemorySafetyDatabase": TestMemorySafetyDatabase, + "MemorySafetyBatch": TestMemorySafetyBatch, + "AtomicClear": TestAtomicClear, + "Clear": TestClear, + "AtomicClearPrefix": TestAtomicClearPrefix, + "ClearPrefix": TestClearPrefix, + "ModifyValueAfterPut": TestModifyValueAfterPut, + "ModifyValueAfterBatchPut": TestModifyValueAfterBatchPut, + "ModifyValueAfterBatchPutReplay": TestModifyValueAfterBatchPutReplay, + "ConcurrentBatches": TestConcurrentBatches, + "ManySmallConcurrentKVPairBatches": TestManySmallConcurrentKVPairBatches, + "PutGetEmpty": TestPutGetEmpty, } // TestSimpleKeyValue tests to make sure that simple Put + Get + Delete + Has diff --git a/database/versiondb/db_test.go b/database/versiondb/db_test.go index fb94eae01fd6..0ff801dfe0dd 100644 --- a/database/versiondb/db_test.go +++ b/database/versiondb/db_test.go @@ -14,9 +14,11 @@ import ( ) func TestInterface(t *testing.T) { - for _, test := range database.Tests { - baseDB := memdb.New() - test(t, New(baseDB)) + for name, test := range database.Tests { + t.Run(name, func(t *testing.T) { + baseDB := memdb.New() + test(t, New(baseDB)) + }) } } diff --git a/x/merkledb/db_test.go b/x/merkledb/db_test.go index 6329f0e4be97..48703556b72a 100644 --- a/x/merkledb/db_test.go +++ b/x/merkledb/db_test.go @@ -99,10 +99,12 @@ func Test_MerkleDB_GetValues_Safety(t *testing.T) { func Test_MerkleDB_DB_Interface(t *testing.T) { for _, bf := range validBranchFactors { - for _, test := range database.Tests { - db, err := getBasicDBWithBranchFactor(bf) - require.NoError(t, err) - test(t, db) + for name, test := range database.Tests { + t.Run(fmt.Sprintf("%s_%d", name, bf), func(t *testing.T) { + db, err := getBasicDBWithBranchFactor(bf) + require.NoError(t, err) + test(t, db) + }) } } }