Skip to content

Commit 96bb97e

Browse files
CDRussellmarcosholgado
CDRussell
and
marcosholgado
authored
fix flakey tests (#1161)
* Fix flakey tests for data clearing duration * Fix UriUtils tests to ensure it can run on all OS versions we support * Fix flaky test * Fix flakey test, the return of the flakey (II) * Change dispatcher to `Default` to match previous behavior Co-authored-by: Marcos Holgado <[email protected]>
1 parent fe354b8 commit 96bb97e

File tree

5 files changed

+53
-30
lines changed

5 files changed

+53
-30
lines changed

‎app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import com.duckduckgo.app.statistics.store.OfflinePixelCountDataStore
3737
import com.nhaarman.mockitokotlin2.*
3838
import kotlinx.coroutines.ExperimentalCoroutinesApi
3939
import kotlinx.coroutines.GlobalScope
40+
import kotlinx.coroutines.runBlocking
4041
import org.junit.Before
4142
import org.junit.Rule
4243
import org.junit.Test
@@ -81,7 +82,8 @@ class BrowserWebViewClientTest {
8182
dosDetector,
8283
globalPrivacyControl,
8384
thirdPartyCookieManager,
84-
GlobalScope
85+
GlobalScope,
86+
coroutinesTestRule.testDispatcherProvider
8587
)
8688
testee.webViewClientListener = listener
8789
}

‎app/src/androidTest/java/com/duckduckgo/app/browser/downloader/UriUtilsFilenameExtractorTest.kt‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.duckduckgo.app.browser.downloader
1818

