From 5bbde3b77970ca22eea803865a454af6f7099054 Mon Sep 17 00:00:00 2001 From: Jorge Antonio Diaz-Benito Soriano Date: Wed, 10 May 2017 18:28:56 +0200 Subject: [PATCH] Finish Dagger adoption. Fix tests. Fix #51. Fix #57. --- .../TopGamingActivityInstrumentation.kt | 111 +++++++++++------- ...amingPostsActivityInstrumentationModule.kt | 34 +++--- .../gaming/TopGamingAllTimePostsActivity.kt | 6 +- .../TopGamingAllTimePostsContentViewConfig.kt | 2 +- .../kotlin/data/top/TopRequestSourceSpek.kt | 11 +- .../util/android/test/BinaryIdlingResource.kt | 1 - .../IndexedPersistedByDiskStoreSpek.kt | 13 +- 7 files changed, 114 insertions(+), 64 deletions(-) diff --git a/app/src/androidTest/kotlin/app/gaming/TopGamingActivityInstrumentation.kt b/app/src/androidTest/kotlin/app/gaming/TopGamingActivityInstrumentation.kt index 80e6cfe..137f481 100644 --- a/app/src/androidTest/kotlin/app/gaming/TopGamingActivityInstrumentation.kt +++ b/app/src/androidTest/kotlin/app/gaming/TopGamingActivityInstrumentation.kt @@ -2,26 +2,34 @@ package app.gaming import android.support.test.espresso.Espresso import android.support.test.espresso.Espresso.onView -import android.support.test.espresso.Espresso.pressBack import android.support.test.espresso.NoActivityResumedException import android.support.test.espresso.assertion.ViewAssertions.matches -import android.support.test.espresso.matcher.ViewMatchers.* +import android.support.test.espresso.matcher.ViewMatchers.isAssignableFrom +import android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed +import android.support.test.espresso.matcher.ViewMatchers.isDisplayed +import android.support.test.espresso.matcher.ViewMatchers.withId +import android.support.test.espresso.matcher.ViewMatchers.withText +import android.support.test.filters.FlakyTest import android.support.test.rule.ActivityTestRule import android.support.v7.widget.Toolbar import android.view.View import domain.entity.Post import org.jorge.ms.app.R +import org.junit.After import org.junit.Rule import org.junit.Test import org.junit.rules.ExpectedException +import org.junit.rules.Timeout import rx.Subscriber +import rx.subjects.PublishSubject import util.android.test.BinaryIdlingResource +import java.net.UnknownHostException import kotlin.test.assertEquals /** - * Instrumentation for TopGamingActivityInstrumentation. Here we could tests other things like - * the error and progress views being shown when they are supposed to or the toolbar hiding on - * scroll. However this would require using mockito, which breaks the dex limit, so we + * IdlingResources are not fully reliable: sometimes the test runs fail to allow the main thread to + * go idle correctly, which causes a fail. Should that be your case, re-run and hopefully you'll get + * better luck. @FlakyTest is used because of this. */ internal class TopGamingActivityInstrumentation { @JvmField @@ -30,27 +38,45 @@ internal class TopGamingActivityInstrumentation { @JvmField @Rule val expectedException: ExpectedException = ExpectedException.none() + @JvmField + @Rule + val globalTimeout: Timeout = Timeout.seconds(5) + + @After + fun afterTest() { + Espresso.unregisterIdlingResources(IDLING_RESOURCE) + } + @FlakyTest @Test fun activityIsShown() { + PUBLISH_SUBJECT.onCompleted() + Espresso.registerIdlingResources(IDLING_RESOURCE) onView(withId(android.R.id.content)).check { view, _ -> - assertEquals(View.VISIBLE, view.visibility, "Window visibility was not VISIBLE") } + assertEquals(View.VISIBLE, view.visibility, "Window visibility was not VISIBLE") } } + @FlakyTest @Test fun toolbarIsCompletelyShownOnOpening() { + PUBLISH_SUBJECT.onCompleted() + Espresso.registerIdlingResources(IDLING_RESOURCE) val completelyDisplayedMatcher = matches(isCompletelyDisplayed()) onView(isAssignableFrom(Toolbar::class.java)).check(completelyDisplayedMatcher) onView(withText(R.string.app_label)).check(completelyDisplayedMatcher) } + @FlakyTest @Test fun goingBackPausesApp() { + PUBLISH_SUBJECT.onCompleted() + Espresso.registerIdlingResources(IDLING_RESOURCE) expectedException.expect(NoActivityResumedException::class.java) expectedException.expectMessage("Pressed back and killed the app") - pressBack() + Espresso.pressBack() } + @FlakyTest @Test fun openingShowsProgress() { onView(withId(R.id.progress)).check { view, _ -> @@ -61,8 +87,11 @@ internal class TopGamingActivityInstrumentation { assertEquals(View.VISIBLE, view.visibility, "Content visibility was not VISIBLE") } } + @FlakyTest @Test fun onLoadItemsAreShown() { + PUBLISH_SUBJECT.onNext(Post("Bananas title", "r/bananas", 879, "bananaLink")) + PUBLISH_SUBJECT.onCompleted() Espresso.registerIdlingResources(IDLING_RESOURCE) onView(withId(R.id.progress)).check { view, _ -> assertEquals(View.GONE, view.visibility, "Progress visibility was not GONE") } @@ -70,46 +99,48 @@ internal class TopGamingActivityInstrumentation { assertEquals(View.GONE, view.visibility, "Error visibility was not GONE") } onView(withId(R.id.content)).check { view, _ -> assertEquals(View.VISIBLE, view.visibility, "Content visibility was not VISIBLE") } - // TODO Check for data content - Espresso.unregisterIdlingResources(IDLING_RESOURCE) + onView(withText("Bananas title")).check(matches(isDisplayed())) } -// @Test -// fun onFailureErrorIsShown() { -// TEST_OBSERVABLE_FACTORY_METHOD = { _ -> Observable.error(UnknownHostException())} -// Espresso.registerIdlingResources(IDLING_RESOURCE) -// onView(withId(R.id.progress)).check { view, _ -> -// assertEquals(View.GONE, view.visibility, "Progress visibility was not GONE") } -// onView(withId(R.id.error)).check { view, _ -> -// assertEquals(View.VISIBLE, view.visibility, "Error visibility was not VISIBLE") } -// onView(withId(R.id.content)).check { view, _ -> -// assertEquals(View.VISIBLE, view.visibility, "Content visibility was not VISIBLE") } -// Espresso.unregisterIdlingResources(IDLING_RESOURCE) -// } -} + @FlakyTest + @Test + fun onFailureErrorIsShown() { + PUBLISH_SUBJECT.onError(UnknownHostException()) + Espresso.registerIdlingResources(IDLING_RESOURCE) + onView(withId(R.id.progress)).check { view, _ -> + assertEquals(View.GONE, view.visibility, "Progress visibility was not GONE") } + onView(withId(R.id.error)).check { view, _ -> + assertEquals(View.VISIBLE, view.visibility, "Error visibility was not VISIBLE") } + onView(withId(R.id.content)).check { view, _ -> + assertEquals(View.VISIBLE, view.visibility, "Content visibility was not VISIBLE") } + } -private val IDLING_RESOURCE = BinaryIdlingResource("load") -internal val SUBSCRIBER_GENERATOR: (TopGamingAllTimePostsCoordinator) -> Subscriber = { - object : Subscriber() { - private val realSubscriberDelegate = PageLoadSubscriber(it) + companion object { + private val IDLING_RESOURCE = BinaryIdlingResource("load") + internal lateinit var PUBLISH_SUBJECT: PublishSubject + internal val SUBSCRIBER_GENERATOR: (TopGamingAllTimePostsCoordinator) -> Subscriber = { + object : Subscriber() { + private val realSubscriberDelegate = PageLoadSubscriber(it) - override fun onStart() { - realSubscriberDelegate.onStart() - IDLING_RESOURCE.setIdleState(false) - } + override fun onStart() { + realSubscriberDelegate.onStart() + IDLING_RESOURCE.setIdleState(false) + } - override fun onNext(post: Post?) { - realSubscriberDelegate.onNext(post) - } + override fun onNext(post: Post?) { + realSubscriberDelegate.onNext(post) + } - override fun onError(throwable: Throwable?) { - realSubscriberDelegate.onError(throwable) - IDLING_RESOURCE.setIdleState(true) - } + override fun onError(throwable: Throwable?) { + realSubscriberDelegate.onError(throwable) + IDLING_RESOURCE.setIdleState(true) + } - override fun onCompleted() { - realSubscriberDelegate.onCompleted() - IDLING_RESOURCE.setIdleState(true) + override fun onCompleted() { + realSubscriberDelegate.onCompleted() + IDLING_RESOURCE.setIdleState(true) + } + } } } } diff --git a/app/src/androidTest/kotlin/app/gaming/TopGamingPostsActivityInstrumentationModule.kt b/app/src/androidTest/kotlin/app/gaming/TopGamingPostsActivityInstrumentationModule.kt index 606a542..2d3d690 100644 --- a/app/src/androidTest/kotlin/app/gaming/TopGamingPostsActivityInstrumentationModule.kt +++ b/app/src/androidTest/kotlin/app/gaming/TopGamingPostsActivityInstrumentationModule.kt @@ -6,8 +6,11 @@ import android.content.Intent import android.net.Uri import android.os.Build import android.support.v7.widget.RecyclerView +import android.util.Log import android.view.View import app.UIPostExecutionThread +import app.gaming.TopGamingActivityInstrumentation.Companion.PUBLISH_SUBJECT +import app.gaming.TopGamingActivityInstrumentation.Companion.SUBSCRIBER_GENERATOR import dagger.Component import dagger.Module import dagger.Provides @@ -15,8 +18,7 @@ import domain.entity.Post import domain.exec.PostExecutionThread import domain.interactor.TopGamingAllTimePostsUseCase import org.jorge.ms.app.BuildConfig -import rx.Observable -import java.util.concurrent.TimeUnit +import rx.subjects.PublishSubject import javax.inject.Singleton /** @@ -57,8 +59,8 @@ internal class TopGamingAllTimePostsFeatureInstrumentationModule( @Provides @Singleton fun pageLoadSubscriberFactory() = object : PageLoadSubscriber.Factory { - override fun newSubscriber(coordinator: TopGamingAllTimePostsCoordinator) - = SUBSCRIBER_GENERATOR(coordinator) + override fun newSubscriber(coordinator: TopGamingAllTimePostsCoordinator) = + SUBSCRIBER_GENERATOR(coordinator) } @Provides @@ -70,20 +72,18 @@ internal class TopGamingAllTimePostsFeatureInstrumentationModule( @Provides @Singleton - fun topGamingAllTimePostsUseCaseFactory(): TopGamingAllTimePostsUseCase.Factory = - object : TopGamingAllTimePostsUseCase.Factory { - override fun newFetch(page: Int, postExecutionThread: PostExecutionThread) = - object : TopGamingAllTimePostsUseCase(page, UIPostExecutionThread) { - override fun buildUseCaseObservable() = Observable.just(Post( - "Page $page", "asubreddit", page, - "https://www.page$page.com")) - .delay(125, TimeUnit.MILLISECONDS) - .repeat(8) - } + fun topGamingAllTimePostsUseCaseFactory(): TopGamingAllTimePostsUseCase.Factory { + PUBLISH_SUBJECT = PublishSubject.create() + return object : TopGamingAllTimePostsUseCase.Factory { + override fun newFetch(page: Int, postExecutionThread: PostExecutionThread) = + object : TopGamingAllTimePostsUseCase(page, UIPostExecutionThread) { + override fun buildUseCaseObservable() = PUBLISH_SUBJECT + } - override fun newGet(page: Int, postExecutionThread: PostExecutionThread) = - newFetch(page, postExecutionThread) - } + override fun newGet(page: Int, postExecutionThread: PostExecutionThread) = + newFetch(page, postExecutionThread) + } + } @Provides @Singleton diff --git a/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsActivity.kt b/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsActivity.kt index 8986c40..f64672a 100644 --- a/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsActivity.kt +++ b/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsActivity.kt @@ -6,8 +6,10 @@ import android.os.Bundle import android.support.v7.app.AppCompatActivity import app.MainApplication import domain.entity.Post -import kotlinx.android.synthetic.main.include_toolbar.* -import kotlinx.android.synthetic.main.include_top_posts_view.* +import kotlinx.android.synthetic.main.include_toolbar.toolbar +import kotlinx.android.synthetic.main.include_top_posts_view.progress +import kotlinx.android.synthetic.main.include_top_posts_view.error +import kotlinx.android.synthetic.main.include_top_posts_view.content import org.jorge.ms.app.R import javax.inject.Inject diff --git a/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsContentViewConfig.kt b/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsContentViewConfig.kt index b3f3e5f..c9e6c94 100644 --- a/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsContentViewConfig.kt +++ b/app/src/main/kotlin/app/gaming/TopGamingAllTimePostsContentViewConfig.kt @@ -56,7 +56,7 @@ internal class TopGamingAllTimePostsContentViewConfig( * An alternative would have been to use the databinding library, but the fact that it does not * support merge layouts would make diverse screen support more complicated. */ -internal class Adapter(private val listener: OnItemClickListener) +internal class Adapter(private val listener: OnItemClickListener) : RecyclerView.Adapter() { private val items = mutableListOf() private lateinit var recyclerView: RecyclerView diff --git a/data/src/test/kotlin/data/top/TopRequestSourceSpek.kt b/data/src/test/kotlin/data/top/TopRequestSourceSpek.kt index 1029342..6bef8ee 100644 --- a/data/src/test/kotlin/data/top/TopRequestSourceSpek.kt +++ b/data/src/test/kotlin/data/top/TopRequestSourceSpek.kt @@ -1,6 +1,14 @@ package data.top -import com.nhaarman.mockito_kotlin.* +import com.nhaarman.mockito_kotlin.anyVararg +import com.nhaarman.mockito_kotlin.doReturn +import com.nhaarman.mockito_kotlin.eq +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.reset +import com.nhaarman.mockito_kotlin.times +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.verifyNoMoreInteractions +import com.nhaarman.mockito_kotlin.whenever import com.nytimes.android.external.store.base.impl.Store import dagger.Component import dagger.Module @@ -10,6 +18,7 @@ import domain.entity.TimeRange import domain.interactor.TopGamingAllTimePostsUseCase import org.jetbrains.spek.api.SubjectSpek import org.jetbrains.spek.api.dsl.it +import org.jetbrains.spek.api.dsl.on import org.junit.platform.runner.JUnitPlatform import org.junit.runner.RunWith import retrofit2.Retrofit diff --git a/util-android-test/src/main/kotlin/util/android/test/BinaryIdlingResource.kt b/util-android-test/src/main/kotlin/util/android/test/BinaryIdlingResource.kt index e142039..fdc6b85 100644 --- a/util-android-test/src/main/kotlin/util/android/test/BinaryIdlingResource.kt +++ b/util-android-test/src/main/kotlin/util/android/test/BinaryIdlingResource.kt @@ -23,7 +23,6 @@ class BinaryIdlingResource(private val name: String) : IdlingResource { * @param isIdleNow The new idle state for the resource. */ fun setIdleState(isIdleNow: Boolean) { - if (isIdle.get() == isIdleNow) return isIdle.set(isIdleNow) if (isIdleNow) { resourceCallback?.onTransitionToIdle() diff --git a/util-android/src/test/kotlin/util/android/IndexedPersistedByDiskStoreSpek.kt b/util-android/src/test/kotlin/util/android/IndexedPersistedByDiskStoreSpek.kt index 3f14e13..1addc95 100644 --- a/util-android/src/test/kotlin/util/android/IndexedPersistedByDiskStoreSpek.kt +++ b/util-android/src/test/kotlin/util/android/IndexedPersistedByDiskStoreSpek.kt @@ -1,6 +1,15 @@ package util.android -import com.nhaarman.mockito_kotlin.* +import com.nhaarman.mockito_kotlin.any +import com.nhaarman.mockito_kotlin.doAnswer +import com.nhaarman.mockito_kotlin.doReturn +import com.nhaarman.mockito_kotlin.eq +import com.nhaarman.mockito_kotlin.mock +import com.nhaarman.mockito_kotlin.reset +import com.nhaarman.mockito_kotlin.spy +import com.nhaarman.mockito_kotlin.verify +import com.nhaarman.mockito_kotlin.verifyZeroInteractions +import com.nhaarman.mockito_kotlin.whenever import org.jetbrains.spek.api.SubjectSpek import org.jetbrains.spek.api.dsl.it import org.junit.platform.runner.JUnitPlatform @@ -57,7 +66,7 @@ internal class IndexedPersistedByDiskStoreSpek : SubjectSpek "$value=${values[index]}" }.joinToString(separator = "\n")) - whenever(MOCK_VALUE_STRINGIFIER.fromString(any())) doAnswer { it.arguments[0] } + whenever(MOCK_VALUE_STRINGIFIER.fromString(any())) doAnswer { it.arguments[0] } subject.restore() values.forEachIndexed { index, value -> verify(MOCK_VALUE_STRINGIFIER).fromString(eq(values[index]))