Skip to content
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 #138: Topic train fragment Low-fi UI (Part 4) #204

Merged
merged 41 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8cdfb33
All xml files for topic-train except layout folder
Oct 3, 2019
afd8196
Skill item related files
Oct 3, 2019
87977ab
TopicTrainFragment implementation
Oct 3, 2019
456d129
Topic Train Test Case
Oct 3, 2019
d21f52b
Introduce QuestionPlayer Activity/Fragment
Oct 4, 2019
e39fa93
Domain layer code integration
Oct 5, 2019
9ff0ff5
Resolve merge conflict develop
Oct 5, 2019
685bd2d
Merge remote-tracking branch 'upstream/topic-train-low-fi-part-3' int…
Oct 5, 2019
1cd605f
Start button activated
Oct 5, 2019
e30e58a
Start button activated
Oct 5, 2019
b1cef82
Separate ViewModel for item
Oct 5, 2019
23e4d27
Managing configuration changes
Oct 5, 2019
7fe1f87
Checkbox state save
Oct 5, 2019
3d0315a
Nit changes
Oct 5, 2019
6c5387f
Fixing HomeActivityController
Oct 6, 2019
5b2dc90
Managing configuration change in checkboxes
Oct 6, 2019
b9b9683
Proper route to QuestionPlayer
Oct 6, 2019
b578d34
Merge remote-tracking branch 'upstream/topic-train-low-fi-part-3' int…
Oct 6, 2019
37a4837
QuestionPlayerActivity test case
Oct 6, 2019
6321130
Nit changes
Oct 6, 2019
de7568a
Merge remote-tracking branch 'upstream/topic-train-low-fi-part-3' int…
Oct 6, 2019
7bb449b
More test cases
Oct 7, 2019
06f03b9
Nit changes
Oct 7, 2019
dcdb082
Resolve merge conflicts
Oct 7, 2019
6968d06
Merge remote-tracking branch 'upstream/topic-train-low-fi-part-3' int…
Oct 7, 2019
81eee40
EOF added
Oct 7, 2019
9f2ad98
Resolve merge conflicts
Oct 8, 2019
45b1b18
Test nit changes
Oct 8, 2019
04125a0
Merge remote-tracking branch 'upstream/develop' into topic-train-low-…
Oct 9, 2019
d9f586d
Test case updated
Oct 9, 2019
1208a4a
Merge remote-tracking branch 'upstream/develop' into topic-train-low-…
Oct 10, 2019
b7fb1e4
Updated RecyclerViewMatcher
Oct 10, 2019
9e81bb5
Nit change
Oct 10, 2019
5274077
Test added
Oct 11, 2019
ba680a3
Merge remote-tracking branch 'upstream/develop' into topic-train-low-…
Oct 11, 2019
3d03e47
Merge remote-tracking branch 'upstream/develop' into topic-train-low-…
Oct 15, 2019
1b446ff
Nit changes
Oct 15, 2019
2e9a334
Nit changes
Oct 15, 2019
79b11ff
Merge remote-tracking branch 'upstream/develop' into topic-train-low-…
Oct 18, 2019
5b9d0dd
Test cases
Oct 18, 2019
8a5f22f
Test case updated
Oct 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,9 @@ class QuestionPlayerActivity : InjectableAppCompatActivity() {
intent.putExtra(QUESTION_PLAYER_ACTIVITY_SKILL_ID_LIST_ARGUMENT_KEY, skillIdList)
return intent
}

fun getIntentKey():String{
rt4914 marked this conversation as resolved.
Show resolved Hide resolved
return QUESTION_PLAYER_ACTIVITY_SKILL_ID_LIST_ARGUMENT_KEY
}
}
}
54 changes: 54 additions & 0 deletions app/src/sharedTest/java/org/oppia/app/RecyclerViewMatcher.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package org.oppia.app

import android.content.res.Resources
import android.view.View
import androidx.recyclerview.widget.RecyclerView
import org.hamcrest.Description
import org.hamcrest.Matcher
import org.hamcrest.TypeSafeMatcher

class RecyclerViewMatcher(private val recyclerViewId: Int) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe use the one in BindableAdapterTest, instead? Prefer extracting that one to here for reuse since I think that one has all the necessary functionality, and is simpler.

