test(db): walking-migration regression guard + Room schema export (#142)#147
test(db): walking-migration regression guard + Room schema export (#142)#147
Conversation
The pre-existing MigrationTest used Room.inMemoryDatabaseBuilder which creates the schema directly from the current entity declarations and never exercises Migration.migrate code paths. So when v1.5.1's entity declarations drifted from what the migrations produced, the in-memory test passed but real upgraders crashed at launch (#141, #143). Adds: 1. Room schema export (exportSchema = true + ksp arg pointing at app/schemas/). Generates a versioned JSON dump per @database version. v9 committed in this PR. Future schema drift becomes visible in PR diffs. 2. WalkingMigrationTest: bootstraps a SQLite file at v8 with a known historical shape, opens it via Room.databaseBuilder, and lets Room run MIGRATION_8_9 + validate the result against entity declarations. Two test methods cover the divergent v8 shapes that existed in the wild after v1.5.1: - Path A (upgrade-from-v1.5.0): walletId DEFAULT '' + partial idx_tx_pending already on disk. - Path B (fresh-install-on-v1.5.1): neither. Both must converge to v9 successfully. 3. Regression-guard test that registers a no-op MIGRATION_8_9 (what the first cut of #143 shipped) and asserts path B fails with the exact 'Migration didn't properly handle' exception that surfaced in production. Permanent in-codebase guard against the test losing its detection power. Note on scope: the JVM/Robolectric harness here covers the v8→v9 upgrade path because that is where the historical fork lives. Walking from v1 forward is harder without an exported schema for v1 and is tracked in #144 Phase 2 (instrumented MigrationTestHelper).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes enable Room database schema export and introduce regression testing infrastructure. Build configuration adds KSP arguments to generate schema JSON files. Database configuration enables schema export with documentation. A Room schema JSON file for version 9 is committed. A new test class validates successful migrations from version 8 to 9 across multiple database state variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 9 minutes and 2 seconds.Comment |
CI's clean build hit AbstractMethodError in Room's SchemaBundle.$serializer because Room 2.8.4's bundled kotlinx-serialization-core is binary-incompatible with the project's kotlinx-serialization-json:1.8.0 on a fresh classpath. Local builds didn't repro because of cached classloaders. When a schema JSON exists at the target version, Room's processor deserializes it to diff against the new export. With no JSON present, Room only writes a new one, no deserialize path, no crash. Workaround: don't commit the JSONs. They're still generated on every build (locally and in CI), they just don't get persisted. The walking-migration test doesn't depend on them — it validates against entity declarations directly. Trade-off lost: schema diffs aren't visible in PR review. We get them back when we either upgrade Room to a version compiled against kotlinx-serialization 1.8.x, or downgrade kotlinx-serialization-json to 1.7.x. Tracked alongside #142.
Same AbstractMethodError surfaced even after .gitignore'ing the JSONs: Room's deserialize path runs on every KSP cycle, not only when an existing schema file is present. The dep conflict between Room 2.8.4 and kotlinx-serialization 1.8.0 needs a real fix before exportSchema can be on. Reverts the @database exportSchema flag and the room.schemaLocation KSP arg. The walking-migration test under src/test/ still validates schema-vs-entity drift via Room's onMigrate path, so #142's regression-guard value is unaffected. Re-enabling tracked in #149.
1 participant
Closes part of #142 (DB-only scope; broader upgrade-smoke harness still tracked in #144).
Summary
The pre-existing `MigrationTest` was a vacuous test: it used `Room.inMemoryDatabaseBuilder` which builds the schema directly from current entity declarations and never executes any `Migration.migrate` code. v1.5.1 shipped two regressions (#141, #143) that crashed real upgraders on launch, while every CI test was happily green.
This PR adds the test infrastructure that would have caught both.
What's in the PR
1. Room schema export
`exportSchema = true` on `@Database`, with `ksp { arg("room.schemaLocation", ...) }` in `build.gradle.kts`. Every build generates `app/schemas//.json`. v9 is committed in this PR. Future schema drift becomes visible in PR diffs.
2. WalkingMigrationTest
Bootstraps a SQLite file at v8 with a known historical shape, then opens it via `Room.databaseBuilder` with all migrations registered. Room runs MIGRATION_8_9 and validates the result against the entity declarations. Two test methods cover the divergent v8 shapes that existed in the wild after v1.5.1:
Both must converge to v9 successfully under the real `MIGRATION_8_9`.
3. Regression-guard test
A third test method registers a no-op MIGRATION_8_9 (literally what the first cut of #143 shipped) and asserts path B fails with the exact `Migration didn't properly handle` exception that crashed users in production. Permanent guard against the test losing its detection power.
What's NOT in the PR
Test plan
Related
Summary by CodeRabbit
Tests
Chores