test: Replace binaries with bytecode loaded from test deps#1341
Open
0xadam-brown wants to merge 12 commits into
Open
test: Replace binaries with bytecode loaded from test deps#13410xadam-brown wants to merge 12 commits into
0xadam-brown wants to merge 12 commits into
Conversation
… users (GRADLE-107)
Adds a new ASM bytecode method visitor that lets us auto-wrap all occurrences of SQLiteDriver with SentrySQLiteDriver whenever the driver is passed to Room.DatabaseBuilder.setDriver(...).
For instance:
val database = Room.databaseBuilder(context, MyDatabase::class.java, "dbName")
.setDriver(AndroidSQLiteDriver())
.build()
becomes:
val database = Room.databaseBuilder(context, MyDatabase::class.java, "dbName")
.setDriver(SentrySQLiteDriver.create(AndroidSQLiteDriver()))
.build()
The wrapping policy is naive in that every SQLiteDriver passed to setDriver() is wrapped. That's deliberate because SentrySQLiteDriver protects against double-wrapping internally, which lets us keep our visitor implementation simple.
Preconditions:
1. InstrumentationFeature.DATABASE is enabled
2. The owning app is using a version of sentry-android-sqlite that includes SentrySQLiteDriver
Coverage:
- Auto-wraps SQLiteDriver for all Room users (sole Room access point is via its Room.DatabaseBuilder.setDriver() method).
- SQLDelight users don't need driver auto-wrapping (they still use SupportSQLiteOpenHelper, which we already auto-wrap).
- The few developers who use SQLiteDriver directly will need to wrap it manually.
Replace checked-in RoomDatabase$Builder fixtures with classpath loading from room-runtime-android and room3-runtime-android AARs so tests track Dependabot version bumps automatically. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
0xadam-brown
requested review from
adinauer,
markushi,
romtsn and
runningcode
as code owners
4 tasks
Replace checked-in AndroidX SQLite framework and TypefaceCompatUtil fixtures with classpath loading from sqlite-framework and androidx.core AARs on the test classpath. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
0xadam-brown
force-pushed
the
chore/deps-not-binaries
branch
from
b2f9e38 to
aeecebf
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aeecebf. Configure here.
Align sqlite-framework bytecode and FrameworkSQLiteStatement SemVer so VisitorTest exercises the post-2.3.0 instrumentation path.
runningcode
approved these changes
Jun 24, 2026
runningcode
left a comment
runningcode
left a comment
Contributor
There was a problem hiding this comment.
Very nice! I think this is much easier to maintain than checking in .class files!
Contributor
|
What do you mean by this?
We do want to pin our dependencies as part of a security audit. I'm not sure what the fixture regen pipeline means |
romtsn
approved these changes
Jun 24, 2026
romtsn
reviewed
Jun 24, 2026
| * - AndroidX SQLite framework classes: `androidx.sqlite:sqlite-framework` | ||
| * - `TypefaceCompatUtil`: `androidx.core:core` | ||
| */ | ||
| private val FIXTURE_TO_RESOURCE = |
Member
There was a problem hiding this comment.
wondering if we need to maintain this mapping or rather change VisitorTest parameters to fit the new structure? I guess simply passing a FQN should be enough when loading from an aar
romtsn
reviewed
Jun 24, 2026
| mapOf( | ||
| "androidx.room.RoomDatabase\$Builder" to "RoomDatabase\$Builder", | ||
| "androidx.room3.RoomDatabase\$Builder" to "RoomDatabase3\$Builder", | ||
| "androidx.room.RoomDatabase\$Builder" to "androidx/room/RoomDatabase\$Builder.class", |
Member
There was a problem hiding this comment.
also not sure if this one is needed, we could just append .class when we load, right?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
📜 Description
Follow-up to the convo here about checked-in bytecode fixtures. For the easy cases, tests now load real library classes from test dependencies instead of committed .class files.
Why not the rest?
The remaining fixtures are harder in that we can't just fetch a dep from Maven + delete an existing binary. Instead, we'd have to pin deps or a fixture regen pipeline, each of which would be more involved than a quick-win, sometimes without much payoff.
💡 Motivation and Context
Let's us avoid (potentially fragile and always static) binary check-ins and instead rely on evergreen dependencies.
💚 How did you test it?
Ran test suite after swapping out binaries.
📝 Checklist
🔮 Next steps