Also, please add documentation and credit links where applicable (if you didn't derive this solution on your own).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not able to use code from BindableAdapterTest , but I have updated the code for better readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I have created a custom adapter for this TopicTrainFragment, so will the BindableAdapterTest work with this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do we need the wrapper RecyclerViewMatcher? Could we just have a top-level Kotlin file with the atPosition method and import it where needed? The BindableAdapterTest version lends itself to this approach since it follows the standard Espresso matcher idiom of being a static-esque, standalone method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed wrapper and started using atPosition and atPositionView method directly.

fun atPosition(position: Int): Matcher<View> {
return atPositionOnView(position, -1)
}

fun atPositionOnView(position: Int, targetViewId: Int): Matcher<View> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should have brief documentation to explain how to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added brief explanation of this method.

return object : TypeSafeMatcher<View>() {
var resources: Resources? = null
var childView: View? = null

override fun describeTo(description: Description) {
var idDescription = Integer.toString(recyclerViewId)
if (this.resources != null) {
idDescription = try {
this.resources!!.getResourceName(recyclerViewId)
} catch (var4: Resources.NotFoundException) {
String.format(
"%s (resource name not found)",
recyclerViewId
)
}
}
description.appendText("with id: $idDescription")
}

public override fun matchesSafely(view: View): Boolean {
this.resources = view.resources
if (childView == null) {
val recyclerView = view.rootView.findViewById<View>(recyclerViewId) as RecyclerView
if (recyclerView.id == recyclerViewId) {
childView = recyclerView.findViewHolderForAdapterPosition(position)!!.itemView
} else {
return false
}
}
return if (targetViewId == -1) {
view === childView
} else {
val targetView = childView!!.findViewById<View>(targetViewId)
view === targetView
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import kotlinx.coroutines.CoroutineDispatcher
import org.junit.Test
import org.junit.runner.RunWith
import org.oppia.app.R
import org.oppia.app.player.exploration.ExplorationActivity
import org.oppia.util.threading.BackgroundDispatcher
import org.oppia.util.threading.BlockingDispatcher
import javax.inject.Singleton
Expand All @@ -27,7 +26,7 @@ class TopicActivityTest {

@Test
fun testTopicActivity_loadTopicFragment_hasDummyString() {
ActivityScenario.launch(ExplorationActivity::class.java).use {
ActivityScenario.launch(TopicActivity::class.java).use {
onView(withId(R.id.dummy_text_view)).check(matches(withText("This is dummy TextView for testing")))
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package org.oppia.app.topic.questionplayer

import android.app.Application
import android.content.Context
import androidx.test.core.app.ActivityScenario
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.runners.AndroidJUnit4
import dagger.BindsInstance
import dagger.Component
import dagger.Module
import dagger.Provides
import kotlinx.coroutines.CoroutineDispatcher
import org.junit.Test
import org.junit.runner.RunWith
import org.oppia.app.R
import org.oppia.util.threading.BackgroundDispatcher
import org.oppia.util.threading.BlockingDispatcher
import javax.inject.Singleton

/** Tests for [QuestionPlayerActivity]. */
@RunWith(AndroidJUnit4::class)
class QuestionPlayerActivityTest {

@Test
fun testQuestionPlayerActivity_loadQuestionPlayerFragment_hasDummyString() {
ActivityScenario.launch(QuestionPlayerActivity::class.java).use {
onView(withId(R.id.dummy_text_view)).check(matches(withText("This is dummy TextView for testing")))
}
}

@Module
class TestModule {
@Provides
@Singleton
fun provideContext(application: Application): Context {
return application
}

// TODO(#89): Introduce a proper IdlingResource for background dispatchers to ensure they all complete before
// proceeding in an Espresso test. This solution should also be interoperative with Robolectric contexts by using a
// test coroutine dispatcher.

@Singleton
@Provides
@BackgroundDispatcher
fun provideBackgroundDispatcher(@BlockingDispatcher blockingDispatcher: CoroutineDispatcher): CoroutineDispatcher {
return blockingDispatcher
}
}

@Singleton
@Component(modules = [TestModule::class])
interface TestApplicationComponent {
@Component.Builder
interface Builder {
@BindsInstance
fun setApplication(application: Application): Builder

fun build(): TestApplicationComponent
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,133 @@ package org.oppia.app.topic.train

import android.app.Application
import android.content.Context
import android.content.pm.ActivityInfo
import androidx.test.core.app.ActivityScenario
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.assertion.ViewAssertions.matches
import androidx.test.espresso.intent.Intents.intended
import androidx.test.espresso.matcher.ViewMatchers.hasDescendant
import androidx.test.espresso.matcher.ViewMatchers.isClickable
import androidx.test.espresso.matcher.ViewMatchers.withId
import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra
import androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent
import dagger.BindsInstance
import dagger.Component
import dagger.Module
import dagger.Provides
import kotlinx.coroutines.CoroutineDispatcher
import org.hamcrest.Matchers.not
import org.junit.Test
import org.junit.runner.RunWith
import org.oppia.app.R
import org.oppia.app.topic.TopicActivity
import org.oppia.util.threading.BackgroundDispatcher
import org.oppia.util.threading.BlockingDispatcher
import javax.inject.Singleton
import androidx.test.espresso.intent.Intents
import androidx.test.espresso.matcher.ViewMatchers.isChecked
import org.oppia.app.R
import org.oppia.app.RecyclerViewMatcher
import org.junit.After
import org.junit.Before
import org.junit.Rule
import androidx.test.rule.ActivityTestRule
import org.oppia.app.topic.questionplayer.QuestionPlayerActivity

/** Tests for [TopicTrainFragment]. */
@RunWith(AndroidJUnit4::class)
class TopicTrainFragmentTest {

private var skillIdList = ArrayList<String>()

@get:Rule
var activityTestRule: ActivityTestRule<TopicActivity> = ActivityTestRule(
TopicActivity::class.java, /* initialTouchMode= */ true, /* launchActivity= */ false
)

@Before
fun setUp() {
Intents.init()
skillIdList.add("test_skill_id_0")
skillIdList.add("test_skill_id_1")
}

@Test
rt4914 marked this conversation as resolved.
Show resolved Hide resolved
fun testTopicTrainFragment_loadFragment_displaySkills_startButtonIsInactive() {
ActivityScenario.launch(TopicActivity::class.java).use {
onView(withId(R.id.master_skills_text_view)).check(matches(withText(R.string.topic_train_master_these_skills)))
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0))
.check(matches(hasDescendant(withId(R.id.skill_check_box))))
onView(withId(R.id.topic_train_start_button)).check(matches(not(isClickable())))
}
}

@Test
fun testTopicTrainFragment_loadFragment_textIsDisplayed() {
fun testTopicTrainFragment_loadFragment_selectSkills_deselectSkills_startButtonIsInactive() {
ActivityScenario.launch(TopicActivity::class.java).use {
onView(withId(R.id.dummy_text_view)).check(matches(withText("This is dummy TextView for testing")))
onView(withId(R.id.master_skills_text_view)).check(matches(withText(R.string.topic_train_master_these_skills)))
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0)).perform(click())
onView(withId(R.id.topic_train_start_button)).check(matches(isClickable()))
rt4914 marked this conversation as resolved.
Show resolved Hide resolved
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0)).perform(click())
onView(withId(R.id.topic_train_start_button)).check(matches(not(isClickable())))
}
}

@Test
fun testTopicTrainFragment_loadFragment_selectSkills_clickStartButton_skillListTransferSuccessfully() {
activityTestRule.launchActivity(null)
onView(withId(R.id.master_skills_text_view)).check(matches(withText(R.string.topic_train_master_these_skills)))
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0)).perform(click())
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(1)).perform(click())
onView(withId(R.id.topic_train_start_button)).check(matches(isClickable()))
onView(withId(R.id.topic_train_start_button)).perform(click())
intended(hasComponent(QuestionPlayerActivity::class.java.name))
intended(hasExtra(QuestionPlayerActivity.getIntentKey(), skillIdList))
}

@Test
fun testTopicTrainFragment_loadFragment_selectSkills_configurationChange_skillsAreSelected() {
activityTestRule.launchActivity(null)
onView(withId(R.id.master_skills_text_view)).check(matches(withText(R.string.topic_train_master_these_skills)))
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0)).perform(click())
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe this is sufficient for a configuration change. Suggest instead: ActivityScenario.recreate()

Ditto elsewhere for configuration changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means I have add this or do I have to remove the current line and replace this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using both and merge code for now, because the test-cases passes in all possible combinations, using current code only, using your code only and using both.

onView(
withRecyclerView(R.id.skill_recycler_view).atPositionOnView(
0,
R.id.skill_check_box
)
).check(matches(isChecked()))
}

@Test
fun testTopicTrainFragment_loadFragment_configurationChange_startButtonRemainsInactive() {
activityTestRule.launchActivity(null)
onView(withId(R.id.topic_train_start_button)).check(matches(not(isClickable())))
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
onView(withId(R.id.topic_train_start_button)).check(matches(not(isClickable())))
}

@Test
fun testTopicTrainFragment_loadFragment_selectSkills_configurationChange_startButtonRemainsActive() {
activityTestRule.launchActivity(null)
onView(withId(R.id.master_skills_text_view)).check(matches(withText(R.string.topic_train_master_these_skills)))
onView(withRecyclerView(R.id.skill_recycler_view).atPosition(0)).perform(click())
onView(withId(R.id.topic_train_start_button)).check(matches(isClickable()))
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
onView(withId(R.id.topic_train_start_button)).check(matches(isClickable()))
}

private fun withRecyclerView(recyclerViewId: Int): RecyclerViewMatcher {
return RecyclerViewMatcher(recyclerViewId)
}

@After
fun tearDown() {
Intents.release()
}

@Module
class TestModule {
@Provides
Expand Down