From 50eb1f4b68f5ddbb1057f906e1fec5d5e72aa361 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Tue, 21 Jan 2020 18:05:34 +0530 Subject: [PATCH 01/13] Merge branch 'develop' of https://github.com/oppia/oppia-android into material-cardview-issue-fix-approach-2 # Conflicts: # app/build.gradle --- .../oppia/domain/OnBoardingFlowController.kt | 61 +++++ .../domain/OnboardingFlowControllerTest.kt | 233 ++++++++++++++++++ model/src/main/proto/example.proto | 4 + 3 files changed, 298 insertions(+) create mode 100644 domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt create mode 100644 domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt diff --git a/domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt b/domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt new file mode 100644 index 00000000000..ec24a6db6c7 --- /dev/null +++ b/domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt @@ -0,0 +1,61 @@ +package org.oppia.domain + +import androidx.lifecycle.LiveData +import org.oppia.app.model.OnBoardingFlow +import org.oppia.data.persistence.PersistentCacheStore +import org.oppia.util.data.AsyncResult +import org.oppia.util.data.DataProviders +import org.oppia.util.logging.Logger +import javax.inject.Inject +import javax.inject.Singleton + +/** Controller for persisting and retrieving the user on-boarding information of the app. */ +@Singleton +class OnBoardingFlowController @Inject constructor( + cacheStoreFactory: PersistentCacheStore.Factory, + private val dataProviders: DataProviders, + private val logger: Logger +) { + private val onBoardingFlowStore = cacheStoreFactory.create("on_boarding_flow", OnBoardingFlow.getDefaultInstance()) + + init { + // Prime the cache ahead of time so that any existing history is read prior to any calls to markOnBoardingFlowCompleted(). + onBoardingFlowStore.primeCacheAsync().invokeOnCompletion { + it?.let { + logger.e("DOMAIN", "Failed to prime cache ahead of LiveData conversion for user on-boarding data.", it) + } + } + } + + /** + * Saves that the user has completed on-boarding the app. Note that this does not notify existing subscribers of the changed state, + * nor can future subscribers observe this state until app restart. + */ + fun markOnBoardingFlowCompleted() { + onBoardingFlowStore.storeDataAsync(updateInMemoryCache = false) { + it.toBuilder().setAlreadyOnBoardedApp(true).build() + }.invokeOnCompletion { + it?.let { + logger.e("DOMAIN", "Failed when storing that the user already on-boarded the app.", it) + } + } + } + + /** Clears any indication that the user has previously completed on-boarding the application. */ + fun clearOnBoardingFlow() { + onBoardingFlowStore.clearCacheAsync().invokeOnCompletion { + it?.let { + logger.e("DOMAIN", "Failed to clear onBoarding flow.", it) + } + } + } + + /** + * Returns a [LiveData] result indicating whether the user has on-boarded the app. This is guaranteed to + * provide the state of the store upon the creation of this controller even if [markOnBoardingFlowCompleted] has since been + * called. + */ + fun getOnBoardingFlow(): LiveData> { + return dataProviders.convertToLiveData(onBoardingFlowStore) + } +} diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt new file mode 100644 index 00000000000..60f25b2557e --- /dev/null +++ b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt @@ -0,0 +1,233 @@ +package org.oppia.domain + +import android.app.Application +import android.content.Context +import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.lifecycle.Observer +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import dagger.BindsInstance +import dagger.Component +import dagger.Module +import dagger.Provides +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.ObsoleteCoroutinesApi +import kotlinx.coroutines.newSingleThreadContext +import kotlinx.coroutines.test.TestCoroutineDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runBlockingTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.Captor +import org.mockito.Mock +import org.mockito.Mockito.atLeastOnce +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit +import org.mockito.junit.MockitoRule +import org.oppia.app.model.OnBoardingFlow +import org.oppia.util.data.AsyncResult +import org.oppia.util.logging.EnableConsoleLog +import org.oppia.util.logging.EnableFileLog +import org.oppia.util.logging.GlobalLogLevel +import org.oppia.util.logging.LogLevel +import org.oppia.util.threading.BackgroundDispatcher +import org.oppia.util.threading.BlockingDispatcher +import org.robolectric.annotation.Config +import javax.inject.Inject +import javax.inject.Qualifier +import javax.inject.Singleton +import kotlin.coroutines.EmptyCoroutineContext + +/** Tests for [OnBoardingFlowController]. */ +@RunWith(AndroidJUnit4::class) +@Config(manifest = Config.NONE) +class OnBoardingFlowControllerTest { + @Rule + @JvmField + val mockitoRule: MockitoRule = MockitoJUnit.rule() + + @Rule + @JvmField + val executorRule = InstantTaskExecutorRule() + + @Inject lateinit var onBoardingFlowController: OnBoardingFlowController + + @Inject + @field:TestDispatcher + lateinit var testDispatcher: CoroutineDispatcher + + private val coroutineContext by lazy { + EmptyCoroutineContext + testDispatcher + } + + @Mock + lateinit var mockOnBoardingObserver: Observer> + + @Captor + lateinit var onBoardingResultCaptor: ArgumentCaptor> + + // https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-test/ + @ObsoleteCoroutinesApi + private val testThread = newSingleThreadContext("TestMain") + + @Before + @ExperimentalCoroutinesApi + @ObsoleteCoroutinesApi + fun setUp() { + Dispatchers.setMain(testThread) + setUpTestApplicationComponent() + } + + @After + @ExperimentalCoroutinesApi + @ObsoleteCoroutinesApi + fun tearDown() { + Dispatchers.resetMain() + testThread.close() + } + + private fun setUpTestApplicationComponent() { + DaggerOnBoardingFlowControllerTest_TestApplicationComponent.builder() + .setApplication(ApplicationProvider.getApplicationContext()) + .build() + .inject(this) + } + + @Test + @ExperimentalCoroutinesApi + fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnBoardedTheApp() = runBlockingTest(coroutineContext) { + val onBoarding = onBoardingFlowController.getOnBoardingFlow() + advanceUntilIdle() + onBoarding.observeForever(mockOnBoardingObserver) + + verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) + assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + } + + @Test + @ExperimentalCoroutinesApi + fun testControllerObserver_observedAfterSettingAppOnBoarded_providesLiveData_userDidNotOnBoardApp() = + runBlockingTest(coroutineContext) { + val onBoarding = onBoardingFlowController.getOnBoardingFlow() + + onBoarding.observeForever(mockOnBoardingObserver) + onBoardingFlowController.markOnBoardingFlowCompleted() + advanceUntilIdle() + + // The result should not indicate that the user on-boarded the app because markUserOnBoardedApp does not notify observers + // of the change. + verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) + assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + } + + @Test + @ExperimentalCoroutinesApi + fun testController_settingAppOnBoarded_observedNewController_userOnBoardedApp() = runBlockingTest(coroutineContext) { + onBoardingFlowController.markOnBoardingFlowCompleted() + advanceUntilIdle() + + // Create the controller by creating another singleton graph and injecting it (simulating the app being recreated). + setUpTestApplicationComponent() + val onBoarding = onBoardingFlowController.getOnBoardingFlow() + onBoarding.observeForever(mockOnBoardingObserver) + advanceUntilIdle() + + // The app should be considered on-boarded since a new LiveData instance was observed after marking the app as on-boarded. + verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) + assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isTrue() + } + + @Test + @ExperimentalCoroutinesApi + fun testController_onBoardedApp_cleared_observeNewController_userDidNotOnBoardApp() = runBlockingTest(coroutineContext) { + onBoardingFlowController.markOnBoardingFlowCompleted() + advanceUntilIdle() + + // Clear, then recreate another controller. + onBoardingFlowController.clearOnBoardingFlow() + setUpTestApplicationComponent() + val onBoarding = onBoardingFlowController.getOnBoardingFlow() + onBoarding.observeForever(mockOnBoardingObserver) + advanceUntilIdle() + + // The app should be considered not yet on-boarded since the previous history was cleared. + verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) + assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + } + + @Qualifier + annotation class TestDispatcher + + // TODO(#89): Move this to a common test application component. + @Module + class TestModule { + @Provides + @Singleton + fun provideContext(application: Application): Context { + return application + } + + @ExperimentalCoroutinesApi + @Singleton + @Provides + @TestDispatcher + fun provideTestDispatcher(): CoroutineDispatcher { + return TestCoroutineDispatcher() + } + + @Singleton + @Provides + @BackgroundDispatcher + fun provideBackgroundDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { + return testDispatcher + } + + @Singleton + @Provides + @BlockingDispatcher + fun provideBlockingDispatcher(@TestDispatcher testDispatcher: CoroutineDispatcher): CoroutineDispatcher { + return testDispatcher + } + + // TODO(#59): Either isolate these to their own shared test module, or use the real logging + // module in tests to avoid needing to specify these settings for tests. + @EnableConsoleLog + @Provides + fun provideEnableConsoleLog(): Boolean = true + + @EnableFileLog + @Provides + fun provideEnableFileLog(): Boolean = false + + @GlobalLogLevel + @Provides + fun provideGlobalLogLevel(): LogLevel = LogLevel.VERBOSE + } + + // TODO(#89): Move this to a common test application component. + @Singleton + @Component(modules = [TestModule::class]) + interface TestApplicationComponent { + @Component.Builder + interface Builder { + @BindsInstance + fun setApplication(application: Application): Builder + + fun build(): TestApplicationComponent + } + + fun inject(onBoardingFlowControllerTest: OnBoardingFlowControllerTest) + } +} diff --git a/model/src/main/proto/example.proto b/model/src/main/proto/example.proto index 0c68a3d68b4..d6641b8c35f 100644 --- a/model/src/main/proto/example.proto +++ b/model/src/main/proto/example.proto @@ -9,6 +9,10 @@ message UserAppHistory { bool already_opened_app = 1; } +message OnBoardingFlow { + bool already_on_boarded_app = 1; + } + message TestMessage { int32 version = 1; } From 7f5910c480aab9d98df04dd11887aeeb06d163b4 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Wed, 22 Jan 2020 15:05:41 +0530 Subject: [PATCH 02/13] nit --- model/src/main/proto/example.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/src/main/proto/example.proto b/model/src/main/proto/example.proto index d6641b8c35f..ae1189b08f2 100644 --- a/model/src/main/proto/example.proto +++ b/model/src/main/proto/example.proto @@ -11,7 +11,7 @@ message UserAppHistory { message OnBoardingFlow { bool already_on_boarded_app = 1; - } +} message TestMessage { int32 version = 1; From eb85c75db5ceb6604bd301519b2f313c6fb0d094 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Wed, 22 Jan 2020 15:17:17 +0530 Subject: [PATCH 03/13] nit --- ...rdingFlowControllerTest.kt => OnBoardingFlowControllerTest.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename domain/src/test/java/org/oppia/domain/{OnboardingFlowControllerTest.kt => OnBoardingFlowControllerTest.kt} (100%) diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnBoardingFlowControllerTest.kt similarity index 100% rename from domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt rename to domain/src/test/java/org/oppia/domain/OnBoardingFlowControllerTest.kt From 161c2162e0121fdad5cdc32918c02eab9c0c1f9c Mon Sep 17 00:00:00 2001 From: marysoloman Date: Tue, 11 Feb 2020 14:37:49 +0530 Subject: [PATCH 04/13] change in package structure, and changed on-board to single word onboard, onboarding proto --- .../OnboardingFlowController.kt} | 26 +++--- ...est.kt => OnboardingFlowControllerTest.kt} | 87 ++++++++++--------- model/src/main/proto/example.proto | 4 - model/src/main/proto/onboarding.proto | 12 +++ 4 files changed, 70 insertions(+), 59 deletions(-) rename domain/src/main/java/org/oppia/domain/{OnBoardingFlowController.kt => onboarding/OnboardingFlowController.kt} (67%) rename domain/src/test/java/org/oppia/domain/{OnBoardingFlowControllerTest.kt => OnboardingFlowControllerTest.kt} (66%) create mode 100644 model/src/main/proto/onboarding.proto diff --git a/domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt similarity index 67% rename from domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt rename to domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt index ec24a6db6c7..67fce39e0fb 100644 --- a/domain/src/main/java/org/oppia/domain/OnBoardingFlowController.kt +++ b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt @@ -1,7 +1,7 @@ -package org.oppia.domain +package org.oppia.domain.onboarding import androidx.lifecycle.LiveData -import org.oppia.app.model.OnBoardingFlow +import org.oppia.app.model.OnboardingFlow import org.oppia.data.persistence.PersistentCacheStore import org.oppia.util.data.AsyncResult import org.oppia.util.data.DataProviders @@ -11,16 +11,16 @@ import javax.inject.Singleton /** Controller for persisting and retrieving the user on-boarding information of the app. */ @Singleton -class OnBoardingFlowController @Inject constructor( +class OnboardingFlowController @Inject constructor( cacheStoreFactory: PersistentCacheStore.Factory, private val dataProviders: DataProviders, private val logger: Logger ) { - private val onBoardingFlowStore = cacheStoreFactory.create("on_boarding_flow", OnBoardingFlow.getDefaultInstance()) + private val onboardingFlowStore = cacheStoreFactory.create("on_boarding_flow", OnboardingFlow.getDefaultInstance()) init { - // Prime the cache ahead of time so that any existing history is read prior to any calls to markOnBoardingFlowCompleted(). - onBoardingFlowStore.primeCacheAsync().invokeOnCompletion { + // Prime the cache ahead of time so that any existing history is read prior to any calls to markOnboardingFlowCompleted(). + onboardingFlowStore.primeCacheAsync().invokeOnCompletion { it?.let { logger.e("DOMAIN", "Failed to prime cache ahead of LiveData conversion for user on-boarding data.", it) } @@ -31,8 +31,8 @@ class OnBoardingFlowController @Inject constructor( * Saves that the user has completed on-boarding the app. Note that this does not notify existing subscribers of the changed state, * nor can future subscribers observe this state until app restart. */ - fun markOnBoardingFlowCompleted() { - onBoardingFlowStore.storeDataAsync(updateInMemoryCache = false) { + fun markOnboardingFlowCompleted() { + onboardingFlowStore.storeDataAsync(updateInMemoryCache = false) { it.toBuilder().setAlreadyOnBoardedApp(true).build() }.invokeOnCompletion { it?.let { @@ -42,8 +42,8 @@ class OnBoardingFlowController @Inject constructor( } /** Clears any indication that the user has previously completed on-boarding the application. */ - fun clearOnBoardingFlow() { - onBoardingFlowStore.clearCacheAsync().invokeOnCompletion { + fun clearOnboardingFlow() { + onboardingFlowStore.clearCacheAsync().invokeOnCompletion { it?.let { logger.e("DOMAIN", "Failed to clear onBoarding flow.", it) } @@ -52,10 +52,10 @@ class OnBoardingFlowController @Inject constructor( /** * Returns a [LiveData] result indicating whether the user has on-boarded the app. This is guaranteed to - * provide the state of the store upon the creation of this controller even if [markOnBoardingFlowCompleted] has since been + * provide the state of the store upon the creation of this controller even if [markOnboardingFlowCompleted] has since been * called. */ - fun getOnBoardingFlow(): LiveData> { - return dataProviders.convertToLiveData(onBoardingFlowStore) + fun getOnboardingFlow(): LiveData> { + return dataProviders.convertToLiveData(onboardingFlowStore) } } diff --git a/domain/src/test/java/org/oppia/domain/OnBoardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt similarity index 66% rename from domain/src/test/java/org/oppia/domain/OnBoardingFlowControllerTest.kt rename to domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt index 60f25b2557e..d193357bbfc 100644 --- a/domain/src/test/java/org/oppia/domain/OnBoardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt @@ -32,7 +32,8 @@ import org.mockito.Mockito.atLeastOnce import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule -import org.oppia.app.model.OnBoardingFlow +import org.oppia.app.model.OnboardingFlow +import org.oppia.domain.onboarding.OnboardingFlowController import org.oppia.util.data.AsyncResult import org.oppia.util.logging.EnableConsoleLog import org.oppia.util.logging.EnableFileLog @@ -46,10 +47,10 @@ import javax.inject.Qualifier import javax.inject.Singleton import kotlin.coroutines.EmptyCoroutineContext -/** Tests for [OnBoardingFlowController]. */ +/** Tests for [OnboardingFlowController]. */ @RunWith(AndroidJUnit4::class) @Config(manifest = Config.NONE) -class OnBoardingFlowControllerTest { +class OnboardingFlowControllerTest { @Rule @JvmField val mockitoRule: MockitoRule = MockitoJUnit.rule() @@ -58,7 +59,7 @@ class OnBoardingFlowControllerTest { @JvmField val executorRule = InstantTaskExecutorRule() - @Inject lateinit var onBoardingFlowController: OnBoardingFlowController + @Inject lateinit var onboardingFlowController: OnboardingFlowController @Inject @field:TestDispatcher @@ -69,10 +70,10 @@ class OnBoardingFlowControllerTest { } @Mock - lateinit var mockOnBoardingObserver: Observer> + lateinit var mockOnboardingObserver: Observer> @Captor - lateinit var onBoardingResultCaptor: ArgumentCaptor> + lateinit var onboardinggResultCaptor: ArgumentCaptor> // https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-test/ @ObsoleteCoroutinesApi @@ -95,7 +96,7 @@ class OnBoardingFlowControllerTest { } private fun setUpTestApplicationComponent() { - DaggerOnBoardingFlowControllerTest_TestApplicationComponent.builder() + DaggerOnboardingFlowControllerTest_TestApplicationComponent.builder() .setApplication(ApplicationProvider.getApplicationContext()) .build() .inject(this) @@ -103,69 +104,71 @@ class OnBoardingFlowControllerTest { @Test @ExperimentalCoroutinesApi - fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnBoardedTheApp() = runBlockingTest(coroutineContext) { - val onBoarding = onBoardingFlowController.getOnBoardingFlow() - advanceUntilIdle() - onBoarding.observeForever(mockOnBoardingObserver) + fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnBoardedTheApp() = + runBlockingTest(coroutineContext) { + val onboardingg = onboardingFlowController.getOnboardingFlow() + advanceUntilIdle() + onboardingg.observeForever(mockOnboardingObserver) - verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) - assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() - assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() - } + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) + assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + } @Test @ExperimentalCoroutinesApi fun testControllerObserver_observedAfterSettingAppOnBoarded_providesLiveData_userDidNotOnBoardApp() = runBlockingTest(coroutineContext) { - val onBoarding = onBoardingFlowController.getOnBoardingFlow() + val onboardingg = onboardingFlowController.getOnboardingFlow() - onBoarding.observeForever(mockOnBoardingObserver) - onBoardingFlowController.markOnBoardingFlowCompleted() + onboardingg.observeForever(mockOnboardingObserver) + onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() // The result should not indicate that the user on-boarded the app because markUserOnBoardedApp does not notify observers // of the change. - verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) - assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() - assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) + assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() } @Test @ExperimentalCoroutinesApi fun testController_settingAppOnBoarded_observedNewController_userOnBoardedApp() = runBlockingTest(coroutineContext) { - onBoardingFlowController.markOnBoardingFlowCompleted() + onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() // Create the controller by creating another singleton graph and injecting it (simulating the app being recreated). setUpTestApplicationComponent() - val onBoarding = onBoardingFlowController.getOnBoardingFlow() - onBoarding.observeForever(mockOnBoardingObserver) + val onboardingg = onboardingFlowController.getOnboardingFlow() + onboardingg.observeForever(mockOnboardingObserver) advanceUntilIdle() // The app should be considered on-boarded since a new LiveData instance was observed after marking the app as on-boarded. - verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) - assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() - assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isTrue() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) + assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isTrue() } @Test @ExperimentalCoroutinesApi - fun testController_onBoardedApp_cleared_observeNewController_userDidNotOnBoardApp() = runBlockingTest(coroutineContext) { - onBoardingFlowController.markOnBoardingFlowCompleted() - advanceUntilIdle() + fun testController_onBoardedApp_cleared_observeNewController_userDidNotOnBoardApp() = + runBlockingTest(coroutineContext) { + onboardingFlowController.markOnboardingFlowCompleted() + advanceUntilIdle() - // Clear, then recreate another controller. - onBoardingFlowController.clearOnBoardingFlow() - setUpTestApplicationComponent() - val onBoarding = onBoardingFlowController.getOnBoardingFlow() - onBoarding.observeForever(mockOnBoardingObserver) - advanceUntilIdle() + // Clear, then recreate another controller. + onboardingFlowController.clearOnboardingFlow() + setUpTestApplicationComponent() + val onboardingg = onboardingFlowController.getOnboardingFlow() + onboardingg.observeForever(mockOnboardingObserver) + advanceUntilIdle() - // The app should be considered not yet on-boarded since the previous history was cleared. - verify(mockOnBoardingObserver, atLeastOnce()).onChanged(onBoardingResultCaptor.capture()) - assertThat(onBoardingResultCaptor.value.isSuccess()).isTrue() - assertThat(onBoardingResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() - } + // The app should be considered not yet on-boarded since the previous history was cleared. + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) + assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + } @Qualifier annotation class TestDispatcher @@ -228,6 +231,6 @@ class OnBoardingFlowControllerTest { fun build(): TestApplicationComponent } - fun inject(onBoardingFlowControllerTest: OnBoardingFlowControllerTest) + fun inject(onboardinggFlowControllerTest: OnboardingFlowControllerTest) } } diff --git a/model/src/main/proto/example.proto b/model/src/main/proto/example.proto index ae1189b08f2..0c68a3d68b4 100644 --- a/model/src/main/proto/example.proto +++ b/model/src/main/proto/example.proto @@ -9,10 +9,6 @@ message UserAppHistory { bool already_opened_app = 1; } -message OnBoardingFlow { - bool already_on_boarded_app = 1; -} - message TestMessage { int32 version = 1; } diff --git a/model/src/main/proto/onboarding.proto b/model/src/main/proto/onboarding.proto new file mode 100644 index 00000000000..cd41590e87a --- /dev/null +++ b/model/src/main/proto/onboarding.proto @@ -0,0 +1,12 @@ +syntax = "proto3"; + +package model; + +option java_package = "org.oppia.app.model"; +option java_multiple_files = true; + +// Structure for user onboarding flow. +message OnboardingFlow { +// Determines whether user already onboarded + bool already_on_boarded_app = 1; +} From 1ba63ea14afabc39c93f94a0e0a66abf368d7550 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Tue, 11 Feb 2020 14:44:15 +0530 Subject: [PATCH 05/13] changed on-boarded to single word onboarded --- .../org/oppia/domain/onboarding/OnboardingFlowController.kt | 4 ++-- model/src/main/proto/onboarding.proto | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt index 67fce39e0fb..0df0922919c 100644 --- a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt +++ b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt @@ -33,7 +33,7 @@ class OnboardingFlowController @Inject constructor( */ fun markOnboardingFlowCompleted() { onboardingFlowStore.storeDataAsync(updateInMemoryCache = false) { - it.toBuilder().setAlreadyOnBoardedApp(true).build() + it.toBuilder().setAlreadyOnboardedApp(true).build() }.invokeOnCompletion { it?.let { logger.e("DOMAIN", "Failed when storing that the user already on-boarded the app.", it) @@ -45,7 +45,7 @@ class OnboardingFlowController @Inject constructor( fun clearOnboardingFlow() { onboardingFlowStore.clearCacheAsync().invokeOnCompletion { it?.let { - logger.e("DOMAIN", "Failed to clear onBoarding flow.", it) + logger.e("DOMAIN", "Failed to clear onboarding flow.", it) } } } diff --git a/model/src/main/proto/onboarding.proto b/model/src/main/proto/onboarding.proto index cd41590e87a..1b27963da5d 100644 --- a/model/src/main/proto/onboarding.proto +++ b/model/src/main/proto/onboarding.proto @@ -8,5 +8,5 @@ option java_multiple_files = true; // Structure for user onboarding flow. message OnboardingFlow { // Determines whether user already onboarded - bool already_on_boarded_app = 1; + bool already_onboarded_app = 1; } From 58c3f397a63cc62586855db411b6d2213faf09f3 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Tue, 11 Feb 2020 14:59:01 +0530 Subject: [PATCH 06/13] changed on-board to single word onboard in text class method names --- .../domain/OnboardingFlowControllerTest.kt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt index d193357bbfc..3d800338e9d 100644 --- a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt @@ -104,7 +104,7 @@ class OnboardingFlowControllerTest { @Test @ExperimentalCoroutinesApi - fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnBoardedTheApp() = + fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnboardedTheApp() = runBlockingTest(coroutineContext) { val onboardingg = onboardingFlowController.getOnboardingFlow() advanceUntilIdle() @@ -112,12 +112,12 @@ class OnboardingFlowControllerTest { verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } @Test @ExperimentalCoroutinesApi - fun testControllerObserver_observedAfterSettingAppOnBoarded_providesLiveData_userDidNotOnBoardApp() = + fun testControllerObserver_observedAfterSettingAppOnboarded_providesLiveData_userDidNotOnboardApp() = runBlockingTest(coroutineContext) { val onboardingg = onboardingFlowController.getOnboardingFlow() @@ -125,16 +125,16 @@ class OnboardingFlowControllerTest { onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() - // The result should not indicate that the user on-boarded the app because markUserOnBoardedApp does not notify observers + // The result should not indicate that the user on-boarded the app because markUserOnboardedApp does not notify observers // of the change. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } @Test @ExperimentalCoroutinesApi - fun testController_settingAppOnBoarded_observedNewController_userOnBoardedApp() = runBlockingTest(coroutineContext) { + fun testController_settingAppOnboarded_observedNewController_userOnboardedApp() = runBlockingTest(coroutineContext) { onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() @@ -147,12 +147,12 @@ class OnboardingFlowControllerTest { // The app should be considered on-boarded since a new LiveData instance was observed after marking the app as on-boarded. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isTrue() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isTrue() } @Test @ExperimentalCoroutinesApi - fun testController_onBoardedApp_cleared_observeNewController_userDidNotOnBoardApp() = + fun testController_onboardedApp_cleared_observeNewController_userDidNotOnboardApp() = runBlockingTest(coroutineContext) { onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() @@ -167,7 +167,7 @@ class OnboardingFlowControllerTest { // The app should be considered not yet on-boarded since the previous history was cleared. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnBoardedApp).isFalse() + assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } @Qualifier From e4b7854394baea4beeb1f5ed0a99646259aaafd6 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Tue, 11 Feb 2020 17:43:27 +0530 Subject: [PATCH 07/13] changed on-board to single word onboard in comments --- .../domain/onboarding/OnboardingFlowController.kt | 12 ++++++------ .../org/oppia/domain/OnboardingFlowControllerTest.kt | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt index 0df0922919c..3987b222710 100644 --- a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt +++ b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt @@ -9,7 +9,7 @@ import org.oppia.util.logging.Logger import javax.inject.Inject import javax.inject.Singleton -/** Controller for persisting and retrieving the user on-boarding information of the app. */ +/** Controller for persisting and retrieving the user onboarding information of the app. */ @Singleton class OnboardingFlowController @Inject constructor( cacheStoreFactory: PersistentCacheStore.Factory, @@ -22,13 +22,13 @@ class OnboardingFlowController @Inject constructor( // Prime the cache ahead of time so that any existing history is read prior to any calls to markOnboardingFlowCompleted(). onboardingFlowStore.primeCacheAsync().invokeOnCompletion { it?.let { - logger.e("DOMAIN", "Failed to prime cache ahead of LiveData conversion for user on-boarding data.", it) + logger.e("DOMAIN", "Failed to prime cache ahead of LiveData conversion for user onboarding data.", it) } } } /** - * Saves that the user has completed on-boarding the app. Note that this does not notify existing subscribers of the changed state, + * Saves that the user has completed onboarding the app. Note that this does not notify existing subscribers of the changed state, * nor can future subscribers observe this state until app restart. */ fun markOnboardingFlowCompleted() { @@ -36,12 +36,12 @@ class OnboardingFlowController @Inject constructor( it.toBuilder().setAlreadyOnboardedApp(true).build() }.invokeOnCompletion { it?.let { - logger.e("DOMAIN", "Failed when storing that the user already on-boarded the app.", it) + logger.e("DOMAIN", "Failed when storing that the user already onboarded the app.", it) } } } - /** Clears any indication that the user has previously completed on-boarding the application. */ + /** Clears any indication that the user has previously completed onboarding the application. */ fun clearOnboardingFlow() { onboardingFlowStore.clearCacheAsync().invokeOnCompletion { it?.let { @@ -51,7 +51,7 @@ class OnboardingFlowController @Inject constructor( } /** - * Returns a [LiveData] result indicating whether the user has on-boarded the app. This is guaranteed to + * Returns a [LiveData] result indicating whether the user has onboarded the app. This is guaranteed to * provide the state of the store upon the creation of this controller even if [markOnboardingFlowCompleted] has since been * called. */ diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt index 3d800338e9d..7f85148dfe4 100644 --- a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt @@ -125,7 +125,7 @@ class OnboardingFlowControllerTest { onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() - // The result should not indicate that the user on-boarded the app because markUserOnboardedApp does not notify observers + // The result should not indicate that the user onboarded the app because markUserOnboardedApp does not notify observers // of the change. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() @@ -144,7 +144,7 @@ class OnboardingFlowControllerTest { onboardingg.observeForever(mockOnboardingObserver) advanceUntilIdle() - // The app should be considered on-boarded since a new LiveData instance was observed after marking the app as on-boarded. + // The app should be considered onboarded since a new LiveData instance was observed after marking the app as onboarded. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isTrue() @@ -164,7 +164,7 @@ class OnboardingFlowControllerTest { onboardingg.observeForever(mockOnboardingObserver) advanceUntilIdle() - // The app should be considered not yet on-boarded since the previous history was cleared. + // The app should be considered not yet onboarded since the previous history was cleared. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() From 897b00e7bb5826b39412728fa870056c9e250f35 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Wed, 12 Feb 2020 16:16:34 +0530 Subject: [PATCH 08/13] nit --- model/src/main/proto/onboarding.proto | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/model/src/main/proto/onboarding.proto b/model/src/main/proto/onboarding.proto index 1b27963da5d..6b4a62bb6a7 100644 --- a/model/src/main/proto/onboarding.proto +++ b/model/src/main/proto/onboarding.proto @@ -3,10 +3,9 @@ syntax = "proto3"; package model; option java_package = "org.oppia.app.model"; -option java_multiple_files = true; // Structure for user onboarding flow. message OnboardingFlow { -// Determines whether user already onboarded + // Determines whether user already onboarded. bool already_onboarded_app = 1; } From 8ad2f2dc46c144ef9c6bdb65fe6e5614717bbec5 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Fri, 14 Feb 2020 15:27:04 +0530 Subject: [PATCH 09/13] clear cache is added in test class --- .../onboarding/OnboardingFlowController.kt | 9 --------- .../domain/OnboardingFlowControllerTest.kt | 19 +++++++++++++++---- model/src/main/proto/onboarding.proto | 1 + 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt index 3987b222710..a36a11b9bad 100644 --- a/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt +++ b/domain/src/main/java/org/oppia/domain/onboarding/OnboardingFlowController.kt @@ -41,15 +41,6 @@ class OnboardingFlowController @Inject constructor( } } - /** Clears any indication that the user has previously completed onboarding the application. */ - fun clearOnboardingFlow() { - onboardingFlowStore.clearCacheAsync().invokeOnCompletion { - it?.let { - logger.e("DOMAIN", "Failed to clear onboarding flow.", it) - } - } - } - /** * Returns a [LiveData] result indicating whether the user has onboarded the app. This is guaranteed to * provide the state of the store upon the creation of this controller even if [markOnboardingFlowCompleted] has since been diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt index 7f85148dfe4..9979858bc37 100644 --- a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt @@ -3,6 +3,7 @@ package org.oppia.domain import android.app.Application import android.content.Context import androidx.arch.core.executor.testing.InstantTaskExecutorRule +import androidx.lifecycle.LiveData import androidx.lifecycle.Observer import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -11,6 +12,7 @@ import dagger.BindsInstance import dagger.Component import dagger.Module import dagger.Provides +import javolution.util.stripped.FastMap.logger import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -33,8 +35,10 @@ import org.mockito.Mockito.verify import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule import org.oppia.app.model.OnboardingFlow +import org.oppia.data.persistence.PersistentCacheStore import org.oppia.domain.onboarding.OnboardingFlowController import org.oppia.util.data.AsyncResult +import org.oppia.util.data.DataProviders import org.oppia.util.logging.EnableConsoleLog import org.oppia.util.logging.EnableFileLog import org.oppia.util.logging.GlobalLogLevel @@ -60,6 +64,7 @@ class OnboardingFlowControllerTest { val executorRule = InstantTaskExecutorRule() @Inject lateinit var onboardingFlowController: OnboardingFlowController + @Inject lateinit var cacheFactory: PersistentCacheStore.Factory @Inject @field:TestDispatcher @@ -154,22 +159,28 @@ class OnboardingFlowControllerTest { @ExperimentalCoroutinesApi fun testController_onboardedApp_cleared_observeNewController_userDidNotOnboardApp() = runBlockingTest(coroutineContext) { + val onboardingFlowStore = cacheFactory.create("on_boarding_flow", OnboardingFlow.getDefaultInstance()) onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() - - // Clear, then recreate another controller. - onboardingFlowController.clearOnboardingFlow() + clearOnboardingFlow(onboardingFlowStore) setUpTestApplicationComponent() val onboardingg = onboardingFlowController.getOnboardingFlow() onboardingg.observeForever(mockOnboardingObserver) advanceUntilIdle() - // The app should be considered not yet onboarded since the previous history was cleared. verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } + /** Clears any indication that the user has previously completed onboarding the application. */ + private fun clearOnboardingFlow(onboardingFlowStore: PersistentCacheStore) { + onboardingFlowStore.clearCacheAsync().invokeOnCompletion { + it?.let { + } + } + } + @Qualifier annotation class TestDispatcher diff --git a/model/src/main/proto/onboarding.proto b/model/src/main/proto/onboarding.proto index 6b4a62bb6a7..58375523cc8 100644 --- a/model/src/main/proto/onboarding.proto +++ b/model/src/main/proto/onboarding.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package model; option java_package = "org.oppia.app.model"; +option java_multiple_files = true; // Structure for user onboarding flow. message OnboardingFlow { From f381f34a94b2069584588eb5aea4a8430241a542 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Fri, 14 Feb 2020 15:34:54 +0530 Subject: [PATCH 10/13] nit --- .../OnboardingFlowControllerTest.kt | 58 +++++++++---------- 1 file changed, 26 insertions(+), 32 deletions(-) rename domain/src/test/java/org/oppia/domain/{ => onboarding}/OnboardingFlowControllerTest.kt (79%) diff --git a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt similarity index 79% rename from domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt rename to domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt index 9979858bc37..5e37a074b12 100644 --- a/domain/src/test/java/org/oppia/domain/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt @@ -1,9 +1,8 @@ -package org.oppia.domain +package org.oppia.domain.onboarding import android.app.Application import android.content.Context import androidx.arch.core.executor.testing.InstantTaskExecutorRule -import androidx.lifecycle.LiveData import androidx.lifecycle.Observer import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -12,7 +11,6 @@ import dagger.BindsInstance import dagger.Component import dagger.Module import dagger.Provides -import javolution.util.stripped.FastMap.logger import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -36,9 +34,8 @@ import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule import org.oppia.app.model.OnboardingFlow import org.oppia.data.persistence.PersistentCacheStore -import org.oppia.domain.onboarding.OnboardingFlowController +import org.oppia.domain.DaggerOnboardingFlowControllerTest_TestApplicationComponent import org.oppia.util.data.AsyncResult -import org.oppia.util.data.DataProviders import org.oppia.util.logging.EnableConsoleLog import org.oppia.util.logging.EnableFileLog import org.oppia.util.logging.GlobalLogLevel @@ -74,15 +71,12 @@ class OnboardingFlowControllerTest { EmptyCoroutineContext + testDispatcher } - @Mock - lateinit var mockOnboardingObserver: Observer> + @Mock lateinit var mockOnboardingObserver: Observer> - @Captor - lateinit var onboardinggResultCaptor: ArgumentCaptor> + @Captor lateinit var onboardingResultCaptor: ArgumentCaptor> // https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-test/ - @ObsoleteCoroutinesApi - private val testThread = newSingleThreadContext("TestMain") + @ObsoleteCoroutinesApi private val testThread = newSingleThreadContext("TestMain") @Before @ExperimentalCoroutinesApi @@ -111,30 +105,30 @@ class OnboardingFlowControllerTest { @ExperimentalCoroutinesApi fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnboardedTheApp() = runBlockingTest(coroutineContext) { - val onboardingg = onboardingFlowController.getOnboardingFlow() + val onboarding = onboardingFlowController.getOnboardingFlow() advanceUntilIdle() - onboardingg.observeForever(mockOnboardingObserver) + onboarding.observeForever(mockOnboardingObserver) - verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) - assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardingResultCaptor.capture()) + assertThat(onboardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } @Test @ExperimentalCoroutinesApi fun testControllerObserver_observedAfterSettingAppOnboarded_providesLiveData_userDidNotOnboardApp() = runBlockingTest(coroutineContext) { - val onboardingg = onboardingFlowController.getOnboardingFlow() + val onboarding = onboardingFlowController.getOnboardingFlow() - onboardingg.observeForever(mockOnboardingObserver) + onboarding.observeForever(mockOnboardingObserver) onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() // The result should not indicate that the user onboarded the app because markUserOnboardedApp does not notify observers // of the change. - verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) - assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardingResultCaptor.capture()) + assertThat(onboardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } @Test @@ -145,14 +139,14 @@ class OnboardingFlowControllerTest { // Create the controller by creating another singleton graph and injecting it (simulating the app being recreated). setUpTestApplicationComponent() - val onboardingg = onboardingFlowController.getOnboardingFlow() - onboardingg.observeForever(mockOnboardingObserver) + val onboarding = onboardingFlowController.getOnboardingFlow() + onboarding.observeForever(mockOnboardingObserver) advanceUntilIdle() // The app should be considered onboarded since a new LiveData instance was observed after marking the app as onboarded. - verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) - assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isTrue() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardingResultCaptor.capture()) + assertThat(onboardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isTrue() } @Test @@ -164,13 +158,13 @@ class OnboardingFlowControllerTest { advanceUntilIdle() clearOnboardingFlow(onboardingFlowStore) setUpTestApplicationComponent() - val onboardingg = onboardingFlowController.getOnboardingFlow() - onboardingg.observeForever(mockOnboardingObserver) + val onboarding = onboardingFlowController.getOnboardingFlow() + onboarding.observeForever(mockOnboardingObserver) advanceUntilIdle() // The app should be considered not yet onboarded since the previous history was cleared. - verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardinggResultCaptor.capture()) - assertThat(onboardinggResultCaptor.value.isSuccess()).isTrue() - assertThat(onboardinggResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() + verify(mockOnboardingObserver, atLeastOnce()).onChanged(onboardingResultCaptor.capture()) + assertThat(onboardingResultCaptor.value.isSuccess()).isTrue() + assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } /** Clears any indication that the user has previously completed onboarding the application. */ @@ -242,6 +236,6 @@ class OnboardingFlowControllerTest { fun build(): TestApplicationComponent } - fun inject(onboardinggFlowControllerTest: OnboardingFlowControllerTest) + fun inject(onboardingFlowControllerTest: OnboardingFlowControllerTest) } } From b6d3b5f3b34fbb576b900c629a8ef7100b5d3cc8 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Fri, 14 Feb 2020 15:41:24 +0530 Subject: [PATCH 11/13] nit --- .../onboarding/OnboardingFlowControllerTest.kt | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt index 5e37a074b12..2c13af55f82 100644 --- a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt @@ -60,8 +60,10 @@ class OnboardingFlowControllerTest { @JvmField val executorRule = InstantTaskExecutorRule() - @Inject lateinit var onboardingFlowController: OnboardingFlowController - @Inject lateinit var cacheFactory: PersistentCacheStore.Factory + @Inject + lateinit var onboardingFlowController: OnboardingFlowController + @Inject + lateinit var cacheFactory: PersistentCacheStore.Factory @Inject @field:TestDispatcher @@ -156,7 +158,8 @@ class OnboardingFlowControllerTest { val onboardingFlowStore = cacheFactory.create("on_boarding_flow", OnboardingFlow.getDefaultInstance()) onboardingFlowController.markOnboardingFlowCompleted() advanceUntilIdle() - clearOnboardingFlow(onboardingFlowStore) + // Clear, then recreate another controller. + onboardingFlowStore.clearCacheAsync() setUpTestApplicationComponent() val onboarding = onboardingFlowController.getOnboardingFlow() onboarding.observeForever(mockOnboardingObserver) @@ -167,14 +170,6 @@ class OnboardingFlowControllerTest { assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } - /** Clears any indication that the user has previously completed onboarding the application. */ - private fun clearOnboardingFlow(onboardingFlowStore: PersistentCacheStore) { - onboardingFlowStore.clearCacheAsync().invokeOnCompletion { - it?.let { - } - } - } - @Qualifier annotation class TestDispatcher From 6ab2039c67817788f09451fd0880722729ea53a9 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Mon, 17 Feb 2020 11:11:49 +0530 Subject: [PATCH 12/13] nit --- .../oppia/domain/onboarding/OnboardingFlowControllerTest.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt index 2c13af55f82..5553630d73c 100644 --- a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt @@ -105,7 +105,7 @@ class OnboardingFlowControllerTest { @Test @ExperimentalCoroutinesApi - fun testController_providesInitialLiveData_thatIndicatesUserHasNotOnboardedTheApp() = + fun testController_providesInitialLiveData_indicatesUserHasNotOnboardedTheApp() = runBlockingTest(coroutineContext) { val onboarding = onboardingFlowController.getOnboardingFlow() advanceUntilIdle() @@ -170,8 +170,7 @@ class OnboardingFlowControllerTest { assertThat(onboardingResultCaptor.value.getOrThrow().alreadyOnboardedApp).isFalse() } - @Qualifier - annotation class TestDispatcher + @Qualifier annotation class TestDispatcher // TODO(#89): Move this to a common test application component. @Module From 6962a53203592f79d2f0b197a869d9bd7831c535 Mon Sep 17 00:00:00 2001 From: marysoloman Date: Mon, 17 Feb 2020 11:34:28 +0530 Subject: [PATCH 13/13] nit --- .../org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt index 5553630d73c..179efde8ce4 100644 --- a/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt +++ b/domain/src/test/java/org/oppia/domain/onboarding/OnboardingFlowControllerTest.kt @@ -34,7 +34,6 @@ import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoRule import org.oppia.app.model.OnboardingFlow import org.oppia.data.persistence.PersistentCacheStore -import org.oppia.domain.DaggerOnboardingFlowControllerTest_TestApplicationComponent import org.oppia.util.data.AsyncResult import org.oppia.util.logging.EnableConsoleLog import org.oppia.util.logging.EnableFileLog