-
Notifications
You must be signed in to change notification settings - Fork 0
Add replication tests #46
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.
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
Conversation
… connection features - el-qvs: Expanded statement_features_test.exs with edge case tests - Added tests for all data types (INTEGER, TEXT, BLOB, REAL, NUMERIC) - Added tests for type casting, UNION queries, and CASE expressions - 35 tests passing - el-g5l: Created replication_integration_test.exs with 24 test scenarios - Tests for frame number tracking (get_frame_number_for_replica) - Tests for frame-specific sync (sync_until_frame) - Tests for flush pending writes (flush_and_get_frame) - Tests for max write tracking (max_write_replication_index) - Tests skip when TURSO_DB_URI/TOKEN not configured - el-i0v: Expanded connection_features_test.exs with functional tests - Added 11 new reset connection tests covering reuse, state clearing, etc. - Added 7 new interrupt tests covering multiple calls, isolation, transactions - 17 tests passing
WalkthroughConfiguration enabling and comprehensive test suite expansion: the sync-branch setting is activated, development workflow documentation is restructured with new branching strategy, completed features are marked as closed in issue tracking, and new test coverage for connection reset/interrupt, replication frame synchronization, and SQL statement introspection is introduced. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CLAUDE.md (1)
220-220: Consider using a heading instead of bold emphasis.The static analysis tool flagged this as emphasis used instead of a heading. Consider changing to a proper heading for better document structure and navigation:
-**⚠️ CRITICAL: Preserving Untracked Files** +#### ⚠️ CRITICAL: Preserving Untracked FilesThis improves markdown semantics and allows the section to appear in document outlines.
test/replication_integration_test.exs (1)
447-491: Solid edge case coverage for replication functions.Tests cover important boundaries including large frame numbers (overflow protection), frame 0 synchronisation, and local-only database compatibility.
Based on learnings, consider adding error case tests for invalid connection IDs in a follow-up:
test "get_frame_number_for_replica returns error for invalid connection" do assert {:error, _reason} = EctoLibSql.Native.get_frame_number_for_replica("invalid_conn_id") end
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.beads/config.yaml.beads/issues.jsonlCLAUDE.mdtest/connection_features_test.exstest/replication_integration_test.exstest/statement_features_test.exs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ex,exs,rs,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use British/Australian English for all code, comments, and documentation (except SQL keywords and compatibility requirements)
Files:
test/replication_integration_test.exsCLAUDE.mdtest/statement_features_test.exstest/connection_features_test.exs
**/*.{ex,exs,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
ALWAYS format before committing:
mix format --check-formattedandcargo fmt
Files:
test/replication_integration_test.exstest/statement_features_test.exstest/connection_features_test.exs
test/**/*.exs
📄 CodeRabbit inference engine (CLAUDE.md)
Add tests for new features with coverage for: happy path, error cases, edge cases, transaction rollback, type conversions, and concurrent operations
Use logging, inspection of connection state (state.mode, state.conn_id), and metadata functions (get_last_insert_rowid/1, get_changes/1, get_total_changes/1) for debugging database operations
Files:
test/replication_integration_test.exstest/statement_features_test.exstest/connection_features_test.exs
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to test/**/*.exs : Add tests for new features with coverage for: happy path, error cases, edge cases, transaction rollback, type conversions, and concurrent operations
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to test/**/*.exs : Add tests for new features with coverage for: happy path, error cases, edge cases, transaction rollback, type conversions, and concurrent operations
Applied to files:
test/replication_integration_test.exstest/statement_features_test.exstest/connection_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : For remote replica mode, use EctoLibSql.Native.sync_until_frame/2 or flush_and_get_frame/1 to ensure read-your-writes consistency for critical operations
Applied to files:
test/replication_integration_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to test/**/*.exs : Use logging, inspection of connection state (state.mode, state.conn_id), and metadata functions (get_last_insert_rowid/1, get_changes/1, get_total_changes/1) for debugging database operations
Applied to files:
test/replication_integration_test.exstest/statement_features_test.exstest/connection_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to config/**/*.exs : Use replica mode (with database file, remote URI, auth_token, sync: true) for read-heavy workloads to achieve microsecond read latency with automatic cloud synchronization
Applied to files:
test/replication_integration_test.exs
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to native/ecto_libsql/src/**/tests/*.rs : Tests MAY use `.unwrap()` for simplicity
Applied to files:
test/replication_integration_test.exstest/statement_features_test.exstest/connection_features_test.exs
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to lib/ecto/**/*.ex : Use proper transactions with timeout in Elixir: `Repo.transaction(fn -> ... end, timeout: 15_000)` and `begin(state, behavior: :immediate)` for writes
Applied to files:
test/replication_integration_test.exstest/connection_features_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Never use freeze_replica/1 function as it is not implemented; instead use backup/restore or application-level failover patterns for disaster recovery
Applied to files:
test/replication_integration_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to config/runtime.exs : For production Phoenix applications, use remote replica mode configuration with environment variables for database file path, Turso URI, and authentication tokens in config/runtime.exs
Applied to files:
test/replication_integration_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Use savepoints (EctoLibSql.Native.create_savepoint/2, release_savepoint_by_name/2, rollback_to_savepoint_by_name/2) for partial rollback and error recovery within transactions
Applied to files:
test/replication_integration_test.exstest/connection_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Use transactions (EctoLibSql.handle_begin/2, handle_commit/2, handle_rollback/2) with :immediate or :exclusive behavior for write-heavy workloads to prevent database lock conflicts
Applied to files:
test/replication_integration_test.exstest/connection_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to {CHANGELOG,README,AGENTS}.md : Update `CHANGELOG.md` and relevant documentation (README.md, AGENTS.md) when adding new features or making significant changes
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to **/*.{ex,exs,rs} : ALWAYS format before committing: `mix format --check-formatted` and `cargo fmt`
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Modern Rustler auto-detects all `#[rustler::nif]` functions - no manual registration needed
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/{repo,repositories,queries}/*.ex : Use statement introspection functions (stmt_parameter_count/2, stmt_column_count/2, stmt_column_name/3) to validate prepared statement structure before execution in production code
Applied to files:
test/statement_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to priv/repo/migrations/*.exs : Use ALTER COLUMN modifications in migrations for type changes, NOT NULL constraints, DEFAULT values, and REFERENCES (foreign keys) when targeting libSQL deployments
Applied to files:
test/statement_features_test.exs.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to lib/ecto/**/*.ex : Verify schema types match database columns and check custom types have loaders/dumpers to fix type conversion errors
Applied to files:
test/statement_features_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/{schemas,models}/*.ex : Use native type mappings for Ecto types: :id/:integer → INTEGER, :string → TEXT, :binary_id/:uuid → TEXT, :boolean → INTEGER (0/1), :float → REAL, :text → TEXT, :date/:time/:naive_datetime/:utc_datetime → ISO8601 TEXT, :map/:json → JSON TEXT; avoid {:array, _} and use JSON or separate tables instead
Applied to files:
test/statement_features_test.exs
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to lib/**/*.ex : Ensure connections are closed in try/after blocks to prevent memory leaks and lock issues
Applied to files:
test/connection_features_test.exs
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Configure busy_timeout/2 to handle database locking scenarios gracefully, with longer timeouts for write-heavy concurrent workloads
Applied to files:
test/connection_features_test.exs
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : Choose the right Rust module based on functionality: connection lifecycle → `connection.rs`, query execution → `query.rs`, transactions → `transaction.rs`, batch operations → `batch.rs`, statements → `statement.rs`, cursors → `cursor.rs`, replication → `replication.rs`, metadata → `metadata.rs`, savepoints → `savepoint.rs`, utilities → `utils.rs`
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Use batch operations (EctoLibSql.Native.batch/2 or EctoLibSql.Native.batch_transactional/2) for bulk inserts instead of individual insert statements to reduce roundtrips
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:25.678Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T11:31:25.678Z
Learning: Applies to native/ecto_libsql/src/**/*.rs : When a function cannot be implemented due to architectural constraints, return `:unsupported` atom error in Rust and document clearly in Elixir wrapper with alternatives
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Use vector search (EctoLibSql.Native.vector_type/2, vector/1, vector_distance_cos/2) for AI/ML applications requiring semantic similarity or RAG (Retrieval-Augmented Generation) functionality
Applied to files:
.beads/issues.jsonl
📚 Learning: 2025-12-30T11:31:53.999Z
Learnt from: CR
Repo: ocean/ecto_libsql PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T11:31:53.999Z
Learning: Applies to **/*.ex : Use prepared statements with automatic statement caching for repeated queries to achieve ~10-15x performance improvement over unprepared queries
Applied to files:
.beads/issues.jsonl
🧬 Code graph analysis (2)
test/replication_integration_test.exs (2)
lib/ecto_libsql.ex (3)
handle_execute(148-160)disconnect(131-133)handle_begin(169-174)lib/ecto_libsql/native.ex (2)
get_frame_number_for_replica(1462-1464)flush_and_get_frame(1539-1541)
test/statement_features_test.exs (2)
lib/ecto_libsql.ex (1)
handle_execute(148-160)lib/ecto_libsql/native.ex (2)
prepare(545-553)close_stmt(629-631)
🪛 LanguageTool
CLAUDE.md
[style] ~231-~231: Consider shortening or rephrasing this to strengthen your wording.
Context: ...ugfix branch from main (see above) 2. Make changes to Elixir or Rust code 3. ALWAYS format: `...
(MAKE_CHANGES)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
220-220: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / ubuntu-latest
- GitHub Check: Elixir 1.18.0 / OTP 27.0 / macos-latest
- GitHub Check: Analyze (rust)
🔇 Additional comments (15)
.beads/config.yaml (1)
45-45: LGTM - sync-branch configuration enabled.Enabling the sync-branch setting ensures consistent branch usage across team clones. This is appropriate for coordinating PR workflows.
test/statement_features_test.exs (2)
690-743: Comprehensive data type coverage for column metadata.Good coverage of all major SQLite data types (INTEGER, TEXT, BLOB, REAL, NUMERIC) with full metadata validation including name, origin, and type. The test properly validates the three-tuple structure returned by
get_stmt_columns/2.
745-819: Good edge case coverage for complex SQL constructs.The tests for CAST expressions, UNION queries, and CASE expressions provide valuable coverage for statement introspection across various SQL patterns. Each test properly manages statement lifecycle with cleanup.
Based on learnings, consider adding an error case test for column introspection on malformed queries, though this may be out of scope for this PR.
CLAUDE.md (1)
206-251: Well-structured development workflow documentation.The new Branch Strategy, Development Steps, and PR Workflow sections provide clear guidance for contributors. The CRITICAL notes about preserving untracked files are particularly valuable for preventing accidental data loss.
.beads/issues.jsonl (1)
14-14: Issue statuses correctly updated to reflect completed work.The closed issues align with the test implementations in this PR:
- el-djv:
max_write_replication_indexNIF wrapper and tests- el-g5l: Replication integration tests in
test/replication_integration_test.exs- el-i0v: Connection reset/interrupt tests in
test/connection_features_test.exs- el-qvs: Statement introspection edge cases in
test/statement_features_test.exsAlso applies to: 18-18, 20-20, 26-26
test/replication_integration_test.exs (5)
19-23: Good conditional test configuration.The
@moduletag skip:pattern effectively skips tests when Turso credentials are unavailable, allowing the test suite to run without remote dependencies while still enabling full integration testing when credentials are provided.
67-114: Comprehensive frame number tracking tests.Good coverage including:
- Happy path validation (integer >= 0)
- Consistency verification (multiple calls return same value without writes)
- API flexibility testing (both
statestruct andconn_idstring)The tests align with the coding guidelines for happy path and edge case coverage.
133-145: Appropriately flexible handling of future frame synchronisation.The test correctly acknowledges that syncing to a future frame may behave differently depending on remote state. Accepting either success or error is pragmatic for integration tests that may run against different backend configurations.
284-314: Excellent test for read/write invariant.This test validates a critical property: read-only operations should not affect the max write replication frame. This ensures accurate replication lag monitoring and prevents false positives in sync tracking.
368-384: Good batch operations integration test.The test demonstrates proper frame tracking with
batch_transactional/2, validating that batch operations correctly advance the replication frame. This aligns with the learnings recommending batch operations for bulk inserts.test/connection_features_test.exs (5)
108-144: Good test for reset preserving database state.The test correctly validates that
reset/1clears connection state without affecting persisted data. This is important for connection pool recycling scenarios.
146-186: Important test for statement lifecycle across resets.Validates that prepared statement handles survive connection reset, which is critical for statement caching performance. The test properly manages statement lifecycle with
close_stmt/1.
318-340: Essential isolation test for interrupt behaviour.This test validates a critical property: interrupting one connection must not affect other connections. This is essential for multi-connection scenarios and connection pool safety.
379-400: Good transaction recovery test after interrupt.The test validates that transactions can be rolled back after an interrupt, ensuring proper cleanup in edge cases. This aligns with the learnings requirement for transaction rollback coverage.
84-279: Comprehensive reset test suite.Excellent coverage including:
- Basic reset functionality
- Data persistence verification
- Prepared statement survival
- Multiple successive resets
- Pool-like scenarios
- Persistent data integrity
Based on learnings, concurrent reset operations could be tested if needed, though such tests may be flaky in CI environments.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.