test: migrate SQLite tests to use in-memory databases #637
+463
−234
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updates database and eth test modules to use in-memory SQLite databases instead of temporary files for faster test execution and reduced I/O. - Add test-only connection customizer for in-memory schema initialization - Update TestFixture to use :memory: by default with file-based fallback - Keep persistence-dependent tests using file-based databases - Migrate slashing database tests to in-memory
diegomrsantos
force-pushed
the
sqlite-in-memory-tests
branch
from
0c23f64 to
68c7e1e
Compare
- Use Option<TempDir> in TestFixture to avoid unnecessary temp directory creation for in-memory tests - Change in-memory DB support from cfg(test) to feature="test-utils" to enable usage in integration tests - Fix restart-related tests to use file-based fixtures for persistence validation - Revert slashing database to use temp files (external crate limitation) - Add documentation for schema migration bypass in in-memory tests - Improve test comment clarity and remove redundant assertions
Renamed TestFixture constructor methods to better describe their behavior: - new() → new_in_memory() - in-memory database with populated cluster - new_empty() → new_in_memory_empty() - in-memory database, empty - new_with_file() - kept as is - file-based database with populated cluster - new_empty_with_file() - kept as is - file-based database, empty Updated all call sites in database and eth integration tests.
diegomrsantos
added
testing
claude-recheck
dknopik
reviewed
Oct 16, 2025
anchor/eth/tests/common/mod.rs
Outdated
Comment on lines
117
to
118
| // Keep temp_dir alive by leaking it - tests are short-lived anyway | ||
| std::mem::forget(temp_dir); |
There was a problem hiding this comment.
This causes the test run to leave behind files. This might be irrelevant for CI runs, but I want my developer machine cleaned up.
Comment on lines
202
to
207
| fn open_or_create(path: &Path, domain: DomainType) -> Result<Pool, DatabaseError> { | ||
| schema::ensure_up_to_date(path, domain)?; | ||
| Self::open_conn_pool(path) | ||
| if path.to_string_lossy() != ":memory:" { | ||
| schema::ensure_up_to_date(path, domain)?; | ||
| } | ||
| Self::open_conn_pool(path, domain) | ||
| } |
There was a problem hiding this comment.
I do not really like this or the conditional function below. Using the connection hook to create the schema also feels like a hack. What do you think about a separate function open_in_memory that creates a pool with a connection that does not time out and creates the schema always?
There was a problem hiding this comment.
pushed a new whole design, let me know what you think
diegomrsantos
added
claude-recheck
…posability Simplified test fixture design with the following improvements: - Eliminated operator generation duplication by extracting generate_default_operators() helper This fixes a bug where operators were generated twice with different RSA keys - Removed legacy TestFixture type alias and updated all tests to use explicit InMemoryTestFixture/FileTestFixture types for clarity - Changed TestFixtureData.db to Arc<NetworkDatabase> to support tests requiring shared ownership (particularly eth integration tests) - Created ProcessorFixture combining InMemoryTestFixture with EventProcessor setup, eliminating the pattern of creating fixtures just to extract the database - Added comprehensive documentation explaining fixture state and design decisions All 38 database tests and 6 eth integration tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
diegomrsantos
force-pushed
the
sqlite-in-memory-tests
branch
from
638fb8e to
0d79eeb
Compare
|
@claude review this PR |
|
Claude finished @diegomrsantos's task —— View job Code Review In Progress
|
|
@claude review this PR |
Code Review In Progress
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
N/A - Performance improvement and code quality enhancement for test suite
Proposed Changes
In-Memory Database Migration
new_in_memory()API to replace magic:memory:stringTest Fixture Refactoring
generate_default_operators()helper to fix bug where operators were generated twice with different RSA keysTestFixturetype alias and updated all 38+ tests to use explicitInMemoryTestFixture/FileTestFixturefor clarityTestFixtureData.dbtoArc<NetworkDatabase>to support tests requiring shared ownership (especially eth integration tests)InMemoryTestFixturewithEventProcessorsetup, eliminating awkward patterns in eth testsAdditional Info
This PR improves both test performance and code quality:
All 38 database tests and 6 eth integration tests pass successfully.