-
Notifications
You must be signed in to change notification settings - Fork 544
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
Fix part of #5308 : Migrate AppVersionActivityTest to ActivityScenarioRule from ActivityTestRule #5705
base: develop
Are you sure you want to change the base?
Fix part of #5308 : Migrate AppVersionActivityTest to ActivityScenarioRule from ActivityTestRule #5705
Changes from all commits
ac241b5
727c88f
1bc99f0
1b09654
76ccc86
f355c2c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import javax.inject.Singleton | |
* This class should never be used directly. Instead of accessing the locale directly, use | ||
* [AppLanguageResourceHandler]. | ||
*/ | ||
|
||
@Singleton | ||
class AppLanguageLocaleHandler @Inject constructor( | ||
private val localeController: LocaleController | ||
|
@@ -76,6 +77,21 @@ class AppLanguageLocaleHandler @Inject constructor( | |
verifyDisplayLocaleIsInitialized() | ||
return displayLocale | ||
} | ||
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() | ||
) | ||
} | ||
} | ||
Comment on lines
+80
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
private fun verifyDisplayLocaleIsInitialized() { | ||
check(isInitialized()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please explain why this new module is required? I do not think it is. You could just inject |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package org.oppia.android.app.translation | ||
|
||
import dagger.Module | ||
import dagger.Provides | ||
import org.oppia.android.domain.locale.LocaleController | ||
import javax.inject.Singleton | ||
|
||
@Module | ||
class AppLanguageLocaleModule { | ||
|
||
@Provides | ||
@Singleton | ||
fun provideAppLanguageLocaleHandler(localeController: LocaleController): AppLanguageLocaleHandler { | ||
return AppLanguageLocaleHandler(localeController) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package org.oppia.android.app.administratorcontrols | ||
|
||
import android.app.Activity | ||
import android.app.Application | ||
import android.content.Context | ||
import android.content.Intent | ||
|
@@ -9,18 +10,18 @@ import androidx.test.core.app.ActivityScenario | |
import androidx.test.core.app.ApplicationProvider | ||
import androidx.test.espresso.Espresso.onView | ||
import androidx.test.espresso.action.ViewActions.click | ||
import androidx.test.espresso.action.ViewActions.pressBack | ||
import androidx.test.espresso.assertion.ViewAssertions.matches | ||
import androidx.test.espresso.contrib.RecyclerViewActions.scrollToPosition | ||
import androidx.test.espresso.intent.Intents | ||
import androidx.test.espresso.intent.Intents.intended | ||
import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent | ||
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed | ||
import androidx.test.espresso.matcher.ViewMatchers.isRoot | ||
import androidx.test.espresso.matcher.ViewMatchers.withId | ||
import androidx.test.espresso.matcher.ViewMatchers.withText | ||
import androidx.test.ext.junit.rules.ActivityScenarioRule | ||
import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
import androidx.test.rule.ActivityTestRule | ||
import androidx.test.platform.app.InstrumentationRegistry | ||
import androidx.test.runner.lifecycle.ActivityLifecycleMonitorRegistry | ||
import androidx.test.runner.lifecycle.Stage | ||
import com.google.common.truth.Truth.assertThat | ||
import dagger.Component | ||
import org.junit.After | ||
|
@@ -45,6 +46,8 @@ import org.oppia.android.app.model.ProfileId | |
import org.oppia.android.app.model.ScreenName | ||
import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule | ||
import org.oppia.android.app.shim.ViewBindingShimModule | ||
import org.oppia.android.app.translation.AppLanguageLocaleHandler | ||
import org.oppia.android.app.translation.AppLanguageLocaleModule | ||
import org.oppia.android.app.translation.testing.ActivityRecreatorTestModule | ||
import org.oppia.android.app.utility.OrientationChangeAction.Companion.orientationLandscape | ||
import org.oppia.android.data.backends.gae.NetworkConfigProdModule | ||
|
@@ -115,23 +118,23 @@ import javax.inject.Singleton | |
qualifiers = "port-xxhdpi" | ||
) | ||
class AppVersionActivityTest { | ||
@get:Rule | ||
@get:Rule(order = 0) | ||
aadityaverma2011 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule() | ||
|
||
@get:Rule | ||
@get:Rule(order = 1) | ||
val oppiaTestRule = OppiaTestRule() | ||
|
||
@get:Rule | ||
val activityTestRule: ActivityTestRule<AppVersionActivity> = ActivityTestRule( | ||
AppVersionActivity::class.java, /* initialTouchMode= */ true, /* launchActivity= */ false | ||
) | ||
@get:Rule(order = 2) | ||
val activityScenarioRule = ActivityScenarioRule(AppVersionActivity::class.java) | ||
Comment on lines
+127
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could get rid of this rule completely, and instead use |
||
|
||
@Inject | ||
lateinit var context: Context | ||
|
||
@Inject | ||
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers | ||
|
||
@Inject | ||
lateinit var appLanguageLocaleHandler: AppLanguageLocaleHandler | ||
@Before | ||
fun setUp() { | ||
Intents.init() | ||
|
@@ -141,12 +144,11 @@ class AppVersionActivityTest { | |
|
||
@Test | ||
fun testAppVersionActivity_hasCorrectActivityLabel() { | ||
activityTestRule.launchActivity(createAppVersionActivityIntent()) | ||
val title = activityTestRule.activity.title | ||
|
||
// Verify that the activity label is correct as a proxy to verify TalkBack will announce the | ||
// correct string when it's read out. | ||
assertThat(title).isEqualTo(context.getString(R.string.app_version_activity_title)) | ||
val scenario = activityScenarioRule.scenario | ||
scenario.onActivity { activity -> | ||
val title = activity.title | ||
assertThat(title).isEqualTo(context.getString(R.string.app_version_activity_title)) | ||
} | ||
} | ||
|
||
private fun createAppVersionActivityIntent(): Intent { | ||
|
@@ -159,6 +161,10 @@ class AppVersionActivityTest { | |
fun tearDown() { | ||
testCoroutineDispatchers.unregisterIdlingResource() | ||
Intents.release() | ||
// Reset locale after each test | ||
if (::appLanguageLocaleHandler.isInitialized) { | ||
appLanguageLocaleHandler.resetLocale() | ||
Comment on lines
+164
to
+166
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed? |
||
} | ||
} | ||
|
||
private fun setUpTestApplicationComponent() { | ||
|
@@ -174,7 +180,8 @@ class AppVersionActivityTest { | |
|
||
@Test | ||
fun testAppVersionActivity_loadFragment_displaysAppVersion() { | ||
launchAppVersionActivityIntent().use { scenario -> | ||
val scenario = activityScenarioRule.scenario | ||
scenario.onActivity { activity -> | ||
val lastUpdateDate = scenario.convertTimeStampToDate(context.getLastUpdateTime()) | ||
onView( | ||
withText( | ||
|
@@ -199,7 +206,8 @@ class AppVersionActivityTest { | |
|
||
@Test | ||
fun testAppVersionActivity_configurationChange_appVersionIsDisplayedCorrectly() { | ||
launchAppVersionActivityIntent().use { scenario -> | ||
val scenario = activityScenarioRule.scenario | ||
scenario.onActivity { activity -> | ||
onView(isRoot()).perform(orientationLandscape()) | ||
val lastUpdateDate = scenario.convertTimeStampToDate(context.getLastUpdateTime()) | ||
onView( | ||
|
@@ -236,23 +244,50 @@ class AppVersionActivityTest { | |
@Test | ||
fun testAppVersionActivity_loadFragment_onBackPressed_displaysAdministratorControlsActivity() { | ||
ActivityScenario.launch<AdministratorControlsActivity>( | ||
launchAdministratorControlsActivityIntent( | ||
internalProfileId = 0 | ||
) | ||
).use { | ||
launchAdministratorControlsActivityIntent(internalProfileId = 0) | ||
).use { scenario -> | ||
|
||
testCoroutineDispatchers.runCurrent() | ||
|
||
// Scroll to the item in the RecyclerView | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
onView(withId(R.id.administrator_controls_list)).perform( | ||
scrollToPosition<RecyclerView.ViewHolder>( | ||
3 | ||
) | ||
scrollToPosition<RecyclerView.ViewHolder>(3) | ||
) | ||
|
||
// Click on the app version option | ||
onView(withText(R.string.administrator_controls_app_version)).perform(click()) | ||
intended(hasComponent(AppVersionActivity::class.java.name)) | ||
onView(isRoot()).perform(pressBack()) | ||
|
||
// Wait for UI update | ||
testCoroutineDispatchers.runCurrent() | ||
|
||
// Verify if the AppVersionActivity is running | ||
var currentActivity: Activity? = null | ||
scenario.onActivity { activity -> | ||
currentActivity = getCurrentActivity() | ||
assertThat(currentActivity).isInstanceOf(AppVersionActivity::class.java) | ||
} | ||
|
||
// Simulate pressing back via activity rather than Espresso | ||
(currentActivity as? AppVersionActivity)?.onBackPressed() | ||
|
||
// Wait for UI update | ||
testCoroutineDispatchers.runCurrent() | ||
|
||
// Verify that AdministratorControlsActivity is visible again | ||
onView(withId(R.id.administrator_controls_list)).check(matches(isDisplayed())) | ||
} | ||
} | ||
|
||
fun getCurrentActivity(): Activity? { | ||
val activity = arrayOfNulls<Activity>(1) | ||
InstrumentationRegistry.getInstrumentation().runOnMainSync { | ||
activity[0] = ActivityLifecycleMonitorRegistry.getInstance() | ||
.getActivitiesInStage(Stage.RESUMED) | ||
.firstOrNull() | ||
} | ||
return activity[0] | ||
} | ||
Comment on lines
+281
to
+289
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
private fun ActivityScenario<AppVersionActivity>.convertTimeStampToDate( | ||
timestampMillis: Long | ||
): String { | ||
|
@@ -264,12 +299,12 @@ class AppVersionActivityTest { | |
return dateTimeString | ||
} | ||
|
||
private fun launchAppVersionActivityIntent(): ActivityScenario<AppVersionActivity> { | ||
val intent = AppVersionActivity.createAppVersionActivityIntent( | ||
ApplicationProvider.getApplicationContext() | ||
) | ||
return ActivityScenario.launch(intent) | ||
} | ||
// private fun launchAppVersionActivityIntent(): ActivityScenario<AppVersionActivity> { | ||
// val intent = AppVersionActivity.createAppVersionActivityIntent( | ||
// ApplicationProvider.getApplicationContext() | ||
// ) | ||
// return ActivityScenario.launch(intent) | ||
// } | ||
Comment on lines
+302
to
+307
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When submitting a PR, be sure to delete any commented out code. |
||
|
||
private fun launchAdministratorControlsActivityIntent(internalProfileId: Int): Intent { | ||
val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build() | ||
|
@@ -308,7 +343,8 @@ class AppVersionActivityTest { | |
SyncStatusModule::class, MetricLogSchedulerModule::class, TestingBuildFlavorModule::class, | ||
ActivityRouterModule::class, | ||
CpuPerformanceSnapshotterModule::class, ExplorationProgressModule::class, | ||
TestAuthenticationModule::class | ||
TestAuthenticationModule::class, | ||
AppLanguageLocaleModule::class, | ||
] | ||
) | ||
interface TestApplicationComponent : ApplicationComponent { | ||
|
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.
There should be no spacing after a KDoc.