test: Replace binaries with bytecode loaded from test deps#1341
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
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
left a comment
runningcode
left a comment
There was a problem hiding this comment.
Very nice! I think this is much easier to maintain than checking in .class files!
|
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 |
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Ah, sorry for the confusion. (I've updated the PR description.) I just meant that additional test updates wouldn't be a simple removal of the check-in binary + introduce a corresponding Maven coordinate. Instead, we'd have to be smarter and design a way to regenerate binaries from coordinates and/or pin specific versions for testing purposes. |
4 participants
📜 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 be more clever about how we extract or regenerate them, 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