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

[Code health] Replace mock with actual implementation in DataCollectionFragmentTest #2886

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ interface DraftSubmissionDao : BaseDao<DraftSubmissionEntity> {
suspend fun findById(draftSubmissionId: String): DraftSubmissionEntity?

@Query("DELETE FROM draft_submission") fun delete()

@Query("SELECT COUNT(*) FROM draft_submission") suspend fun countAll(): Int
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import com.google.android.ground.persistence.local.room.fields.MutationEntityTyp
import com.google.android.ground.persistence.local.room.fields.UserDetails
import com.google.android.ground.persistence.local.stores.LocalSubmissionStore
import com.google.android.ground.util.Debug.logOnFailure
import com.google.firebase.crashlytics.FirebaseCrashlytics
import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.collections.immutable.toPersistentList
Expand Down Expand Up @@ -211,9 +210,10 @@ class RoomSubmissionStore @Inject internal constructor() : LocalSubmissionStore
apply(mutation)
enqueue(mutation)
} catch (e: LocalDataStoreException) {
FirebaseCrashlytics.getInstance()
.log("Error enqueueing ${mutation.type} mutation for submission ${mutation.submissionId}")
FirebaseCrashlytics.getInstance().recordException(e)
Timber.e(
e,
"Error enqueueing ${mutation.type} mutation for submission ${mutation.submissionId}",
)
throw e
}
}
Expand Down Expand Up @@ -269,4 +269,6 @@ class RoomSubmissionStore @Inject internal constructor() : LocalSubmissionStore
override suspend fun deleteDraftSubmissions() {
draftSubmissionDao.delete()
}

override suspend fun countDraftSubmissions(): Int = draftSubmissionDao.countAll()
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,7 @@ interface LocalSubmissionStore : LocalMutationStore<SubmissionMutation, Submissi

/** Removes all locally stored draft submissions. */
suspend fun deleteDraftSubmissions()

