CI test flakiness collection #11091
Labels
CI
Dev
Development, testing & CI
Good First Issue!
Help Wanted
Requesting Pull Requests from volunteers
Stale
Comments
|
Here is one I just saw: https://github.com/ankidroid/Anki-Android/runs/6157540326?check_suite_focus=true#step:9:275 Completed successfully after re-run Seen again once 20220430 or thereabouts https://github.com/ankidroid/Anki-Android/runs/6227166753?check_suite_focus=true#step:9:274 on #11162 |
mikehardy
changed the title
|
This just popped up on unit tests, first time I've seen it. Logging for frequency analysis over time https://github.com/ankidroid/Anki-Android/runs/6372125564?check_suite_focus=true#step:7:143 From #11173 |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
These are failing for me semi-frequently locally as part of unit test on ubuntu |
|
The cache restore step has failed on macOS once recently and windows twice recently. I'm not sure 5mins is enough for that step. |
This comment was marked as outdated.
This comment was marked as outdated.
mikehardy
added
Help Wanted
|
windows unit test gradle cache restore timeout hit at 5 minutes, perhaps slacken. Tracking failure rate here. |
This comment was marked as outdated.
This comment was marked as outdated.
This was referenced Jun 12, 2022
|
Some new ones, these are odd Saw this one again 20220616 |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
7 tasks
7 tasks
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
mikehardy
added a commit
to mikehardy/Anki-Android
that referenced
this issue
Aug 20, 2022
5 tasks
mikehardy
added a commit
that referenced
this issue
Aug 20, 2022
5 tasks
|
Only seen locally (macOS): |
|
Can't reproduce, but does this help? Changed the test to exercise the non-null case, and removed the mutex in the process, as withCol should enforce single execution. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
index 7b0dab6df..c783889f5 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
@@ -2206,7 +2206,12 @@ open class DeckPicker :
context.renderPage()
// Update the mini statistics bar as well
deckPicker?.launchCatchingTask {
- AnkiStatsTaskHandler.createReviewSummaryStatistics(context.col, context.mReviewSummaryTextView)
+ val text = AnkiStatsTaskHandler.createReviewSummaryStatistics(context)
+ context.mReviewSummaryTextView.apply {
+ this.text = text ?: ""
+ visibility = View.VISIBLE
+ invalidate()
+ }
}
Timber.d("Startup - Deck List UI Completed")
}
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/stats/AnkiStatsTaskHandler.kt b/AnkiDroid/src/main/java/com/ichi2/anki/stats/AnkiStatsTaskHandler.kt
index f31d80d47..8cff1dc11 100644
--- a/AnkiDroid/src/main/java/com/ichi2/anki/stats/AnkiStatsTaskHandler.kt
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/stats/AnkiStatsTaskHandler.kt
@@ -16,10 +16,13 @@
package com.ichi2.anki.stats
import android.R
+import android.content.Context
import android.view.View
import android.webkit.WebView
import android.widget.ProgressBar
import android.widget.TextView
+import com.ichi2.anki.CollectionManager.withOpenColOrNull
import com.ichi2.libanki.Collection
import com.ichi2.libanki.DeckId
import com.ichi2.libanki.stats.Stats
@@ -129,43 +132,32 @@ class AnkiStatsTaskHandler private constructor(
@JvmStatic
suspend fun createReviewSummaryStatistics(
- col: Collection,
- view: TextView,
- mainDispatcher: CoroutineDispatcher = Dispatchers.Main,
- defaultDispatcher: CoroutineDispatcher = Dispatchers.IO
- ): Unit = mutex.withLock {
- withContext(defaultDispatcher) {
- val todayStatString = if (!isActive || col.dbClosed) {
- Timber.d("Quitting DeckPreviewStatistics before execution")
- null
- } else {
- Timber.d("Starting DeckPreviewStatistics")
- // eventually put this in Stats (in desktop it is not though)
- var cards: Int
- var minutes: Int
- /* cards, excludes rescheduled cards https://github.com/ankidroid/Anki-Android/issues/8592 */
- val query = "select sum(case when ease > 0 then 1 else 0 end), " +
- "sum(time)/1000 from revlog where id > " + (col.sched.dayCutoff - Stats.SECONDS_PER_DAY) * 1000
- Timber.d("DeckPreviewStatistics query: %s", query)
- col.db
- .query(query).use { cur ->
- cur.moveToFirst()
- cards = cur.getInt(0)
- minutes = (cur.getInt(1) / 60.0).roundToInt()
- }
- val res = view.resources
- val span = res.getQuantityString(com.ichi2.anki.R.plurals.in_minutes, minutes, minutes)
- res.getQuantityString(com.ichi2.anki.R.plurals.studied_cards_today, cards, cards, span)
- }
- todayStatString?.let {
- withContext(mainDispatcher) {
- view.apply {
- text = it
- visibility = View.VISIBLE
- invalidate()
- }
+ context: Context,
+ ): String? {
+ return withOpenColOrNull {
+ Timber.d("Starting DeckPreviewStatistics")
+ // eventually put this in Stats (in desktop it is not though)
+ var cards: Int
+ var minutes: Int
+ /* cards, excludes rescheduled cards https://github.com/ankidroid/Anki-Android/issues/8592 */
+ val query = "select sum(case when ease > 0 then 1 else 0 end), " +
+ "sum(time)/1000 from revlog where id > " + (sched.dayCutoff - Stats.SECONDS_PER_DAY) * 1000
+ Timber.d("DeckPreviewStatistics query: %s", query)
+ db
+ .query(query).use { cur ->
+ cur.moveToFirst()
+ cards = cur.getInt(0)
+ minutes = (cur.getInt(1) / 60.0).roundToInt()
}
- }
+ val res = context.resources
+ val span =
+ res.getQuantityString(com.ichi2.anki.R.plurals.in_minutes, minutes, minutes)
+ res.getQuantityString(
+ com.ichi2.anki.R.plurals.studied_cards_today,
+ cards,
+ cards,
+ span
+ )
}
}
}
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/stats/AnkiStatsTaskHandlerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/stats/AnkiStatsTaskHandlerTest.kt
index be542c2d1..98442b49d 100644
--- a/AnkiDroid/src/test/java/com/ichi2/anki/stats/AnkiStatsTaskHandlerTest.kt
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/stats/AnkiStatsTaskHandlerTest.kt
@@ -15,8 +15,11 @@
****************************************************************************************/
package com.ichi2.anki.stats
+import android.content.Intent
import android.widget.TextView
+import androidx.test.core.app.ActivityScenario
import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.ichi2.anki.DeckPicker
import com.ichi2.anki.RobolectricTest
import com.ichi2.anki.stats.AnkiStatsTaskHandler.Companion.createReviewSummaryStatistics
import com.ichi2.annotations.NeedsTest
@@ -33,34 +36,18 @@ import org.mockito.Mockito.*
import org.mockito.MockitoAnnotations
import org.mockito.kotlin.whenever
import java.util.concurrent.ExecutionException
+import kotlin.coroutines.resume
+import kotlin.coroutines.suspendCoroutine
+import kotlin.test.assertEquals
@OptIn(ExperimentalCoroutinesApi::class)
@RunWith(AndroidJUnit4::class)
class AnkiStatsTaskHandlerTest : RobolectricTest() {
- @Mock
- private lateinit var mCol: Collection
-
- @Mock
- private lateinit var mView: TextView
-
- private val testDispatcher = StandardTestDispatcher()
-
- @Before
- override fun setUp() {
- super.setUp()
- MockitoAnnotations.openMocks(this)
- whenever(mCol.db).thenReturn(null)
- whenever(mCol.dbClosed).thenReturn(true)
- }
-
@Test
- @Throws(ExecutionException::class, InterruptedException::class)
- @NeedsTest("explain this test")
- fun testCreateReviewSummaryStatistics() = runTest(testDispatcher) {
- verify(mCol, atMost(0))!!.db
- createReviewSummaryStatistics(mCol, mView, testDispatcher, testDispatcher)
- advanceUntilIdle()
- verify(mCol, atLeast(0))!!.db
- verify(mCol, atLeast(1))!!.dbClosed
+ fun testCreateReviewSummaryStatistics() = runTest {
+ val deckPicker = startActivityNormallyOpenCollectionWithIntent(
+ DeckPicker::class.java, Intent()
+ )
+ assertEquals(createReviewSummaryStatistics(deckPicker), "Studied 0 cards in 0 minutes today")
}
} |
|
It's only occasionally flaky. The patch provided fails with: |
|
Whoops, I changed withCol to withOpenColOrNull at the last minute, so the assert should now check for null as well. Looks like I got the arg order on the assert wrong too. |
5 tasks
|
Some sort of windows flake in CI, using legacy schema |
|
Is there a way to get the logcat output from the failing test? |
|
Hmm - for the emulator test it's available as an artifact uploaded to the workflow which you may access by clicking on the summary, you can see an example here https://github.com/ankidroid/Anki-Android/actions/runs/3048002169 For the unit tests though (and the windows run is unit test only), not sure... I made some provision for android logs to flow through here When I run tests in Android Studio so it never occurred to me, but now that you mention it I've never seen them in my command-line runs, nor in the github actions logs Looks like one of the configurables needs to be changed? http://robolectric.org/configuring/ Then something the output probably needs to be piped to a file somehow, as it will otherwise overwhelm the github actions output limits and make that window useless. Then upload the file similar to the androidTest strategy - Anki-Android/.github/workflows/tests_emulator.yml Lines 150 to 160 in db59ccb all looks a little fiddly to discover the exact way to get logcat flowing through to stdout or stderr, selecting which of those is best, and redirecting that to a file. Alternatively instead of binding the ShadowLog stream to System.out, maybe detecting an env var that's a desired filename and sending it there directly 🤔 |
|
Looks like something racy that windows (just happens to be our slowest CI unit) appears to expose: |
|
windows unit test just kicked out the one above ☝️ again plus this one: |
|
In my latest PR CardBrowserTest > checkDisplayOrderPersistence FAILED happened on both Ubuntu and Windows; retrying now. |
|
Failed again. Will push a probable fix. |
|
Strange one, only turning up on macOS runner unit test workflow (which gives me a hunch it is timing-related, as that runner is bare metal so should be faster, maybe exposing unexpected race conditions) This one was of interest to me because that's my library, and I honestly cannot see how the executor there can ever be null. UsageAnalytics must have been initialized or the analytics system itself would have null pointered earlier, so initialize must have been called, and in there the GoogleAnalyticsBuilder is used with a final When you ask for an analytics "hit" of some time (in this case an Yet...it is - could be related to this #12317 ( |
|
Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reproduction Steps
Expected Result
Tests should succeed every single time unless there is a legitimate test failure
Actual Result
They flake sometimes.
The situation has been improved recently with #11032 but this issue will collect experiences after that merge to tack it down completely
The text was updated successfully, but these errors were encountered: