-
Notifications
You must be signed in to change notification settings - Fork 2
Programmatic store screenshot generation #210
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
base: main
Are you sure you want to change the base?
Conversation
Job Summary for GradleBuild & Test :: build |
gradle/libs.versions.toml
Outdated
| ktor = "3.1.3" | ||
| activity-compose = "1.11.0-rc01" | ||
| compose-plugin = "1.8.0" | ||
| compose = "1.8.0" |
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.
| compose = "1.8.0" | |
| compose = "1.8.2" |
| androidx-test-core = { module = "androidx.test:core", version = "1.6.1" } | ||
| androidx-ui-test-manifest = { module = "androidx.compose.ui:ui-test-manifest", version.ref = "compose" } | ||
| androidx-ui-test-junit4 = { module = "androidx.compose.ui:ui-test-junit4", version.ref = "compose" } | ||
| androidx-uiautomator = { module = "androidx.test.uiautomator:uiautomator", version = "2.3.0" } |
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.
- check latest version before merging
df21956 to
37b918e
Compare
50d4e78 to
59439ff
Compare
5260253 to
4ddecd6
Compare
- system back navigation - screenshot
|
/gemini review this pr |
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.
Code Review
The pull request introduces a 'demo' flavor to facilitate programmatic screenshot generation for the store, along with UI tests and scripts to automate the process. The changes include adding dependencies, creating a demo flavor, pre-filling demo content, and adding a screenshot test. The code looks well-structured and addresses the stated goals.
| "demoImplementation"(projects.tasksCore) { | ||
| because("needed for prefilled content for screenshot generation") | ||
| } |
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.
Consider moving this dependency to androidTestImplementation if it's only needed for tests. This avoids including it in the main application code, reducing the app size. If tasksCore is truly needed for prefilled content in the demo flavor's main code (not just the tests), then this placement is correct.
| // TODO would be more elegant to use Room preloaded data mechanism. | ||
| // Was tested but didn't work first time, not investigated a lot. | ||
| // That being said, it's simpler to make evolution on pre-filled content programmatically. | ||
| // see https://developer.android.com/training/data-storage/room/prepopulate |
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.
The comment suggests that using Room's preloaded data mechanism would be more elegant. If feasible, explore this option to simplify the initialization process and potentially improve performance. If the programmatic approach is preferred due to easier evolution, consider documenting the reasons for this choice more explicitly.
| if (target in arrayOf("test", "demo") && dbFile.exists()) { | ||
| dbFile.delete() | ||
| } |
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.
This logic deletes the database if the target is 'test' or 'demo'. While this is useful for testing and demo purposes, ensure that this behavior is clearly documented and doesn't inadvertently affect production or development environments. Consider using a more specific flag or environment variable to control this behavior.
Description
Add
adb_demo_mode.shscript to ease screenshot generationIntroduce
demoflavorCreate pre-filled Demo content
Add
StoreScreenshotTestto generate store screenshots programmaticallyOnly relying on
TestTasksApplicationinstead of a dedicated flavor could simplify the whole thingdemoImplementationtoandroidTestImplementation@get:Rule val koinTestRule = KoinTestRule( listOf( // here tweak module with prefilled DB databaseTestModule, ... ) )Checklist