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

Move Android test as jvm test: TabDataRepository, and fix flaky test #5316

Merged
merged 2 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 0 additions & 19 deletions app/src/androidTest/java/com/duckduckgo/app/TestExtensions.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,9 @@

package com.duckduckgo.app

import androidx.annotation.UiThread
import androidx.lifecycle.LiveData
import androidx.lifecycle.Observer
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.di.AppComponent
import com.duckduckgo.app.global.DuckDuckGoApplication
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit

// from https://stackoverflow.com/a/44991770/73479
@UiThread
fun <T> LiveData<T>.blockingObserve(): T? {
var value: T? = null
val latch = CountDownLatch(1)
val innerObserver = Observer<T> {
value = it
latch.countDown()
}
observeForever(innerObserver)
latch.await(2, TimeUnit.SECONDS)
return value
}

fun getApp(): DuckDuckGoApplication {
return InstrumentationRegistry.getInstrumentation().targetContext.applicationContext as DuckDuckGoApplication
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.duckduckgo.app.browser.logindetection
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.browser.logindetection.FireproofDialogsEventHandler.Event
import com.duckduckgo.app.browser.logindetection.FireproofDialogsEventHandler.Event.FireproofWebSiteSuccess
import com.duckduckgo.app.fire.fireproofwebsite.data.FireproofWebsiteDao
Expand All @@ -33,6 +32,7 @@ import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.blockingObserve
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Assert.assertNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package com.duckduckgo.app.fire.fireproofwebsite.data
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.browser.favicon.FaviconManager
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.blockingObserve
import dagger.Lazy
import kotlinx.coroutines.test.runTest
import org.junit.After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ package com.duckduckgo.app.location.data
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.browser.favicon.FaviconManager
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.blockingObserve
import dagger.Lazy
import kotlinx.coroutines.test.runTest
import org.junit.After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ package com.duckduckgo.app.privacy.db
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.common.test.blockingObserve
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ package com.duckduckgo.app.privacy.db
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.blockingObserve
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import org.junit.After
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.duckduckgo.common.utils.swap
import com.duckduckgo.di.scopes.AppScope
import dagger.SingleInstanceIn
import java.time.LocalDateTime
import java.time.ZoneOffset
import kotlinx.coroutines.flow.Flow

@Dao
Expand Down Expand Up @@ -170,7 +169,7 @@ abstract class TabsDao {
@Query("update tabs set lastAccessTime=:lastAccessTime where tabId=:tabId")
abstract fun updateTabLastAccess(
tabId: String,
lastAccessTime: LocalDateTime = LocalDateTime.now(ZoneOffset.UTC),
lastAccessTime: LocalDateTime,
)

@Query("update tabs set url=:url, title=:title, viewed=:viewed where tabId=:tabId")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@ import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType
import com.duckduckgo.app.tabs.model.TabSwitcherData.UserState
import com.duckduckgo.app.tabs.store.TabSwitcherDataStore
import com.duckduckgo.common.utils.ConflatedJob
import com.duckduckgo.common.utils.CurrentTimeProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import dagger.SingleInstanceIn
import io.reactivex.Scheduler
import io.reactivex.schedulers.Schedulers
import java.time.LocalDateTime
import java.time.ZoneOffset
import java.util.UUID
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
Expand All @@ -57,6 +56,7 @@ class TabDataRepository @Inject constructor(
private val webViewPreviewPersister: WebViewPreviewPersister,
private val faviconManager: FaviconManager,
private val tabSwitcherDataStore: TabSwitcherDataStore,
private val timeProvider: CurrentTimeProvider,
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
private val dispatchers: DispatcherProvider,
) : TabRepository {
Expand Down Expand Up @@ -199,7 +199,7 @@ class TabDataRepository @Inject constructor(
}

override fun countTabsAccessedWithinRange(accessOlderThan: Long, accessNotMoreThan: Long?): Int {
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = timeProvider.localDateTimeNow()
cmonfortep marked this conversation as resolved.
Show resolved Hide resolved
val start = now.minusDays(accessOlderThan)
val end = accessNotMoreThan?.let { now.minusDays(it).minusSeconds(1) } // subtracted a second to make the end limit inclusive
return tabsDao.tabs().filter {
Expand Down Expand Up @@ -245,7 +245,7 @@ class TabDataRepository @Inject constructor(

override suspend fun updateTabLastAccess(tabId: String) {
databaseExecutor().scheduleDirect {
tabsDao.updateTabLastAccess(tabId)
tabsDao.updateTabLastAccess(tabId, timeProvider.localDateTimeNow())
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018 DuckDuckGo
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,12 +14,14 @@
* limitations under the License.
*/

package com.duckduckgo.app.tabs.db
package com.duckduckgo.tabs.db

import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.room.Room
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.app.tabs.db.TabsDao
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.model.TabSelectionEntity
import kotlinx.coroutines.test.runTest
Expand All @@ -28,7 +30,9 @@ import org.junit.Assert.*
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith

@RunWith(AndroidJUnit4::class)
class TabsDaoTest {

@get:Rule
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018 DuckDuckGo
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,15 +14,13 @@
* limitations under the License.
*/

@file:Suppress("RemoveExplicitTypeArguments")

package com.duckduckgo.app.tabs.model
package com.duckduckgo.tabs.model

import androidx.arch.core.executor.testing.InstantTaskExecutorRule
import androidx.lifecycle.MutableLiveData
import androidx.room.Room
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import com.duckduckgo.app.blockingObserve
import com.duckduckgo.app.browser.DuckDuckGoUrlDetector
import com.duckduckgo.app.browser.certificates.BypassedSSLCertificatesRepository
import com.duckduckgo.app.browser.favicon.FaviconManager
Expand All @@ -31,24 +29,42 @@ import com.duckduckgo.app.global.db.AppDatabase
import com.duckduckgo.app.global.model.SiteFactoryImpl
import com.duckduckgo.app.privacy.db.UserAllowListRepository
import com.duckduckgo.app.tabs.db.TabsDao
import com.duckduckgo.app.tabs.model.TabDataRepository
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.model.TabSelectionEntity
import com.duckduckgo.app.tabs.store.TabSwitcherDataStore
import com.duckduckgo.app.trackerdetection.EntityLookup
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.InstantSchedulersRule
import com.duckduckgo.common.test.blockingObserve
import com.duckduckgo.common.utils.CurrentTimeProvider
import com.duckduckgo.duckplayer.api.DuckPlayer
import com.duckduckgo.privacy.config.api.ContentBlocking
import java.time.Instant
import java.time.LocalDateTime
import java.time.ZoneOffset
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.consumeAsFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Assert.*
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNotSame
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.mockito.kotlin.*

import org.junit.runner.RunWith
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever

@RunWith(AndroidJUnit4::class)
class TabDataRepositoryTest {

@get:Rule
Expand Down Expand Up @@ -206,6 +222,7 @@ class TabDataRepositoryTest {
val existingTabs = listOf(tab0)

whenever(mockDao.tabs()).thenReturn(existingTabs)
whenever(mockDao.lastTab()).thenReturn(existingTabs.last())

testee.add("http://www.example.com")

Expand Down Expand Up @@ -468,7 +485,7 @@ class TabDataRepositoryTest {
@Test
fun getActiveTabCountReturnsCorrectCountWhenTabsYoungerThanSpecifiedDay() = runTest {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@0nko after fixing the flaky test, this one is now failing. Can you evaluate the expected result?

Copy link
Member

@0nko 0nko Nov 26, 2024

Choose a reason for hiding this comment

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

The fake DateTime provider revealed the timing issue, which made it pass even though it shouldn’t, nice! 👍 It’s not this test that’s failing, it’s getInactiveTabCountReturnsCorrectCountWhenAllTabsOlderThanSpecifiedDay() below (see my comment).

// Arrange: No tabs in the repository
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = now()
val tab1 = TabEntity(tabId = "tab1", lastAccessTime = now.minusDays(6))
val tab2 = TabEntity(tabId = "tab2", lastAccessTime = now.minusDays(8))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(10))
Expand Down Expand Up @@ -497,10 +514,10 @@ class TabDataRepositoryTest {
@Test
fun getInactiveTabCountReturnsCorrectCountWhenAllTabsOlderThanSpecifiedDay() = runTest {
// Arrange: Add some tabs with different last access times
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = now()
val tab1 = TabEntity(tabId = "tab1", lastAccessTime = now.minusDays(8))
val tab2 = TabEntity(tabId = "tab2", lastAccessTime = now.minusDays(10))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(9))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(9).minusSeconds(1))
val tab4 = TabEntity(tabId = "tab4")
whenever(mockDao.tabs()).thenReturn(listOf(tab1, tab2, tab3, tab4))
val testee = tabDataRepository()
Expand All @@ -514,7 +531,7 @@ class TabDataRepositoryTest {
@Test
fun getInactiveTabCountReturnsCorrectCountWhenAllTabsInactiveWithinRange() = runTest {
// Arrange: Add some tabs with different last access times
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = now()
val tab1 = TabEntity(tabId = "tab1", lastAccessTime = now.minusDays(8))
val tab2 = TabEntity(tabId = "tab2", lastAccessTime = now.minusDays(10))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(9))
Expand All @@ -531,7 +548,7 @@ class TabDataRepositoryTest {
@Test
fun getInactiveTabCountReturnsZeroWhenNoTabsInactiveWithinRange() = runTest {
// Arrange: Add some tabs with different last access times
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = now()
val tab1 = TabEntity(tabId = "tab1", lastAccessTime = now.minusDays(5))
val tab2 = TabEntity(tabId = "tab2", lastAccessTime = now.minusDays(6))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(13))
Expand All @@ -548,7 +565,7 @@ class TabDataRepositoryTest {
@Test
fun getInactiveTabCountReturnsCorrectCountWhenSomeTabsInactiveWithinRange() = runTest {
// Arrange: Add some tabs with different last access times
val now = LocalDateTime.now(ZoneOffset.UTC)
val now = now()
val tab1 = TabEntity(tabId = "tab1", lastAccessTime = now.minusDays(5))
val tab2 = TabEntity(tabId = "tab2", lastAccessTime = now.minusDays(10))
val tab3 = TabEntity(tabId = "tab3", lastAccessTime = now.minusDays(15))
Expand All @@ -572,6 +589,7 @@ class TabDataRepositoryTest {
faviconManager: FaviconManager = mock(),
tabSwitcherDataStore: TabSwitcherDataStore = mock(),
duckDuckGoUrlDetector: DuckDuckGoUrlDetector = mock(),
timeProvider: CurrentTimeProvider = FakeTimeProvider(),
): TabDataRepository {
return TabDataRepository(
dao,
Expand All @@ -588,6 +606,7 @@ class TabDataRepositoryTest {
webViewPreviewPersister,
faviconManager,
tabSwitcherDataStore,
timeProvider,
coroutinesTestRule.testScope,
coroutinesTestRule.testDispatcherProvider,
)
Expand All @@ -608,7 +627,19 @@ class TabDataRepositoryTest {
return mockDao
}

private fun now(): LocalDateTime {
return FakeTimeProvider().localDateTimeNow()
}

companion object {
const val TAB_ID = "abcd"
}

private class FakeTimeProvider : CurrentTimeProvider {
var currentTime: Instant = Instant.parse("2024-10-16T00:00:00.00Z")

override fun currentTimeMillis(): Long = currentTime.toEpochMilli()
override fun elapsedRealtime(): Long = throw UnsupportedOperationException()
override fun localDateTimeNow(): LocalDateTime = currentTime.atZone(ZoneOffset.UTC).toLocalDateTime()
}
}
1 change: 1 addition & 0 deletions common/common-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ dependencies {
// Dependencies for this Module
implementation project(path: ':common-utils')

implementation AndroidX.lifecycle.liveDataKtx
implementation "io.reactivex.rxjava2:rxjava:_"
implementation "io.reactivex.rxjava2:rxandroid:_"
implementation "com.squareup.moshi:moshi-kotlin:_"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2024 DuckDuckGo
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.duckduckgo.common.test

import androidx.lifecycle.LiveData
import androidx.lifecycle.Observer
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit

// https://stackoverflow.com/a/44991770/73479
fun <T> LiveData<T>.blockingObserve(): T? {
var value: T? = null
val latch = CountDownLatch(1)
val innerObserver = Observer<T> {
value = it
latch.countDown()
}
observeForever(innerObserver)
latch.await(2, TimeUnit.SECONDS)
return value
}
Loading