Skip to content

framework for migration tests#3381

Open
cody-littley wants to merge 2 commits intomainfrom
cjl/migration-test-framework
Open

framework for migration tests#3381
cody-littley wants to merge 2 commits intomainfrom
cjl/migration-test-framework

Conversation

@cody-littley
Copy link
Copy Markdown
Contributor

@cody-littley cody-littley commented May 4, 2026

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMay 5, 2026, 2:29 PM

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMay 4, 2026, 4:53 PM

@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 2.72109% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.03%. Comparing base (ab2b373) to head (34d0dbe).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
sei-db/common/testutil/test_random.go 0.00% 90 Missing ⚠️
sei-db/state_db/sc/migration/router_builder.go 0.00% 51 Missing ⚠️
sei-db/state_db/sc/memiavl/store.go 66.66% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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              
Flag Coverage Δ
sei-chain-pr 73.06% <2.72%> (?)
sei-db 70.41% <ø> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
sei-db/state_db/sc/memiavl/store.go 90.69% <66.66%> (-1.90%) ⬇️
sei-db/state_db/sc/migration/router_builder.go 0.00% <0.00%> (ø)
sei-db/common/testutil/test_random.go 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

consider move this file to sei-db/common/utils/testutil/ with _test.go to avoid building it into binary

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

existing in sei-db/db_engine/litt/util/test_random.go

Copy link
Copy Markdown
Contributor Author

@cody-littley cody-littley May 5, 2026

Choose a reason for hiding this comment

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

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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

test_helper.go is not _test.go, how about rename to countKeysForTest and move into a _test.go file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this dup from utils.NewTestRandom

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

}

// --- Constructor tests ---

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants