framework for migration tests#3381
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3381 +/- ##
==========================================
- Coverage 59.08% 59.03% -0.05%
==========================================
Files 2099 2101 +2
Lines 172988 173134 +146
==========================================
+ Hits 102212 102216 +4
- Misses 61911 62053 +142
Partials 8865 8865
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
consider move this file to sei-db/common/utils/testutil/ with _test.go to avoid building it into binary
There was a problem hiding this comment.
existing in sei-db/db_engine/litt/util/test_random.go
There was a problem hiding this comment.
moved to sei-db/common/testutil. Can't rename to end with _test.go, since that prevents me from exporting utilities to other packages. There is a pre-existing testutil package (outside the sei-db directory), and none of the files there currently end in _test.go for this reason.
| } | ||
|
|
||
| // Get the underlying memiavl DB. | ||
| func (cs *CommitStore) GetDB() *DB { |
There was a problem hiding this comment.
this function only called in the test file:
for _, namedTree := range memIAVL.GetDB().Trees()
do we want to expose the entire internal db? how about we keep db private and only export needed stuff
There was a problem hiding this comment.
This function actually is called in production pathways in the migration code. That code is in another branch. (I'm merging the diff in that branch in small PRs to reduce reviewer overhead.)
// Manages migration and routing for keys in the evm/ module.
migrationManager, err := NewMigrationManager(
migrationBatchSize,
Version0_MemiavlOnly,
Version1_MigrateEVM,
buildMemIAVLReader(memIAVL),
buildMemIAVLWriter(memIAVL),
buildFlatKVReader(flatKV),
buildFlatKVWriter(flatKV),
NewMemiavlMigrationIterator(memIAVL.GetDB(), []string{EVMStoreKey}), // this line calls GetDB()
NewMigrationMetrics(ctx, Version1_MigrateEVM, 10*time.Second),
)
|
|
||
| // CountKeys returns the total number of non-meta keys across all data DBs in s. | ||
| // It uses RawGlobalIterator, so pending (uncommitted) writes are not counted. | ||
| func CountKeys(s *CommitStore) (int64, error) { |
There was a problem hiding this comment.
test_helper.go is not _test.go, how about rename to countKeysForTest and move into a _test.go file
There was a problem hiding this comment.
renamed to testutil_test.go, exported function copied into test files in other package
| // MIGRATION_TEST_SEED env var when set, or the current Unix nanos otherwise. | ||
| // The seed is logged via t.Logf as a shell-pasteable assignment so that copying | ||
| // the line and prefixing "go test ..." reproduces a failure exactly. | ||
| func newSeededTestRandom(t *testing.T) *utils.TestRandom { |
There was a problem hiding this comment.
is this dup from utils.NewTestRandom
| } | ||
|
|
||
| // --- Constructor tests --- | ||
|
|
There was a problem hiding this comment.
TestApplyChangeSets_ContextCancellationReturnsError uses
blockingWriter, which is ctx-aware on purpose while migration_manager.go:303's ApplyChangeSets accepts a ctx but never inspects it itself
the test passes → readers conclude cancellation works → in
production, calling cancel() after ApplyChangeSets does not cause the
call to return early
There was a problem hiding this comment.
Good point. Previously the routers used threads, but that was a premature optimization that opened race conditions, so I removed the threads. Back when we had threads, context cancellation could abort early for a blocking reader. But now that I've removed that, they no longer can.
I've removed the test TestApplyChangeSets_ContextCancellationReturnsError.
Describe your changes and provide context
This PR contains test framework for migration tests. These tests are fully written and passing in another branch, and will merge as follow up PRs.
This branch also has some light changes to production code which were needed for the test framework to compile.
Testing performed to validate your change
unit tests in other branch