-
Notifications
You must be signed in to change notification settings - Fork 527
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 #5485: Create means for verifying Fragment Arguments #5537
Changes from 13 commits
d819e70
c6f54ea
d44015b
d604bc2
cc0ddef
52c6bb1
773c810
56af5ae
96b6f44
6afebe6
2ecb6b2
70e0674
b7bd4e6
002ceba
07ec171
b261b7c
906e8c8
19dc149
8c580ac
f3aea03
e09e428
820569c
ab0977c
17e6174
fe46540
94fd8be
492a54c
a4aeed9
168c98d
e78a3fa
2fdb5c6
ec3f615
368af6c
9d4edf0
2837f94
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 | ||
---|---|---|---|---|
|
@@ -8,6 +8,7 @@ import android.view.ViewParent | |||
import android.widget.FrameLayout | ||||
import androidx.appcompat.app.AppCompatActivity | ||||
import androidx.core.widget.NestedScrollView | ||||
import androidx.drawerlayout.widget.DrawerLayout | ||||
import androidx.test.core.app.ActivityScenario.launch | ||||
import androidx.test.core.app.ApplicationProvider | ||||
import androidx.test.espresso.Espresso.onView | ||||
|
@@ -23,6 +24,7 @@ import androidx.test.espresso.matcher.ViewMatchers.isClickable | |||
import androidx.test.espresso.matcher.ViewMatchers.isRoot | ||||
import androidx.test.espresso.util.HumanReadables | ||||
import androidx.test.ext.junit.runners.AndroidJUnit4 | ||||
import com.google.common.truth.Truth.assertThat | ||||
import dagger.Component | ||||
import org.hamcrest.Matcher | ||||
import org.hamcrest.Matchers | ||||
|
@@ -44,6 +46,7 @@ import org.oppia.android.app.application.ApplicationStartupListenerModule | |||
import org.oppia.android.app.application.testing.TestingBuildFlavorModule | ||||
import org.oppia.android.app.devoptions.DeveloperOptionsModule | ||||
import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule | ||||
import org.oppia.android.app.model.AdministratorControlsFragmentArguments | ||||
import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule | ||||
import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.atPositionOnView | ||||
import org.oppia.android.app.recyclerview.RecyclerViewMatcher.Companion.scrollToPosition | ||||
|
@@ -98,6 +101,7 @@ import org.oppia.android.testing.time.FakeOppiaClockModule | |||
import org.oppia.android.util.accessibility.AccessibilityTestModule | ||||
import org.oppia.android.util.caching.AssetModule | ||||
import org.oppia.android.util.caching.testing.CachingTestModule | ||||
import org.oppia.android.util.extensions.getProto | ||||
import org.oppia.android.util.gcsresource.GcsResourceModule | ||||
import org.oppia.android.util.locale.LocaleProdModule | ||||
import org.oppia.android.util.logging.EventLoggingConfigurationModule | ||||
|
@@ -434,6 +438,36 @@ class AdministratorControlsFragmentTest { | |||
} | ||||
} | ||||
|
||||
@Test | ||||
fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { | ||||
launch<AdministratorControlsFragmentTestActivity>( | ||||
createAdministratorControlsFragmentTestActivityIntent( | ||||
profileId = internalProfileId | ||||
) | ||||
).use { scenario -> | ||||
testCoroutineDispatchers.runCurrent() | ||||
scenario.onActivity { activity -> | ||||
|
||||
val administratorControlsFragment = activity.supportFragmentManager | ||||
.findFragmentById(R.id.administrator_controls_fragment_test_activity_fragment_container) | ||||
as AdministratorControlsFragment | ||||
val isMultipane = activity | ||||
.findViewById<DrawerLayout>(R.id.administrator_controls_activity_drawer_layout) != null | ||||
|
||||
val arguments = checkNotNull(administratorControlsFragment.arguments) { | ||||
"Expected arguments to be passed to AdministratorControlsFragment" | ||||
} | ||||
val args = arguments.getProto( | ||||
ADMINISTRATOR_CONTROLS_FRAGMENT_ARGUMENTS_KEY, | ||||
AdministratorControlsFragmentArguments.getDefaultInstance() | ||||
) | ||||
val receivedIsMultipane = args.isMultipane | ||||
|
||||
assertThat(receivedIsMultipane).isEqualTo(isMultipane) | ||||
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. Will 'isMultipane' always be true/false based on the configuration of the test? It would probably be more robust to set up the test such that we can always verify true/false as expected per the test setup (and add a second test for the opposite case so that both 'true' and 'false' are explicitly verified). 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. @BenHenning , Please check, Could we do like this way?
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. I don't think that will work--the activity lifecycle methods need to be handled by Android (and specifically Robolectric when running the tests locally). Normally multi-pane is automatically determined in production code by checking whether the current device is a tablet. We're able to configure tests to run with a tablet display environment (using per-test qualifiers). Would that work in this case? Here's one example of such a test configuration: oppia-android/app/src/sharedTest/java/org/oppia/android/app/help/HelpFragmentTest.kt Line 324 in 55ab73e
|
||||
} | ||||
} | ||||
} | ||||
|
||||
private fun clickAutoUpdateTopicContainer() { | ||||
onView( | ||||
atPositionOnView( | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,6 +80,7 @@ import org.oppia.android.app.devoptions.DeveloperOptionsModule | |
import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule | ||
import org.oppia.android.app.model.OppiaLanguage | ||
import org.oppia.android.app.model.ProfileId | ||
import org.oppia.android.app.model.StateFragmentArguments | ||
import org.oppia.android.app.model.WrittenTranslationLanguageSelection | ||
import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule | ||
import org.oppia.android.app.player.state.itemviewmodel.StateItemViewModel | ||
|
@@ -177,6 +178,7 @@ import org.oppia.android.util.accessibility.AccessibilityTestModule | |
import org.oppia.android.util.caching.AssetModule | ||
import org.oppia.android.util.caching.LoadImagesFromAssets | ||
import org.oppia.android.util.caching.LoadLessonProtosFromAssets | ||
import org.oppia.android.util.extensions.getProto | ||
import org.oppia.android.util.gcsresource.GcsResourceModule | ||
import org.oppia.android.util.locale.LocaleProdModule | ||
import org.oppia.android.util.logging.EventLoggingConfigurationModule | ||
|
@@ -5004,6 +5006,39 @@ class StateFragmentTest { | |
} | ||
} | ||
|
||
@Test | ||
fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { | ||
setUpTestWithLanguageSwitchingFeatureOff() | ||
launchForExploration( | ||
FRACTIONS_EXPLORATION_ID_1, | ||
shouldSavePartialProgress = false | ||
).use { scenario -> | ||
testCoroutineDispatchers.unregisterIdlingResource() | ||
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 unregister the idling resource? This seems rather unusual. |
||
startPlayingExploration() | ||
|
||
scenario.onActivity { activity -> | ||
val stateFragment = activity.supportFragmentManager | ||
.findFragmentById(R.id.state_fragment_placeholder) as StateFragment | ||
|
||
val args = | ||
stateFragment.arguments?.getProto( | ||
StateFragment.STATE_FRAGMENT_ARGUMENTS_KEY, | ||
StateFragmentArguments.getDefaultInstance() | ||
) | ||
|
||
val receivedInternalProfileId = args?.internalProfileId ?: -1 | ||
val receivedTopicId = args?.topicId!! | ||
val receivedStoryId = args.storyId!! | ||
val reveivedExplorationId = args.explorationId!! | ||
|
||
assertThat(receivedInternalProfileId).isEqualTo(profileId.internalId) | ||
assertThat(receivedTopicId).isEqualTo(TEST_TOPIC_ID_0) | ||
assertThat(receivedStoryId).isEqualTo(TEST_STORY_ID_0) | ||
assertThat(reveivedExplorationId).isEqualTo(FRACTIONS_EXPLORATION_ID_1) | ||
} | ||
} | ||
} | ||
|
||
private fun addShadowMediaPlayerException(dataSource: Any, exception: Exception) { | ||
val classLoader = StateFragmentTest::class.java.classLoader!! | ||
val shadowMediaPlayerClass = classLoader.loadClass("org.robolectric.shadows.ShadowMediaPlayer") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,8 @@ import org.oppia.android.app.application.ApplicationStartupListenerModule | |
import org.oppia.android.app.application.testing.TestingBuildFlavorModule | ||
import org.oppia.android.app.devoptions.DeveloperOptionsModule | ||
import org.oppia.android.app.devoptions.DeveloperOptionsStarterModule | ||
import org.oppia.android.app.model.PoliciesActivityParams | ||
import org.oppia.android.app.model.PoliciesFragmentArguments | ||
import org.oppia.android.app.model.PolicyPage | ||
import org.oppia.android.app.player.state.itemviewmodel.SplitScreenInteractionModule | ||
import org.oppia.android.app.shim.ViewBindingShimModule | ||
|
@@ -101,6 +103,8 @@ import org.oppia.android.testing.time.FakeOppiaClockModule | |
import org.oppia.android.util.accessibility.AccessibilityTestModule | ||
import org.oppia.android.util.caching.AssetModule | ||
import org.oppia.android.util.caching.testing.CachingTestModule | ||
import org.oppia.android.util.extensions.getProto | ||
import org.oppia.android.util.extensions.getProtoExtra | ||
import org.oppia.android.util.gcsresource.DefaultResourceBucketName | ||
import org.oppia.android.util.gcsresource.GcsResourceModule | ||
import org.oppia.android.util.locale.LocaleProdModule | ||
|
@@ -325,6 +329,44 @@ class PoliciesFragmentTest { | |
} | ||
} | ||
|
||
@Test | ||
fun testFragment_fragmentLoaded_verifyCorrectArgumentsPassed() { | ||
launch<PoliciesFragmentTestActivity>( | ||
createPoliciesFragmentTestIntent( | ||
getApplicationContext(), | ||
PolicyPage.TERMS_OF_SERVICE | ||
) | ||
).use { scenario -> | ||
testCoroutineDispatchers.runCurrent() | ||
scenario.onActivity { activity -> | ||
|
||
val policiesFragment = activity.supportFragmentManager | ||
.findFragmentById(R.id.policies_fragment_placeholder) as PoliciesFragment | ||
|
||
val policiesActivityParams = activity.intent.getProtoExtra( | ||
PoliciesFragmentTestActivity.POLICIES_FRAGMENT_TEST_POLICY_PAGE_PARAMS_PROTO, | ||
PoliciesActivityParams.getDefaultInstance() | ||
) | ||
val policiesFragmentArguments = | ||
PoliciesFragmentArguments | ||
.newBuilder() | ||
.setPolicyPage(policiesActivityParams.policyPage) | ||
.build() | ||
|
||
val args = checkNotNull(policiesFragment.arguments) { | ||
"Expected arguments to be passed to PoliciesFragment" | ||
} | ||
val receivedPolicies = | ||
args.getProto( | ||
"PoliciesFragment.policy_page", | ||
theMr17 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
PoliciesFragmentArguments.getDefaultInstance() | ||
) | ||
|
||
assertThat(receivedPolicies).isEqualTo(policiesFragmentArguments) | ||
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. Prefer checking the properties of the proto rather than the whole thing (since it helps make the test more deliberate and contextual). |
||
} | ||
} | ||
} | ||
|
||
private fun setUpTestApplicationComponent() { | ||
getApplicationContext<TestApplication>().inject(this) | ||
} | ||
|
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.
Rather than exposing internal state variables for test verification, is it possible to verify that the UI is in the correct state such that we can infer these? For example, could we check that the (expected) default story is currently expanded?