19+
import androidx.test.filters.SdkSuppress
1920
import com.duckduckgo.app.pixels.AppPixelName
2021
import com.duckduckgo.app.statistics.pixels.Pixel
2122
import com.nhaarman.mockitokotlin2.mock
@@ -68,7 +69,7 @@ class UriUtilsFilenameExtractorTest {
6869
fun whenUrlContainsFilenameButContentDispositionSaysOtherwiseThenExtractFromContentDisposition() {
6970
val url = "https://example.com/filename.jpg"
7071
val mimeType: String? = null
71-
val contentDisposition: String? = "Content-Disposition: attachment; filename=fromDisposition.jpg"
72+
val contentDisposition: String = "Content-Disposition: attachment; filename=fromDisposition.jpg"
7273
val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
7374
assertEquals("fromDisposition.jpg", extracted)
7475
}
@@ -119,9 +120,20 @@ class UriUtilsFilenameExtractorTest {
119120
}
120121

121122
@Test
123+
@SdkSuppress(maxSdkVersion = 21)
124+
fun whenUrlIsEmptyStringAndMimeTypeProvidedThenDefaultNameAndFiletypeFromMimeReturnedLollipop() {
125+
val url = ""
126+
val mimeType = "image/jpeg"
127+
val contentDisposition: String? = null
128+
val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
129+
assertEquals("downloadfile.jpeg", extracted)
130+
}
131+
132+
@Test
133+
@SdkSuppress(minSdkVersion = 22)
122134
fun whenUrlIsEmptyStringAndMimeTypeProvidedThenDefaultNameAndFiletypeFromMimeReturned() {
123135
val url = ""
124-
val mimeType: String? = "image/jpeg"
136+
val mimeType = "image/jpeg"
125137
val contentDisposition: String? = null
126138
val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
127139
assertEquals("downloadfile.jpg", extracted)
@@ -131,7 +143,7 @@ class UriUtilsFilenameExtractorTest {
131143
fun whenUrlIsEmptyStringAndContentDispositionProvidedThenExtractFromContentDisposition() {
132144
val url = ""
133145
val mimeType: String? = null
134-
val contentDisposition: String? = "Content-Disposition: attachment; filename=fromDisposition.jpg"
146+
val contentDisposition = "Content-Disposition: attachment; filename=fromDisposition.jpg"
135147
val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
136148
assertEquals("fromDisposition.jpg", extracted)
137149
}
@@ -159,7 +171,7 @@ class UriUtilsFilenameExtractorTest {
159171
val url = "http://example.com/cat/600/400"
160172
val mimeType: String? = null
161173
val contentDisposition: String? = null
162-
val extracted = testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
174+
testee.extract(buildPendingDownload(url, contentDisposition, mimeType))
163175
verify(mockedPixel).fire(AppPixelName.DOWNLOAD_FILE_DEFAULT_GUESSED_NAME)
164176
}
165177

‎app/src/androidTest/java/com/duckduckgo/app/fire/DataClearerTimeKeeperTest.kt‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ import org.junit.runner.RunWith
2525
import org.junit.runners.Parameterized
2626
import org.junit.runners.Parameterized.Parameters
2727
import java.util.*
28-
import java.util.concurrent.TimeUnit
28+
import java.util.concurrent.TimeUnit.MINUTES
29+
import java.util.concurrent.TimeUnit.SECONDS
2930

3031
@RunWith(Parameterized::class)
3132
class DataClearerTimeKeeperTest(private val testCase: TestCase) {
@@ -38,44 +39,49 @@ class DataClearerTimeKeeperTest(private val testCase: TestCase) {
3839
@Parameters(name = "Test case: {index} - {0}")
3940
fun testData(): Array<TestCase> {
4041

41-
val timeNow = SystemClock.elapsedRealtime()
42+
fun timeNow(): () -> Long = { SystemClock.elapsedRealtime() }
4243

4344
return arrayOf(
4445
// APP_EXIT_ONLY shouldn't be passed to this method - always expected to return false regardless of other configuration/inputs
45-
TestCase(false, APP_EXIT_ONLY, TimeUnit.MINUTES.toMillis(5), timeNow),
46-
TestCase(false, APP_EXIT_ONLY, TimeUnit.MINUTES.toMillis(0), timeNow),
47-
TestCase(false, APP_EXIT_ONLY, TimeUnit.MINUTES.toMillis(-5), timeNow),
46+
TestCase(false, APP_EXIT_ONLY, MINUTES.toMillis(5), timeNow()),
47+
TestCase(false, APP_EXIT_ONLY, MINUTES.toMillis(0), timeNow()),
48+
TestCase(false, APP_EXIT_ONLY, MINUTES.toMillis(-5), timeNow()),
49+
50+
// will return true when duration is >= 5 secs
51+
TestCase(false, APP_EXIT_OR_5_SECONDS, SECONDS.toMillis(4), timeNow()),
52+
TestCase(true, APP_EXIT_OR_5_SECONDS, SECONDS.toMillis(5), timeNow()),
53+
TestCase(true, APP_EXIT_OR_5_SECONDS, SECONDS.toMillis(6), timeNow()),
4854

4955
// will return true when duration is >= 5 mins
50-
TestCase(false, APP_EXIT_OR_5_MINS, TimeUnit.MINUTES.toMillis(4), timeNow),
51-
TestCase(true, APP_EXIT_OR_5_MINS, TimeUnit.MINUTES.toMillis(5), timeNow),
52-
TestCase(true, APP_EXIT_OR_5_MINS, TimeUnit.MINUTES.toMillis(6), timeNow),
56+
TestCase(false, APP_EXIT_OR_5_MINS, MINUTES.toMillis(4), timeNow()),
57+
TestCase(true, APP_EXIT_OR_5_MINS, MINUTES.toMillis(5), timeNow()),
58+
TestCase(true, APP_EXIT_OR_5_MINS, MINUTES.toMillis(6), timeNow()),
5359

5460
// will return true when duration is >= 15 mins
55-
TestCase(false, APP_EXIT_OR_15_MINS, TimeUnit.MINUTES.toMillis(14), timeNow),
56-
TestCase(true, APP_EXIT_OR_15_MINS, TimeUnit.MINUTES.toMillis(15), timeNow),
57-
TestCase(true, APP_EXIT_OR_15_MINS, TimeUnit.MINUTES.toMillis(16), timeNow),
61+
TestCase(false, APP_EXIT_OR_15_MINS, MINUTES.toMillis(14), timeNow()),
62+
TestCase(true, APP_EXIT_OR_15_MINS, MINUTES.toMillis(15), timeNow()),
63+
TestCase(true, APP_EXIT_OR_15_MINS, MINUTES.toMillis(16), timeNow()),
5864

5965
// will return true when duration is >= 30 mins
60-
TestCase(false, APP_EXIT_OR_30_MINS, TimeUnit.MINUTES.toMillis(29), timeNow),
61-
TestCase(true, APP_EXIT_OR_30_MINS, TimeUnit.MINUTES.toMillis(30), timeNow),
62-
TestCase(true, APP_EXIT_OR_30_MINS, TimeUnit.MINUTES.toMillis(31), timeNow),
66+
TestCase(false, APP_EXIT_OR_30_MINS, MINUTES.toMillis(29), timeNow()),
67+
TestCase(true, APP_EXIT_OR_30_MINS, MINUTES.toMillis(30), timeNow()),
68+
TestCase(true, APP_EXIT_OR_30_MINS, MINUTES.toMillis(31), timeNow()),
6369

6470
// will return true when duration is >= 60 mins
65-
TestCase(false, APP_EXIT_OR_60_MINS, TimeUnit.MINUTES.toMillis(59), timeNow),
66-
TestCase(true, APP_EXIT_OR_60_MINS, TimeUnit.MINUTES.toMillis(60), timeNow),
67-
TestCase(true, APP_EXIT_OR_60_MINS, TimeUnit.MINUTES.toMillis(61), timeNow)
71+
TestCase(false, APP_EXIT_OR_60_MINS, MINUTES.toMillis(59), timeNow()),
72+
TestCase(true, APP_EXIT_OR_60_MINS, MINUTES.toMillis(60), timeNow()),
73+
TestCase(true, APP_EXIT_OR_60_MINS, MINUTES.toMillis(61), timeNow())
6874
)
6975
}
7076
}
7177

7278
@Test
7379
fun enoughTimePassed() {
74-
val timestamp = getPastTimestamp(testCase.durationBackgrounded, testCase.timeNow)
80+
val timestamp = getPastTimestamp(testCase.durationBackgrounded, testCase.timeNow.invoke())
7581
assertEquals(testCase.expected, testee.hasEnoughTimeElapsed(backgroundedTimestamp = timestamp, clearWhenOption = testCase.clearWhenOption))
7682
}
7783

78-
private fun getPastTimestamp(millisPreviously: Long, timeNow: Long = SystemClock.elapsedRealtime()): Long {
84+
private fun getPastTimestamp(millisPreviously: Long, timeNow: Long): Long {
7985
return Calendar.getInstance().also {
8086
it.timeInMillis = timeNow
8187
it.add(Calendar.MILLISECOND, (-millisPreviously).toInt())
@@ -86,6 +92,6 @@ class DataClearerTimeKeeperTest(private val testCase: TestCase) {
8692
val expected: Boolean,
8793
val clearWhenOption: ClearWhenOption,
8894
val durationBackgrounded: Long,
89-
val timeNow: Long
95+
val timeNow: () -> Long
9096
)
9197
}

‎app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import com.duckduckgo.app.browser.logindetection.DOMLoginDetector
3434
import com.duckduckgo.app.browser.logindetection.WebNavigationEvent
3535
import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
3636
import com.duckduckgo.app.browser.navigation.safeCopyBackForwardList
37-
import com.duckduckgo.app.di.AppCoroutineScope
37+
import com.duckduckgo.app.global.DispatcherProvider
3838
import com.duckduckgo.app.global.exception.UncaughtExceptionRepository
3939
import com.duckduckgo.app.global.exception.UncaughtExceptionSource.*
4040
import com.duckduckgo.app.globalprivacycontrol.GlobalPrivacyControl
@@ -56,7 +56,8 @@ class BrowserWebViewClient(
5656
private val dosDetector: DosDetector,
5757
private val globalPrivacyControl: GlobalPrivacyControl,
5858
private val thirdPartyCookieManager: ThirdPartyCookieManager,
59-
@AppCoroutineScope private val appCoroutineScope: CoroutineScope
59+
private val appCoroutineScope: CoroutineScope,
60+
private val dispatcherProvider: DispatcherProvider
6061
) : WebViewClient() {
6162

6263
var webViewClientListener: WebViewClientListener? = null
@@ -146,7 +147,7 @@ class BrowserWebViewClient(
146147
try {
147148
Timber.v("onPageStarted webViewUrl: ${webView.url} URL: $url")
148149
url?.let {
149-
appCoroutineScope.launch {
150+
appCoroutineScope.launch(dispatcherProvider.default()) {
150151
thirdPartyCookieManager.processUriForThirdPartyCookies(webView, url.toUri())
151152
}
152153
}

‎app/src/main/java/com/duckduckgo/app/browser/di/BrowserModule.kt‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ class BrowserModule {
101101
dosDetector: DosDetector,
102102
globalPrivacyControl: GlobalPrivacyControl,
103103
thirdPartyCookieManager: ThirdPartyCookieManager,
104-
@AppCoroutineScope appCoroutineScope: CoroutineScope
104+
@AppCoroutineScope appCoroutineScope: CoroutineScope,
105+
dispatcherProvider: DispatcherProvider
105106
): BrowserWebViewClient {
106107
return BrowserWebViewClient(
107108
webViewHttpAuthStore,
@@ -116,7 +117,8 @@ class BrowserModule {
116117
dosDetector,
117118
globalPrivacyControl,
118119
thirdPartyCookieManager,
119-
appCoroutineScope
120+
appCoroutineScope,
121+
dispatcherProvider
120122
)
121123
}
122124

0 commit comments

Comments
 (0)