WIP: Migrate benchmarks to UiAutomator 3.0 API #322
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @keyboardsurfer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request migrates the macrobenchmark tests to use the UiAutomator 3.0 API. This involves updating the UiAutomator dependency to a 2.4.0-SNAPSHOT version and refactoring the tests to use the uiAutomator DSL for interacting with the UI. The primary goal is to modernize the test infrastructure and leverage the latest features and improvements in UiAutomator.
Highlights
- UiAutomator Upgrade: The UiAutomator dependency is updated to version 2.4.0-SNAPSHOT in
gradle/libs.versions.toml. This brings in the latest API changes and improvements. - DSL Adoption: The tests are refactored to use the
uiAutomatorDSL, which provides a more concise and expressive way to interact with UI elements. This includes replacing directUiDevicecalls withonViewand other DSL constructs. - Clear Data Refactoring: The
clearDatafunction inCommon.ktis updated to useUiAutomatorTestScopeand clears the data for the launcher package instead of the target package. - Test Modernization: Multiple tests across different benchmark types (Baseline Profile Generation, Login, Click Latency, Frame Timing, Startup) are updated to use the new UiAutomator 3.0 API, ensuring consistency and maintainability.
Changelog
Click here to see the changelog
- MacrobenchmarkSample/gradle/libs.versions.toml
- Updated
uiAutomatorversion from2.3.0to2.4.0-SNAPSHOT.
- Updated
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/Common.kt
- Modified
clearDatafunction to useUiAutomatorTestScopeand clear launcher package data. - Removed
MacrobenchmarkScopeandUiDeviceimports.
- Modified
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/ComposeActivityBaselineProfileGenerator.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
By,findOrFail, andwaitAndFindimports. - Removed
@Ignoreannotation.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/LoginBaselineProfileGenerator.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
Byimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/RecyclerViewActivityBaselineProfileGenerator.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
Byimport. - Removed
@Ignoreannotation.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/StartupProfileGenerator.kt
- Replaced direct
startActivityAndWait()call withuiAutomator { startApp() }. - Added
uiAutomatorimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/LoginBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Added
UiAutomatorTestScopeimport. - Removed
Byimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/clickslatency/ClickLatencyBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Added
UiAutomatorTestScopeimport. - Removed
ByandUntilimports. - Removed
firstStartvariable and setup blocks.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/frames/FrameTimingBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
Byimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/frames/NestedRecyclerFrameTimingBenchmarks.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
ByandUntilimports. - Removed
waitAndFindimport. - Removed
java.util.concurrent.TimeUnitimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/frames/NonExportedActivityBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
ByandUntilimports. - Removed
junit.framework.TestCase.failimport. - Removed
java.util.concurrent.TimeUnitimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/frames/TextInputFrameTimingBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
Byimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/scroll/ScrollBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
ByandUntilimports. - Removed
junit.framework.TestCaseimport. - Removed
firstStartvariable and setup blocks.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/startup/FullyDrawnStartupBenchmark.kt
- Replaced direct
devicecalls withuiAutomatorDSL. - Removed
ByandUntilimports.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/startup/SampleStartupBenchmark.kt
- Replaced direct
startActivityAndWait()call withuiAutomator { startApp() }. - Added
uiAutomatorimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/startup/SmallListStartupBenchmark.kt
- Replaced direct
startActivityAndWait()call withuiAutomator { startIntent(intent) }inmeasureStartupandmeasureComposeStartup. - Added
uiAutomatorimport.
- Replaced direct
- MacrobenchmarkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/benchmark/startup/StartupBenchmark.kt
- Replaced direct
startActivityAndWait()call withuiAutomator { startApp() }. - Added
uiAutomatorimport. - Removed
@SdkSuppressannotations.
- Replaced direct
- MacrobenchmarkSample/settings.gradle.kts
- Added maven repository URL for androidx.dev snapshots.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
In realms of code, where tests reside,
UiAutomator's power, a helpful guide.
With DSL's grace,
We set the pace,
For benchmarks swift, with naught to hide.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request migrates the benchmarks to use the UiAutomator 3.0 API. The changes involve updating the UiAutomator dependency and refactoring the benchmark code to use the new API. Overall, the changes look good and improve the maintainability of the benchmark tests.
Summary of Findings
- UiAutomator version update: The UiAutomator dependency is updated to version 2.4.0-SNAPSHOT, which includes the new UiAutomator 3.0 API.
- Refactoring to use UiAutomator 3.0 API: The benchmark code is refactored to use the new UiAutomator 3.0 API, which provides a more fluent and readable way to interact with UI elements.
- Removal of deprecated methods: Deprecated methods like
device.findObjectandstartActivityAndWaitare replaced with the new UiAutomator 3.0 API.
Merge Readiness
The pull request is almost ready to be merged. However, there are a few medium severity issues that should be addressed before merging. Once these issues are resolved, the pull request will be ready to be merged. I am unable to directly approve the pull request, and that users should have others review and approve this code before merging.
...rkSample/macrobenchmark/src/main/kotlin/com/example/macrobenchmark/baselineprofile/Common.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/example/macrobenchmark/benchmark/clickslatency/ClickLatencyBenchmark.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/example/macrobenchmark/benchmark/clickslatency/ClickLatencyBenchmark.kt
Outdated
Show resolved
Hide resolved
keyboardsurfer
force-pushed
the
uiautomator3
branch
from
7837b2f to
abe3a93
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request migrates the benchmarks to use the UiAutomator 3.0 API. This involves updating the UiAutomator dependency and refactoring the benchmark code to use the new UiAutomator API. The changes look good overall, but there are a few areas that could be improved.
Summary of Findings
- Redundant
startActivityAndWaitcalls: ThestartActivityAndWaitcalls are now redundant sincestartApp()andstartIntent()are used within theuiAutomatorblock. These calls can be removed to simplify the code. - Inconsistent use of
uiAutomatorblock: TheuiAutomatorblock is not consistently used across all benchmark tests. Some tests still use the old UiDevice methods directly. It would be better to wrap all UiAutomator interactions within theuiAutomatorblock for consistency. - Clarity of
clearDatafunction: TheclearDatafunction's documentation should clearly state that it clears data for the launcher package, not the target package.
Merge Readiness
The pull request is almost ready for merging. However, the redundant startActivityAndWait calls and inconsistent use of the uiAutomator block should be addressed before merging. Additionally, the documentation for the clearData function should be clarified. I am unable to approve this pull request, and users should have others review and approve this code before merging.
keyboardsurfer
force-pushed
the
uiautomator3
branch
from
abe3a93 to
6ab5766
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request migrates the benchmarks to use the UiAutomator 3.0 API, which is a good step towards keeping the codebase up-to-date and leveraging the latest features. The changes generally look good, but there are a few areas where improvements can be made to enhance readability and maintainability.
Summary of Findings
- Redundant
firstStartvariable: ThefirstStartvariable is used in several benchmark tests to ensure that the activity is started only once. This variable can be removed by usingstartupMode = StartupMode.COLDin themeasureRepeatedblock. - Inconsistent use of
uiAutomatorblock: TheuiAutomatorblock is used in some tests but not in others. It would be better to use it consistently across all tests for better readability and maintainability. - Missing
waitForIdle()call: InLoginBaselineProfileGenerator.kt, thewaitForIdle()call is removed. This call is important to ensure that the UI is fully loaded before proceeding with the next action. It should be added back to ensure the reliability of the benchmark.
Merge Readiness
The pull request is almost ready for merging. However, there are a few issues that need to be addressed before merging. Specifically, the redundant firstStart variable should be removed, the use of uiAutomator block should be consistent, and the waitForIdle() call should be added back to LoginBaselineProfileGenerator.kt. I am unable to approve this pull request, and users should have others review and approve this code before merging. I recommend that the pull request not be merged until those are addressed (at a minimum).
| startIntent(Intent("$packageName.LOGIN_ACTIVITY")) | ||
| onView { viewIdResourceName == "userName" }.text = "user" | ||
| onView { viewIdResourceName == "password" }.text = "password" | ||
| onView { viewIdResourceName == "login" }.click() |
There was a problem hiding this comment.
The waitForIdle() call is removed. This call is important to ensure that the UI is fully loaded before proceeding with the next action. It should be added back to ensure the reliability of the benchmark.
onView { viewIdResourceName == "login" }.click()
uiDevice.waitForIdle()| fun UiAutomatorTestScope.clearData() { | ||
| val command = "pm clear $TARGET_PACKAGE" | ||
| uiDevice.executeShellCommand(command) |
There was a problem hiding this comment.
The KDoc comment should describe the purpose of the function, not the parameter. Also, it should describe what package the data is being cleared for.
| device.findObject(By.res("password")).text = "password" | ||
| device.findObject(By.res("login")).click() | ||
| device.waitForIdle() | ||
| private fun UiAutomatorTestScope.login() { |
There was a problem hiding this comment.
Consider adding uiDevice.waitForIdle() after the click action to ensure the UI is fully updated before the benchmark finishes.
onView { viewIdResourceName == "login" }.click()
uiDevice.waitForIdle()| fun simpleViewClick() { | ||
| var firstStart = true | ||
| benchmarkRule.measureRepeated( | ||
| packageName = TARGET_PACKAGE, | ||
| metrics = listOf(TraceSectionMetric("ClickTrace")), | ||
| compilationMode = CompilationMode.Full(), | ||
| startupMode = null, | ||
| iterations = DEFAULT_ITERATIONS, | ||
| setupBlock = { | ||
| if (firstStart) { | ||
| startActivityAndWait() | ||
| firstStart = false | ||
| } | ||
| } | ||
| packageName = TARGET_PACKAGE, | ||
| metrics = listOf(TraceSectionMetric("ClickTrace")), | ||
| compilationMode = CompilationMode.Full(), | ||
| startupMode = null, | ||
| iterations = DEFAULT_ITERATIONS, | ||
| ) { |
There was a problem hiding this comment.
The firstStart variable is redundant here. You can use startupMode = StartupMode.COLD in the measureRepeated block to achieve the same result. This applies to all the other tests in this file as well.
benchmarkRule.measureRepeated(
packageName = TARGET_PACKAGE,
metrics = listOf(TraceSectionMetric("ClickTrace")),
compilationMode = CompilationMode.Full(),
startupMode = StartupMode.COLD,
iterations = DEFAULT_ITERATIONS,
) {| uiAutomator { | ||
| startApp() | ||
| val title = "RecyclerView Sample" | ||
| onView { viewIdResourceName == "launchRecyclerActivity" }.click() | ||
| onView { text == title && isVisibleToUser } | ||
| pressBack() | ||
| onView { text == title && !isVisibleToUser } | ||
| } |
There was a problem hiding this comment.
The startApp() function is used in this test, but startIntent() is used in other tests. Consider using startIntent() consistently across all tests for better readability and maintainability.
...kotlin/com/example/macrobenchmark/baselineprofile/ComposeActivityBaselineProfileGenerator.kt
Outdated
Show resolved
Hide resolved
...n/com/example/macrobenchmark/baselineprofile/RecyclerViewActivityBaselineProfileGenerator.kt
Outdated
Show resolved
Hide resolved
| ) { | ||
| startActivityAndWait() | ||
| uiAutomator { | ||
| startApp() |
There was a problem hiding this comment.
startActivityAndWait is a macrobenchmark function, this new one is a UIAutomator function. Are we sure this is correct? startActivityAndWait isn't doing anything else that now wouldn't be done?
keyboardsurfer
force-pushed
the
uiautomator3
branch
from
6ab5766 to
08a4d2e
Compare
keyboardsurfer
changed the title
The UiAutomator API is still a SNAPSHOT build. Asking to get this added was on my backlog and I just filed a bug for it. |
2 participants
No description provided.