Fix part of #5308 : Migrate AppVersionActivityTest to ActivityScenarioRule from ActivityTestRule #5705
Conversation
aadityaverma2011
changed the title
APK & AAB differences analysisNote that this is a summarized snapshot. See the CI artifacts for detailed differences. DevExpand to see flavor specificsUniversal APKAPK file size: 19 MiB (old), 19 MiB (new), 1752 bytes (Removed) APK download size (estimated): 17 MiB (old), 17 MiB (new), 805 bytes (Removed) Method count: 260329 (old), 260322 (new), 7 (Removed) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 6832 (old), 6830 (new), 2 (Removed)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 18 MiB (old), 18 MiB (new), 1752 bytes (Removed)
Configuration hdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change) AlphaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 1536 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 3910 bytes (Removed) Method count: 115836 (old), 115840 (new), 4 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5798 (new), 2 (Removed)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 11 MiB (old), 11 MiB (new), 1548 bytes (Removed)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) BetaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 1528 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 1577 bytes (Removed) Method count: 115842 (old), 115846 (new), 4 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5798 (new), 2 (Removed)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 1536 bytes (Removed)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) GaExpand to see flavor specificsUniversal APKAPK file size: 11 MiB (old), 11 MiB (new), 1500 bytes (Removed) APK download size (estimated): 10 MiB (old), 10 MiB (new), 240 bytes (Removed) Method count: 115842 (old), 115846 (new), 4 (Added) Features: 2 (old), 2 (new), 0 (No change) Permissions: 6 (old), 6 (new), 0 (No change) Resources: 5800 (old), 5798 (new), 2 (Removed)
Lesson assets: 111 (old), 111 (new), 0 (No change) AAB differencesExpand to see AAB specificsSupported configurations:
Base APKAPK file size: 10 MiB (old), 10 MiB (new), 1508 bytes (Removed)
Configuration hdpiAPK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change) Configuration ldpiAPK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change) Configuration mdpiAPK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change) Configuration tvdpiAPK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change) Configuration xhdpiAPK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change) Configuration xxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) Configuration xxxhdpiAPK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change) |
There was a problem hiding this comment.
Thanks @aadityaverma2011!
I have left several comments, but I would like to highlight that we typically avoid introducing new patterns in the codebase unless there is a strong foundation and explanation for it. Otherwise, we prefer using already established patterns, and you can check other existing files for reference on how accomplish certain tasks, especially repetitive ones.
| @@ -17,6 +17,7 @@ import javax.inject.Singleton | |||
| * This class should never be used directly. Instead of accessing the locale directly, use | |||
| * [AppLanguageResourceHandler]. | |||
| */ | |||
|
|
|||
There was a problem hiding this comment.
There should be no spacing after a KDoc.
| fun resetLocale() { | ||
| if (!isInitialized()) { | ||
| // Initialize with system default locale if not already set | ||
| initializeLocale( | ||
| localeController.reconstituteDisplayLocale( | ||
| localeController.getLikelyDefaultAppStringLocaleContext() | ||
| ) | ||
| ) | ||
| } else { | ||
| // If already initialized, just update the locale | ||
| displayLocale = localeController.reconstituteDisplayLocale( | ||
| localeController.getLikelyDefaultAppStringLocaleContext() | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
These changes are unrelated to this PR. if fixing a different issue, please also include that issue number in the PR title and descriprion. If fixing an unfile issue, Please create that issue so that we can review the details.
There was a problem hiding this comment.
Could you please explain why this new module is required? I do not think it is. You could just inject LocaleController, or any controller in a class where you want to use it.
| @@ -115,23 +118,23 @@ import javax.inject.Singleton | |||
| qualifiers = "port-xxhdpi" | |||
| ) | |||
| class AppVersionActivityTest { | |||
| @get:Rule | |||
| @get:Rule(order = 0) | |||
There was a problem hiding this comment.
There is no need to specify rule order.
| @get:Rule(order = 2) | ||
| val activityScenarioRule = ActivityScenarioRule(AppVersionActivity::class.java) |
There was a problem hiding this comment.
We could get rid of this rule completely, and instead use ActivityScenario.launch to initialize acticities in tests. Please look at this class(OnboardingFragmentTest) for examples of how to implement this.
| // Reset locale after each test | ||
| if (::appLanguageLocaleHandler.isInitialized) { | ||
| appLanguageLocaleHandler.resetLocale() |
There was a problem hiding this comment.
Why is this needed?
| testCoroutineDispatchers.runCurrent() | ||
|
|
||
| // Scroll to the item in the RecyclerView |
There was a problem hiding this comment.
Comments are not required since ViewActions and Matchers are pretty self explanatory. We only add comments whne it is not clear some code has been added.
| fun getCurrentActivity(): Activity? { | ||
| val activity = arrayOfNulls<Activity>(1) | ||
| InstrumentationRegistry.getInstrumentation().runOnMainSync { | ||
| activity[0] = ActivityLifecycleMonitorRegistry.getInstance() | ||
| .getActivitiesInStage(Stage.RESUMED) | ||
| .firstOrNull() | ||
| } | ||
| return activity[0] | ||
| } |
There was a problem hiding this comment.
We currently don't use this implementation in any of our UI tests, and there is probaby an existing pattern that you can follow to achieve the same objective as here. Please search for any tests that use ActivityScenario.launch to see the implementation patterns used.
| // private fun launchAppVersionActivityIntent(): ActivityScenario<AppVersionActivity> { | ||
| // val intent = AppVersionActivity.createAppVersionActivityIntent( | ||
| // ApplicationProvider.getApplicationContext() | ||
| // ) | ||
| // return ActivityScenario.launch(intent) | ||
| // } |
There was a problem hiding this comment.
When submitting a PR, be sure to delete any commented out code.
Explanation
Fix Part of #5308
This PR migrates the AppVersionActivityTest.kt class from using the deprecated ActivityTestRule to the recommended ActivityScenarioRule, in line with the Android Testing Library updates. The migration ensures that the test class adheres to the latest practices for testing activities in Android.
What does this PR do?: It updates AppVersionActivityTest.kt to replace ActivityTestRule with ActivityScenarioRule, improving test stability and ensuring compatibility with the current Android testing framework.
Why is this needed?: ActivityTestRule has been deprecated in favor of ActivityScenarioRule which provides better lifecycle management and more consistent behavior when working with activities in tests.
How was this tested?: After migrating the test, all tests for AppVersionActivityTest.kt were run successfully to ensure they still work as expected.
This migration is part of a broader effort to update tests across the project. Once this PR is merged, further migrations will continue for other test classes.
Essential Checklist