CI coverage gaps from #42: Room migration test, lint with baseline, unsigned release build - #47
Conversation
…build Three of the gaps the Android CI workflow (#36) left open: 1. Robolectric test for the Room 6 -> 7 migration. Runs on the JVM under the existing testDebugUnitTest, no emulator. Schema export only starts at v7, so the test derives the v6 schema from the Room-generated v7 schema minus crash_logs (the only thing MIGRATION_6_7 adds), then lets Room's own schema validation on open be the assertion. Also flips on room.schemaLocation so 7.json onward are exported for future MigrationTestHelper tests. 2. Android Lint in CI, gated by a baseline: existing findings are frozen in app/lint-baseline.xml, new findings fail the build (abortOnError is now true). The baseline itself is generated by the first CI run via a clearly-marked temporary bootstrap step, then committed. 3. Unsigned assembleRelease in CI to exercise R8/ProGuard, which debug builds never touch. Requires the new explicit -PallowUnsignedRelease opt-in; without it the signing guard still fails release builds that have no signing config, exactly as before. Closes nothing on its own - items 2 (androidTest source set) and 4 (manual device smoke test) of #42 remain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwZujP1Xxz6d9SypGb8hwN
…test app - app/lint-baseline.xml: generated by the bootstrap CI run (8 errors, 39 warnings frozen); new lint findings now fail the build. The bootstrap log-print step is removed from the workflow. - app/schemas/.../7.json: Room schema export recovered from the same run. - Migration tests now run under a plain Application instead of OpenDroidApp: the real app's startup reaches the Android Keystore via SecurePrefs, which does not exist on the Robolectric JVM and is not needed to open a database. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwZujP1Xxz6d9SypGb8hwN
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Confidence Score: 5/5Safe to merge — all changes are additive CI infrastructure and test code with no modifications to production runtime paths. All production code changes are a single-line exportSchema flag flip. The new migration test is correctly structured: it derives the v6 schema from the live Room database, applies only the guards needed to detect future drift, and validates via Room's own identity-hash check on open. Previous review concerns (idempotency comment, derived-schema expansion, missing APK artifact) have all been addressed in this revision. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| app/src/test/java/com/opendroid/ai/data/db/OpenDroidDatabaseMigrationTest.kt | New Robolectric migration test for 6→7 with two guards against schema-derivation drift, data-preservation checks, and idempotency verification — previous review concerns addressed |
| .github/workflows/android-ci.yml | Adds lint and release-build jobs; release APK is now uploaded as an artifact (addressing the previous outside-diff concern) |
| app/build.gradle | Adds unsigned-release opt-in flag, Robolectric test deps, Room schema export via kapt arg, lint baseline, and abortOnError true — all correctly scoped |
| app/src/main/java/com/opendroid/ai/data/db/OpenDroidDatabase.kt | Single-line change: exportSchema flipped from false to true, enabling committed schema JSONs for future MigrationTestHelper-based tests |
| app/schemas/com.opendroid.ai.data.db.OpenDroidDatabase/7.json | Generated Room schema export for v7 with all 10 entities; identity hash matches what Room would compute from the current annotations |
| app/lint-baseline.xml | Baseline freezes 35 pre-existing lint findings so only new issues fail the build; findings are real pre-existing issues, not suppressions of new problems |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI Push / PR] --> B[test job\ntestDebugUnitTest]
A --> C[lint job\nlintDebug]
A --> D[release-build job\nassembleRelease]
B --> B1[OpenDroidDatabaseMigrationTest\nRobolectric / JVM]
B1 --> B2[createVersion6Database\nderive DDL from REFERENCE_DB\nminus crash_logs]
B2 --> B3{version guard\nassertEquals 7}
B3 -->|pass| B4{table set guard\nassertEquals VERSION_6_TABLES}
B4 -->|pass| B5[Open v6 DB through\nMIGRATION_6_7]
B5 --> B6[Room schema validation\non open — identity hash check]
B6 --> B7[data + index\nassertions pass]
B1 --> B8[idempotency test\nrun migrate on v7 DB\nIF NOT EXISTS no-ops]
C --> C1[abortOnError true\nbaseline filters pre-existing\nonly new findings fail]
D --> D1[-PallowUnsignedRelease\nskips signing guard]
D1 --> D2[R8 / ProGuard\nminification on release path]
D2 --> D3[upload-artifact\napp-release-unsigned]
Reviews (2): Last reviewed commit: "test: address review - accurate idempote..." | Re-trigger Greptile
…ds, APK artifact - Correct the idempotency test comment: Room wraps migrations in a transaction, so crash recovery replays whole migrations against the original schema - the IF NOT EXISTS guards only matter when the migration meets an already-existing crash_logs table. - Guard the v6-schema derivation against schema drift: assert the reference database is version 7 and the derived table set matches v6 exactly, with messages pointing at MigrationTestHelper + app/schemas/ for migrations 7 -> 8 onward. - Upload the unsigned release APK as a CI artifact so R8 breakage can be inspected with apkanalyzer/aapt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GwZujP1Xxz6d9SypGb8hwN
Closes the three automatable gaps from #42, in the issue's suggested order.
1. Robolectric test for the Room 6 → 7 migration (issue item 1)
OpenDroidDatabaseMigrationTestruns on the JVM under the existingtestDebugUnitTest— no emulator, no workflow change needed.Schema export only begins at version 7, so there is no exported
6.jsonforMigrationTestHelperto build the old database from. Instead the test derives the v6 schema from the current Room-generated v7 schema minuscrash_logsand its index — exactly whatMIGRATION_6_7adds — stamps ituser_version = 6, seeds it with data, and opens it through the real migration chain withoutfallbackToDestructiveMigration. Room's own schema validation on open is the assertion: a migration that produces a schema Room's generated code doesn't expect (the crash-on-launch-after-upgrade failure mode) fails the test. It also verifies pre-upgrade rows survive, the timestamp index exists, and the migrated table round-trips through the realCrashLogDao. A second test holds the migration to itsIF NOT EXISTSre-run-safety claim.Room schema export is now enabled (
room.schemaLocation→app/schemas/, committed), so migrations from 7 → 8 onward can be tested against real historical schemas withMigrationTestHelper.2. Android Lint with a baseline (issue item 3)
lintjob added to the workflow, running:app:lintDebug.app/lint-baseline.xml;abortOnErroris nowtrue, so only new findings fail the build.dl.google.com, so Gradle cannot run locally).3. Unsigned
assembleReleasein CI (issue item 5)release-buildjob runs:app:assembleRelease -PallowUnsignedRelease, exercising R8/ProGuard — where reflection-dependent code (Room entities, serialization models) breaks — with no signing secrets in CI.-PallowUnsignedReleaseis an explicit opt-in added to the signing guard inapp/build.gradle. Without it, a release build with no signing config still fails loudly, exactly as before.Not in scope
Items 2 (
androidTestsource set) and 4 (manual device smoke test) of #42 remain open — the issue itself ranks them last, and neither is automatable here.🤖 Generated with Claude Code
https://claude.ai/code/session_01GwZujP1Xxz6d9SypGb8hwN
Generated by Claude Code