/** Counts the number of draft submissions in the local database. */
suspend fun countDraftSubmissions(): Int
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ constructor(
suspend fun getDraftSubmission(draftSubmissionId: String, survey: Survey): DraftSubmission? =
localSubmissionStore.getDraftSubmission(draftSubmissionId = draftSubmissionId, survey = survey)

suspend fun countDraftSubmissions() = localSubmissionStore.countDraftSubmissions()

fun getDraftSubmissionsId() = localValueStore.draftSubmissionId ?: ""

suspend fun saveDraftSubmission(
jobId: String,
loiId: String?,
Expand All @@ -103,7 +107,7 @@ constructor(
suspend fun getTotalSubmissionCount(loi: LocationOfInterest) =
loi.submissionCount + getPendingCreateCount(loi.id) - getPendingDeleteCount(loi.id)

private suspend fun getPendingCreateCount(loiId: String) =
suspend fun getPendingCreateCount(loiId: String) =
localSubmissionStore.getPendingCreateCount(loiId)

private suspend fun getPendingDeleteCount(loiId: String) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.model.submission.DraftSubmission
import com.google.android.ground.persistence.local.LocalValueStore
import com.google.android.ground.persistence.sync.MediaUploadWorkManager
import com.google.android.ground.persistence.sync.MutationSyncWorkManager
import com.google.android.ground.repository.MutationRepository
Expand All @@ -42,7 +41,6 @@ import timber.log.Timber
class HomeScreenViewModel
@Inject
internal constructor(
private val localValueStore: LocalValueStore,
private val navigator: Navigator,
private val offlineAreaRepository: OfflineAreaRepository,
private val submissionRepository: SubmissionRepository,
Expand Down Expand Up @@ -80,10 +78,10 @@ internal constructor(

/** Attempts to return draft submission for the currently active survey. */
suspend fun getDraftSubmission(): DraftSubmission? {
val draftId = localValueStore.draftSubmissionId
val draftId = submissionRepository.getDraftSubmissionsId()
val survey = surveyRepository.activeSurvey

if (draftId.isNullOrEmpty() || survey == null) {
if (draftId.isEmpty() || survey == null) {
// Missing draft submission
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package com.google.android.ground.ui.datacollection
import android.os.Bundle
import com.google.android.ground.BaseHiltTest
import com.google.android.ground.R
import com.google.android.ground.capture
import com.google.android.ground.domain.usecases.survey.ActivateSurveyUseCase
import com.google.android.ground.launchFragmentWithNavController
import com.google.android.ground.model.mutation.Mutation
import com.google.android.ground.model.mutation.SubmissionMutation
import com.google.android.ground.model.submission.DraftSubmission
import com.google.android.ground.model.submission.MultipleChoiceTaskData
import com.google.android.ground.model.submission.TextTaskData
import com.google.android.ground.model.submission.ValueDelta
Expand All @@ -31,28 +33,25 @@ import com.google.android.ground.model.task.MultipleChoice
import com.google.android.ground.model.task.Option
import com.google.android.ground.model.task.Task
import com.google.android.ground.persistence.local.room.converter.SubmissionDeltasConverter
import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus
import com.google.android.ground.repository.MutationRepository
import com.google.android.ground.repository.SubmissionRepository
import com.google.android.ground.repository.UserRepository
import com.google.common.truth.Truth.assertThat
import com.sharedtest.FakeData
import com.sharedtest.FakeData.LOCATION_OF_INTEREST
import com.sharedtest.FakeData.LOCATION_OF_INTEREST_NAME
import com.sharedtest.FakeData.USER
import com.sharedtest.persistence.remote.FakeRemoteDataStore
import dagger.hilt.android.testing.BindValue
import dagger.hilt.android.testing.HiltAndroidTest
import java.util.Date
import javax.inject.Inject
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.test.advanceUntilIdle
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.Captor
import org.mockito.Mock
import org.mockito.kotlin.eq
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.robolectric.RobolectricTestRunner
import org.robolectric.shadows.ShadowToast

Expand All @@ -63,16 +62,14 @@ class DataCollectionFragmentTest : BaseHiltTest() {

@Inject lateinit var activateSurvey: ActivateSurveyUseCase
@Inject lateinit var fakeRemoteDataStore: FakeRemoteDataStore
@BindValue @Mock lateinit var submissionRepository: SubmissionRepository
@Captor lateinit var deltaCaptor: ArgumentCaptor<List<ValueDelta>>
@Inject lateinit var mutationRepository: MutationRepository
@Inject lateinit var submissionRepository: SubmissionRepository
@Inject lateinit var userRepository: UserRepository

lateinit var fragment: DataCollectionFragment
@Inject lateinit var uuidGenerator: OfflineUuidGenerator
lateinit var collectionId: String

override fun setUp() = runBlocking {
super.setUp()
collectionId = uuidGenerator.generateUuid()

setupSubmission()
setupFragment()
}
Expand Down Expand Up @@ -123,20 +120,7 @@ class DataCollectionFragmentTest : BaseHiltTest() {
fun `Next click saves draft`() = runWithTestDispatcher {
runner().inputText(TASK_1_RESPONSE).clickNextButton()

// Validate that previous drafts were cleared
verify(submissionRepository, times(1)).deleteDraftSubmission()

// Validate that new draft was created
verify(submissionRepository, times(1))
.saveDraftSubmission(
eq(JOB.id),
eq(LOCATION_OF_INTEREST.id),
eq(SURVEY.id),
capture(deltaCaptor),
eq(LOCATION_OF_INTEREST_NAME),
)

listOf(TASK_1_VALUE_DELTA).forEach { value -> assertThat(deltaCaptor.value).contains(value) }
assertDraftSaved(listOf(TASK_1_VALUE_DELTA))
}

@Test
Expand All @@ -147,20 +131,7 @@ class DataCollectionFragmentTest : BaseHiltTest() {
.selectMultipleChoiceOption(TASK_2_OPTION_LABEL)
.clickPreviousButton()

// Both deletion and creating happens twice as we do it on every previous/next step
verify(submissionRepository, times(2)).deleteDraftSubmission()
verify(submissionRepository, times(2))
.saveDraftSubmission(
eq(JOB.id),
eq(LOCATION_OF_INTEREST.id),
eq(SURVEY.id),
capture(deltaCaptor),
eq(LOCATION_OF_INTEREST_NAME),
)

listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA).forEach { value ->
assertThat(deltaCaptor.value).contains(value)
}
assertDraftSaved(listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA))
}

@Test
Expand Down Expand Up @@ -210,25 +181,19 @@ class DataCollectionFragmentTest : BaseHiltTest() {
.selectMultipleChoiceOption(TASK_2_OPTION_LABEL)
.clickDoneButton() // Click "done" on final task

verify(submissionRepository)
.saveSubmission(
eq(SURVEY.id),
eq(LOCATION_OF_INTEREST.id),
capture(deltaCaptor),
eq(collectionId),
)

listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA).forEach { value ->
assertThat(deltaCaptor.value).contains(value)
}
assertSubmissionSaved(listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA))
}

@Test
fun `Clicking back button on first task clears the draft and returns false`() =
runWithTestDispatcher {
runner().pressBackButton(false)
runner()
.inputText(TASK_1_RESPONSE)
.clickNextButton()
.pressBackButton(true)
.pressBackButton(false)

verify(submissionRepository, times(1)).deleteDraftSubmission()
assertNoDraftSaved()
}

@Test
Expand All @@ -245,17 +210,10 @@ class DataCollectionFragmentTest : BaseHiltTest() {
.inputText(TASK_CONDITIONAL_RESPONSE)
.clickDoneButton()

verify(submissionRepository)
.saveSubmission(
eq(SURVEY.id),
eq(LOCATION_OF_INTEREST.id),
capture(deltaCaptor),
eq(collectionId),
)

// Conditional task data is submitted.
listOf(TASK_1_VALUE_DELTA, TASK_2_CONDITIONAL_VALUE_DELTA, TASK_CONDITIONAL_VALUE_DELTA)
.forEach { value -> assertThat(deltaCaptor.value).contains(value) }
assertSubmissionSaved(
listOf(TASK_1_VALUE_DELTA, TASK_2_CONDITIONAL_VALUE_DELTA, TASK_CONDITIONAL_VALUE_DELTA)
)
}

@Test
Expand All @@ -279,21 +237,68 @@ class DataCollectionFragmentTest : BaseHiltTest() {
.clickDoneButton()
.validateTextIsNotDisplayed(TASK_CONDITIONAL_NAME)

verify(submissionRepository)
.saveSubmission(
eq(SURVEY.id),
eq(LOCATION_OF_INTEREST.id),
capture(deltaCaptor),
eq(collectionId),
)

// Conditional task data is not submitted.
listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA).forEach { value ->
assertThat(deltaCaptor.value).contains(value)
}
assertSubmissionSaved(listOf(TASK_1_VALUE_DELTA, TASK_2_VALUE_DELTA))
}

private suspend fun assertSubmissionSaved(valueDeltas: List<ValueDelta>) {
assertNoDraftSaved()

val loiId = LOCATION_OF_INTEREST.id

// Exactly 1 submission should be saved.
assertThat(submissionRepository.getPendingCreateCount(loiId)).isEqualTo(1)

val testDate = Date()
val mutation =
(mutationRepository.getMutations(loiId, MutationEntitySyncStatus.PENDING)[0]
as SubmissionMutation)
.copy(clientTimestamp = testDate) // seed dummy test date

assertThat(mutation)
.isEqualTo(
SubmissionMutation(
id = 1,
type = Mutation.Type.CREATE,
syncStatus = Mutation.SyncStatus.PENDING,
surveyId = SURVEY.id,
locationOfInterestId = loiId,
userId = USER.id,
clientTimestamp = testDate,
collectionId = "TEST UUID",
job = JOB,
submissionId = "TEST UUID",
deltas = valueDeltas,
)
)
}

private suspend fun assertDraftSaved(valueDeltas: List<ValueDelta>) {
val draftId = submissionRepository.getDraftSubmissionsId()
assertThat(draftId).isNotEmpty()

// Exactly 1 draft should be present always.
assertThat(submissionRepository.countDraftSubmissions()).isEqualTo(1)
assertThat(submissionRepository.getDraftSubmission(draftId, SURVEY))
.isEqualTo(
DraftSubmission(
id = draftId,
jobId = JOB.id,
loiId = LOCATION_OF_INTEREST.id,
loiName = LOCATION_OF_INTEREST_NAME,
surveyId = SURVEY.id,
deltas = valueDeltas,
)
)
}

private suspend fun assertNoDraftSaved() {
assertThat(submissionRepository.getDraftSubmissionsId()).isEmpty()
assertThat(submissionRepository.countDraftSubmissions()).isEqualTo(0)
}

private fun setupSubmission() = runWithTestDispatcher {
userRepository.saveUserDetails(USER)
fakeRemoteDataStore.surveys = listOf(SURVEY)
fakeRemoteDataStore.predefinedLois = listOf(LOCATION_OF_INTEREST)
activateSurvey(SURVEY.id)
Expand Down
Loading