From 9ac567cddd57a124187bf80539b1df054a0907a3 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 02:37:05 +0100 Subject: [PATCH 1/6] refactor(navigation): move deck options methods to :anki-common 097c7cdc6c` moved `CollectionManager` down, so these methods can now be moved to `:anki-common`. More work for the for `:widgets` multimodule migration Part of 20737 Assisted-by: Claude Fable 5 --- .../java/com/ichi2/anki/CoroutineHelpers.kt | 2 +- .../anki/pages/DeckOptionsDestination.kt | 19 ------------------- .../widget/cardanalysis/CardAnalysisWidget.kt | 2 +- .../widget/deckpicker/DeckPickerWidget.kt | 2 +- .../destinations/DeckOptionsDestination.kt | 18 ++++++++++++++++++ 5 files changed, 21 insertions(+), 22 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index b9de1b56aeb4..54c009621b37 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -31,12 +31,12 @@ import com.ichi2.anki.common.annotations.UseContextParameter import com.ichi2.anki.common.coroutines.applicationScope import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination +import com.ichi2.anki.common.destinations.fromCurrentDeck import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType import com.ichi2.anki.exception.StorageAccessException import com.ichi2.anki.exception.StorageNotConfiguredException import com.ichi2.anki.libanki.exception.InvalidSearchException -import com.ichi2.anki.pages.fromCurrentDeck import com.ichi2.anki.pages.toIntent import com.ichi2.anki.snackbar.showSnackbar import com.ichi2.anki.startup.redirectToMainEntryPoint diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptionsDestination.kt b/AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptionsDestination.kt index 25dc721294fa..71260b786b57 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptionsDestination.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/pages/DeckOptionsDestination.kt @@ -4,11 +4,8 @@ package com.ichi2.anki.pages import android.content.Context import android.content.Intent -import androidx.annotation.CheckResult -import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.filtered.FilteredDeckOptionsFragment -import com.ichi2.anki.libanki.DeckId /** Builds the [Intent] that opens the deck options screen for this destination. */ fun DeckOptionsDestination.toIntent(context: Context): Intent = @@ -17,19 +14,3 @@ fun DeckOptionsDestination.toIntent(context: Context): Intent = } else { DeckOptions.getIntent(context, deckId) } - -suspend fun DeckOptionsDestination.Companion.fromDeckId(deckId: DeckId): DeckOptionsDestination = - DeckOptionsDestination( - deckId = deckId, - isFiltered = withCol { decks.isFiltered(deckId) }, - ) - -@CheckResult -suspend fun DeckOptionsDestination.Companion.fromCurrentDeck(): DeckOptionsDestination = - withCol { - val deckId = decks.getCurrentId() - DeckOptionsDestination( - deckId = deckId, - isFiltered = decks.isFiltered(deckId), - ) - } diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt index 27da73b38062..858305152dcc 100644 --- a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt +++ b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt @@ -17,11 +17,11 @@ import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.common.destinations.DeferredNavigation import com.ichi2.anki.common.destinations.ReviewDeckDestination +import com.ichi2.anki.common.destinations.fromDeckId import com.ichi2.anki.common.destinations.toIntent import com.ichi2.anki.isCollectionEmpty import com.ichi2.anki.libanki.DeckId import com.ichi2.anki.libanki.Decks.Companion.NOT_FOUND_DECK_ID -import com.ichi2.anki.pages.fromDeckId import com.ichi2.widget.ACTION_UPDATE_WIDGET import com.ichi2.widget.AnalyticsWidgetProvider import com.ichi2.widget.AppWidgetId diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt b/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt index a3d8bb8709cd..ae4c1056b124 100644 --- a/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt +++ b/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt @@ -18,10 +18,10 @@ import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.common.destinations.DeferredNavigation import com.ichi2.anki.common.destinations.ReviewDeckDestination +import com.ichi2.anki.common.destinations.fromDeckId import com.ichi2.anki.common.destinations.toIntent import com.ichi2.anki.isCollectionEmpty import com.ichi2.anki.libanki.DeckId -import com.ichi2.anki.pages.fromDeckId import com.ichi2.widget.ACTION_UPDATE_WIDGET import com.ichi2.widget.AnalyticsWidgetProvider import com.ichi2.widget.AppWidgetId diff --git a/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt b/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt index bc0327a8a858..89bdc9fb28fc 100644 --- a/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt +++ b/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt @@ -2,6 +2,8 @@ package com.ichi2.anki.common.destinations +import androidx.annotation.CheckResult +import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.libanki.DeckId /** @@ -26,3 +28,19 @@ data class DeckOptionsEntry( val name: String?, val isFiltered: Boolean, ) + +suspend fun DeckOptionsDestination.Companion.fromDeckId(deckId: DeckId): DeckOptionsDestination = + DeckOptionsDestination( + deckId = deckId, + isFiltered = withCol { decks.isFiltered(deckId) }, + ) + +@CheckResult +suspend fun DeckOptionsDestination.Companion.fromCurrentDeck(): DeckOptionsDestination = + withCol { + val deckId = decks.getCurrentId() + DeckOptionsDestination( + deckId = deckId, + isFiltered = decks.isFiltered(deckId), + ) + } From a548da1b8771dc74b3d4e7772f6cf100ea0c823d Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 02:37:50 +0100 Subject: [PATCH 2/6] refactor(navigation): inline extension methods Part of 20737 Assisted-by: Claude Fable 5 --- .../java/com/ichi2/anki/CoroutineHelpers.kt | 1 - .../widget/cardanalysis/CardAnalysisWidget.kt | 1 - .../widget/deckpicker/DeckPickerWidget.kt | 1 - .../destinations/DeckOptionsDestination.kt | 34 +++++++++---------- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index 54c009621b37..2d727f075457 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -31,7 +31,6 @@ import com.ichi2.anki.common.annotations.UseContextParameter import com.ichi2.anki.common.coroutines.applicationScope import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination -import com.ichi2.anki.common.destinations.fromCurrentDeck import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType import com.ichi2.anki.exception.StorageAccessException diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt index 858305152dcc..3997e058f87f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt +++ b/AnkiDroid/src/main/java/com/ichi2/widget/cardanalysis/CardAnalysisWidget.kt @@ -17,7 +17,6 @@ import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.common.destinations.DeferredNavigation import com.ichi2.anki.common.destinations.ReviewDeckDestination -import com.ichi2.anki.common.destinations.fromDeckId import com.ichi2.anki.common.destinations.toIntent import com.ichi2.anki.isCollectionEmpty import com.ichi2.anki.libanki.DeckId diff --git a/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt b/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt index ae4c1056b124..ece3b92dcfd7 100644 --- a/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt +++ b/AnkiDroid/src/main/java/com/ichi2/widget/deckpicker/DeckPickerWidget.kt @@ -18,7 +18,6 @@ import com.ichi2.anki.common.crashreporting.CrashReportService import com.ichi2.anki.common.destinations.DeckOptionsDestination import com.ichi2.anki.common.destinations.DeferredNavigation import com.ichi2.anki.common.destinations.ReviewDeckDestination -import com.ichi2.anki.common.destinations.fromDeckId import com.ichi2.anki.common.destinations.toIntent import com.ichi2.anki.isCollectionEmpty import com.ichi2.anki.libanki.DeckId diff --git a/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt b/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt index 89bdc9fb28fc..8fad0b3873da 100644 --- a/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt +++ b/anki-common/src/main/kotlin/com/ichi2/anki/common/destinations/DeckOptionsDestination.kt @@ -16,7 +16,23 @@ data class DeckOptionsDestination( val isFiltered: Boolean, val options: List = emptyList(), ) : Destination() { - companion object + companion object { + suspend fun fromDeckId(deckId: DeckId): DeckOptionsDestination = + DeckOptionsDestination( + deckId = deckId, + isFiltered = withCol { decks.isFiltered(deckId) }, + ) + + @CheckResult + suspend fun fromCurrentDeck(): DeckOptionsDestination = + withCol { + val deckId = decks.getCurrentId() + DeckOptionsDestination( + deckId = deckId, + isFiltered = decks.isFiltered(deckId), + ) + } + } } /** @@ -28,19 +44,3 @@ data class DeckOptionsEntry( val name: String?, val isFiltered: Boolean, ) - -suspend fun DeckOptionsDestination.Companion.fromDeckId(deckId: DeckId): DeckOptionsDestination = - DeckOptionsDestination( - deckId = deckId, - isFiltered = withCol { decks.isFiltered(deckId) }, - ) - -@CheckResult -suspend fun DeckOptionsDestination.Companion.fromCurrentDeck(): DeckOptionsDestination = - withCol { - val deckId = decks.getCurrentId() - DeckOptionsDestination( - deckId = deckId, - isFiltered = decks.isFiltered(deckId), - ) - } From 977ca24f546561f506555e16cf76cd5fe03f1b0e Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 10:55:15 +0100 Subject: [PATCH 3/6] build(anki-common): add unit test & testFixtures support Part of 20737 Assisted-by: Claude Fable 5 --- anki-common/build.gradle.kts | 10 ++++++++++ build.gradle.kts | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/anki-common/build.gradle.kts b/anki-common/build.gradle.kts index 55f596f3cd0d..7a1a4121e069 100644 --- a/anki-common/build.gradle.kts +++ b/anki-common/build.gradle.kts @@ -37,9 +37,19 @@ dependencies { implementation(libs.jakewharton.timber) implementation(libs.kotlinx.coroutines.core) + testImplementation(testFixtures(project(":anki-common"))) + testImplementation(libs.hamcrest) + testImplementation(libs.junit.jupiter) + testImplementation(libs.junit.platform.launcher) + testImplementation(libs.junit.vintage.engine) + testImplementation(libs.kotlin.reflect) + testImplementation(libs.kotlinx.coroutines.test) + testFixturesImplementation(project(":common:android")) testFixturesImplementation(libs.androidx.core.ktx) testFixturesImplementation(libs.androidx.test.core) + testFixturesImplementation(libs.jakewharton.timber) + testFixturesImplementation(libs.kotlin.test) // The Kotlin Compose Compiler plugin attaches to every Kotlin compilation in the // module including testFixtures, which has no @Composable code and refuses to // run unless the Compose Runtime is on the classpath. compileOnly satisfies the diff --git a/build.gradle.kts b/build.gradle.kts index a9fdc9dc975f..d54bbf278357 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -138,7 +138,7 @@ subprojects { compilerArgs += "-XXLanguage:+ExplicitBackingFields" } - if (project.path !in listOf(":anki-common", ":api", ":common", ":common:android")) { + if (project.path !in listOf(":api", ":common", ":common:android")) { compilerArgs += "-opt-in=kotlinx.coroutines.ExperimentalCoroutinesApi" } if (project.path != ":api") { From 554f2c90ac2efe5891feca2135550dd62c4db30b Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 13:25:04 +0100 Subject: [PATCH 4/6] test: move ChangeManager tests to :anki-common TestChangeSubscriber moves to testFixtures, and ChangeManagerTest uses its ChangeCounter directly Part of 20737 Assisted-by: Claude Fable 5 --- .../ichi2/anki/StudyOptionsViewModelTest.kt | 2 +- .../anki/browser/CardBrowserViewModelTest.kt | 6 +- .../deckpicker/DeckPickerViewModelTest.kt | 2 +- .../anki/tags/ManageTagsViewModelTest.kt | 2 +- .../anki/observability/ChangeManagerTest.kt | 94 ++++++++++++++++--- .../observability}/TestChangeSubscriber.kt | 22 ++--- 6 files changed, 96 insertions(+), 32 deletions(-) rename {AnkiDroid/src/test/java => anki-common/src/test/kotlin}/com/ichi2/anki/observability/ChangeManagerTest.kt (64%) rename {AnkiDroid/src/test/java/com/ichi2/testutils => anki-common/src/testFixtures/kotlin/com/ichi2/anki/observability}/TestChangeSubscriber.kt (80%) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt index 1c697ee7facb..85f4377a6b8b 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/StudyOptionsViewModelTest.kt @@ -6,7 +6,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import anki.scheduler.CardAnswer.Rating import app.cash.turbine.test import com.ichi2.anki.CollectionManager.withCol -import com.ichi2.testutils.ensureOpsExecuted +import com.ichi2.anki.observability.ensureOpsExecuted import kotlinx.coroutines.joinAll import kotlinx.coroutines.launch import org.hamcrest.MatcherAssert.assertThat diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt index bb6777e953c9..23fca6425617 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/browser/CardBrowserViewModelTest.kt @@ -67,6 +67,9 @@ import com.ichi2.anki.model.SelectableDeck import com.ichi2.anki.model.SortType import com.ichi2.anki.model.cardBrowserNoSorting import com.ichi2.anki.noteeditor.toIntent +import com.ichi2.anki.observability.ensureNoOpsExecuted +import com.ichi2.anki.observability.ensureOpWithHandler +import com.ichi2.anki.observability.ensureOpsExecuted import com.ichi2.anki.servicelayer.NoteService import com.ichi2.anki.setFlagFilterSync import com.ichi2.anki.settings.Prefs @@ -75,9 +78,6 @@ import com.ichi2.anki.utils.ext.ignoreAccentsInSearch import com.ichi2.testutils.IntentAssert import com.ichi2.testutils.JvmTest import com.ichi2.testutils.createTransientDirectory -import com.ichi2.testutils.ensureNoOpsExecuted -import com.ichi2.testutils.ensureOpWithHandler -import com.ichi2.testutils.ensureOpsExecuted import com.ichi2.testutils.ext.reopenWithLanguage import com.ichi2.testutils.mockIt import kotlinx.coroutines.CancellationException diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt index f218e15f292e..b61a14b6f3a8 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/deckpicker/DeckPickerViewModelTest.kt @@ -14,7 +14,7 @@ import com.ichi2.anki.libanki.Consts import com.ichi2.anki.libanki.DeckId import com.ichi2.anki.libanki.Note import com.ichi2.anki.libanki.emptyCids -import com.ichi2.testutils.ensureOpsExecuted +import com.ichi2.anki.observability.ensureOpsExecuted import org.hamcrest.CoreMatchers.not import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/tags/ManageTagsViewModelTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/tags/ManageTagsViewModelTest.kt index 018ab1f713cd..b3f1b947ddb5 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/tags/ManageTagsViewModelTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/tags/ManageTagsViewModelTest.kt @@ -6,7 +6,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.MediumTest import com.ichi2.anki.RobolectricTest import com.ichi2.anki.dialogs.utils.AnKingTags -import com.ichi2.testutils.ensureOpsExecuted +import com.ichi2.anki.observability.ensureOpsExecuted import kotlinx.coroutines.flow.first import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.containsInAnyOrder diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/observability/ChangeManagerTest.kt b/anki-common/src/test/kotlin/com/ichi2/anki/observability/ChangeManagerTest.kt similarity index 64% rename from AnkiDroid/src/test/java/com/ichi2/anki/observability/ChangeManagerTest.kt rename to anki-common/src/test/kotlin/com/ichi2/anki/observability/ChangeManagerTest.kt index 5e402a50eb1e..9dc7499cd623 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/observability/ChangeManagerTest.kt +++ b/anki-common/src/test/kotlin/com/ichi2/anki/observability/ChangeManagerTest.kt @@ -2,30 +2,38 @@ package com.ichi2.anki.observability -import androidx.test.ext.junit.runners.AndroidJUnit4 +import android.app.Activity +import android.content.Context import anki.collection.OpChanges -import com.ichi2.testutils.EmptyApplication -import com.ichi2.testutils.JvmTest -import com.ichi2.testutils.subscriberChangeCounter +import com.ichi2.anki.common.crashreporting.CrashReportService +import com.ichi2.anki.common.crashreporting.CrashReporter import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.hamcrest.Matchers.greaterThan +import org.junit.After +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized -import org.robolectric.annotation.Config import kotlin.reflect.KProperty1 import kotlin.reflect.full.memberProperties import kotlin.reflect.javaType import kotlin.reflect.jvm.isAccessible -// this cannot yet use `EmptyApplication` - `CrashReportService` dependency -@RunWith(AndroidJUnit4::class) -class ChangeManagerExceptionHandlingTest : JvmTest() { +class ChangeManagerExceptionHandlingTest { + @Before + fun setUp() { + ChangeManager.resetForTesting() + // an exception in a subscriber is sent to the crash reporting service + CrashReportService.setReporter(NoOpCrashReporter) + } + @Test fun `all subscribers notified even if one throws exception`() { var firstCalled = false @@ -96,9 +104,17 @@ class ChangeManagerTest { } } -@RunWith(AndroidJUnit4::class) -@Config(application = EmptyApplication::class) -class ChangeManagerPublishTest : JvmTest() { +class ChangeManagerPublishTest { + @Before + fun setUp() { + ChangeManager.resetForTesting() + } + + @After + fun tearDown() { + Dispatchers.resetMain() + } + @Test fun `publish notifies multiple subscribers asynchronously`() = runTest { @@ -118,3 +134,59 @@ class ChangeManagerPublishTest : JvmTest() { assertThat("Second subscriber should be notified", counter2.hasChanges, equalTo(true)) } } + +/** + * Produces a [ChangeCounter] which is subscribed to [ChangeManager]. + * + * Query the result via [.hasChanges][ChangeCounter.hasChanges] + */ +private fun subscriberChangeCounter(): ChangeCounter { + val counter = ChangeCounter() + ChangeManager.subscribe(counter) + return counter +} + +/** A [CrashReporter] which ignores all reports */ +private object NoOpCrashReporter : CrashReporter { + override fun sendExceptionReport( + message: String?, + origin: String?, + ) { + } + + override fun sendExceptionReport( + e: Throwable, + origin: String?, + additionalInfo: String?, + onlyIfSilent: Boolean, + ) { + } + + override fun sendExceptionReport( + e: Throwable, + origin: String?, + additionalInfo: String?, + onlyIfSilent: Boolean, + context: Context, + ) { + } + + override fun onPreferenceChanged( + ctx: Context, + newValue: String, + ) { + } + + override fun deleteLimiterData(context: Context) { + } + + override fun setReportingMode(value: String) { + } + + override fun isEnabled( + context: Context, + defaultValue: Boolean, + ): Boolean = false + + override fun sendReport(activity: Activity): Boolean = false +} diff --git a/AnkiDroid/src/test/java/com/ichi2/testutils/TestChangeSubscriber.kt b/anki-common/src/testFixtures/kotlin/com/ichi2/anki/observability/TestChangeSubscriber.kt similarity index 80% rename from AnkiDroid/src/test/java/com/ichi2/testutils/TestChangeSubscriber.kt rename to anki-common/src/testFixtures/kotlin/com/ichi2/anki/observability/TestChangeSubscriber.kt index 48b8cb0c1c44..ae1fa9b41020 100644 --- a/AnkiDroid/src/test/java/com/ichi2/testutils/TestChangeSubscriber.kt +++ b/anki-common/src/testFixtures/kotlin/com/ichi2/anki/observability/TestChangeSubscriber.kt @@ -1,9 +1,8 @@ // SPDX-License-Identifier: GPL-3.0-or-later -package com.ichi2.testutils +package com.ichi2.anki.observability import anki.collection.OpChanges -import com.ichi2.anki.observability.ChangeManager import timber.log.Timber import kotlin.test.fail @@ -59,8 +58,12 @@ suspend fun ensureOpWithHandler( fail("ChangeManager: expected handler to be $handler, but was ${handlerAccessor.handler}") } -// used to ensure a strong reference to the subscription is held -internal class ChangeCounter : ChangeManager.Subscriber { +/** + * Counts calls to [ChangeManager.notifySubscribers] + * + * Used to ensure a strong reference to the subscription is held + */ +class ChangeCounter : ChangeManager.Subscriber { private var changes = 0 val changeCount get() = changes val hasChanges get() = changes > 0 @@ -85,14 +88,3 @@ private class ExtractOpHandler : ChangeManager.Subscriber { this.handler = handler } } - -/** - * Produces a [ChangeCounter] which is subscribed to [ChangeManager]. - * - * Query the result via [.changeCount][ChangeCounter.changeCount] or [.hasChanges][ChangeCounter.hasChanges] - */ -internal fun subscriberChangeCounter(): ChangeCounter { - val counter = ChangeCounter() - ChangeManager.subscribe(counter) - return counter -} From b0d4d1f268b410c48dd7ea55687339c8d7131be1 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 13:01:21 +0100 Subject: [PATCH 5/6] test: use the TestChangeSubscriber fixture in ContentProviderTest Part of 20737 Assisted-by: Claude Fable 5 --- AnkiDroid/build.gradle | 2 + .../ichi2/anki/tests/ContentProviderTest.kt | 40 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/AnkiDroid/build.gradle b/AnkiDroid/build.gradle index 62ec003b5091..72975c7452e3 100644 --- a/AnkiDroid/build.gradle +++ b/AnkiDroid/build.gradle @@ -804,6 +804,8 @@ dependencies { // for testing flows testImplementation libs.cashapp.turbine + androidTestImplementation testFixtures(project(":anki-common")) + // May need a resolution strategy for support libs to our versions androidTestImplementation libs.androidx.espresso.core androidTestImplementation(libs.androidx.espresso.contrib) { diff --git a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt index 5b414f92a46b..ec4627c4b966 100644 --- a/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt +++ b/AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ContentProviderTest.kt @@ -12,7 +12,6 @@ import android.database.Cursor import android.database.CursorWindow import android.net.Uri import anki.cards.FsrsMemoryState -import anki.collection.OpChanges import anki.notetypes.StockNotetype import com.ichi2.anki.CollectionManager import com.ichi2.anki.Flag @@ -37,6 +36,7 @@ import com.ichi2.anki.libanki.backend.BackendUtils import com.ichi2.anki.libanki.exception.ConfirmModSchemaException import com.ichi2.anki.libanki.getStockNotetype import com.ichi2.anki.libanki.sched.Scheduler +import com.ichi2.anki.observability.ChangeCounter import com.ichi2.anki.observability.ChangeManager import com.ichi2.anki.provider.pureAnswer import com.ichi2.anki.testutil.DatabaseUtils.cursorFillWindow @@ -2475,7 +2475,7 @@ class ContentProviderTest : InstrumentedTest() { @Test fun testInsertNotifiesUI() { - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.subscribe(counter) try { val mid = noteTypeId @@ -2504,7 +2504,7 @@ class ContentProviderTest : InstrumentedTest() { ContentValues().apply { put(FlashCardsContract.Note.TAGS, "new_tag") } - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.subscribe(counter) try { contentResolver.update(uri, values, null, null) @@ -2516,7 +2516,7 @@ class ContentProviderTest : InstrumentedTest() { @Test fun testUpdateNonExistentNoteDoesNotNotifyUI() { - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.awaitPendingOpChanges() ChangeManager.subscribe(counter) try { @@ -2531,7 +2531,7 @@ class ContentProviderTest : InstrumentedTest() { } ChangeManager.awaitPendingOpChanges() - assertEquals("UI should not be notified if update is failed", 0, counter.count) + assertEquals("UI should not be notified if update is failed", 0, counter.changeCount) } finally { ChangeManager.unsubscribe(counter) } @@ -2541,7 +2541,7 @@ class ContentProviderTest : InstrumentedTest() { fun testDeleteNotifiesUI() { val noteId = createdNotes.first().lastPathSegment!!.toLong() val uri = Uri.withAppendedPath(FlashCardsContract.Note.CONTENT_URI, noteId.toString()) - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.subscribe(counter) try { contentResolver.delete(uri, null, null) @@ -2553,7 +2553,7 @@ class ContentProviderTest : InstrumentedTest() { @Test fun testDeleteNonExistentNoteDoesNotNotifyUI() { - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.awaitPendingOpChanges() ChangeManager.subscribe(counter) try { @@ -2562,7 +2562,7 @@ class ContentProviderTest : InstrumentedTest() { assertEquals("It should return 0 for non-existent note", 0, deletedCount) ChangeManager.awaitPendingOpChanges() - assertEquals("UI should not be notify if nothing was deleted", 0, counter.count) + assertEquals("UI should not be notify if nothing was deleted", 0, counter.changeCount) } finally { ChangeManager.unsubscribe(counter) } @@ -2570,7 +2570,7 @@ class ContentProviderTest : InstrumentedTest() { @Test fun testBulkInsertNotifiesUI() { - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.subscribe(counter) try { val mid = noteTypeId @@ -2595,14 +2595,14 @@ class ContentProviderTest : InstrumentedTest() { @Test fun testBulkInsertEmptyListDoesNotNotifyUI() { - val counter = TestSubscriber() + val counter = ChangeCounter() ChangeManager.awaitPendingOpChanges() ChangeManager.subscribe(counter) try { contentResolver.bulkInsert(FlashCardsContract.Note.CONTENT_URI, emptyArray()) ChangeManager.awaitPendingOpChanges() - assertEquals("UI should not be notified for empty bulk insert", 0, counter.count) + assertEquals("UI should not be notified for empty bulk insert", 0, counter.changeCount) } finally { ChangeManager.unsubscribe(counter) } @@ -2692,26 +2692,14 @@ class ContentProviderTest : InstrumentedTest() { } } - // TODO: PERF: use TestChangeSubscriber once we've moved to testFixtures - private class TestSubscriber : ChangeManager.Subscriber { - var count = 0 - - override fun opExecuted( - changes: OpChanges, - handler: Any?, - ) { - count++ - } - } - - private fun assertNotificationReceived(subscriber: TestSubscriber) { + private fun assertNotificationReceived(subscriber: ChangeCounter) { val timeout = 5000L val startTime = TimeManager.time.intTimeMS() - while (subscriber.count == 0 && TimeManager.time.intTimeMS() - startTime < timeout) { + while (!subscriber.hasChanges && TimeManager.time.intTimeMS() - startTime < timeout) { Thread.sleep(100) } - assertTrue("UI should be notified of the change", subscriber.count > 0) + assertTrue("UI should be notified of the change", subscriber.hasChanges) } private val contentResolver: ContentResolver From e866129b331651590e154fbe6940fc71153dd213 Mon Sep 17 00:00:00 2001 From: David Allison <62114487+david-allison@users.noreply.github.com> Date: Sun, 16 Aug 2026 02:43:06 +0100 Subject: [PATCH 6/6] refactor: move launchCatching to :anki-common For the widget feature module extraction Part of 20737 Assisted-by: Claude Opus 4.8 (cherry picked from commit 28885ffe7d677a28397b5ffddc32561d29ce4cca) --- .../java/com/ichi2/anki/CoroutineHelpers.kt | 35 -------------- .../kotlin/com/ichi2/anki/LaunchCatching.kt | 46 +++++++++++++++++++ .../com/ichi2/anki/LaunchCatchingTest.kt | 2 +- 3 files changed, 47 insertions(+), 36 deletions(-) create mode 100644 anki-common/src/main/kotlin/com/ichi2/anki/LaunchCatching.kt rename AnkiDroid/src/test/java/com/ichi2/anki/CoroutineHelpersTest.kt => anki-common/src/test/kotlin/com/ichi2/anki/LaunchCatchingTest.kt (97%) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index 2d727f075457..83a33d1016c1 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -35,7 +35,6 @@ import com.ichi2.anki.dialogs.DatabaseErrorDialog import com.ichi2.anki.dialogs.DatabaseErrorDialog.DatabaseErrorDialogType import com.ichi2.anki.exception.StorageAccessException import com.ichi2.anki.exception.StorageNotConfiguredException -import com.ichi2.anki.libanki.exception.InvalidSearchException import com.ichi2.anki.pages.toIntent import com.ichi2.anki.snackbar.showSnackbar import com.ichi2.anki.startup.redirectToMainEntryPoint @@ -72,8 +71,6 @@ import net.ankiweb.rsdroid.exceptions.BackendNetworkException import net.ankiweb.rsdroid.exceptions.BackendSyncException import org.jetbrains.annotations.VisibleForTesting import timber.log.Timber -import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds @@ -86,38 +83,6 @@ var ioDispatcher: CoroutineDispatcher = Dispatchers.IO @VisibleForTesting var throwOnShowError = false -/** - * Runs a suspend function that catches any uncaught errors and reports them to the user. - * Errors from the backend contain localized text that is often suitable to show to the user as-is. - * Other errors should ideally be handled in the block. - * - * @param context Coroutine context passed to [launch] - * @param errorMessageHandler Called after an exception is caught and logged, input is either - * `Exception.localizedMessage` or `Exception.toString()` - * @param block code to execute inside [launch] - */ -fun CoroutineScope.launchCatching( - context: CoroutineContext = EmptyCoroutineContext, - errorMessageHandler: suspend (String) -> Unit, - block: suspend CoroutineScope.() -> Unit, -): Job = - launch(context) { - try { - block() - } catch (cancellationException: CancellationException) { - // CancellationException should be re-thrown to propagate it to the parent coroutine - throw cancellationException - } catch (exception: Exception) { - Timber.w(exception) - val message = - when (exception) { - is BackendException, is InvalidSearchException -> exception.localizedMessage - else -> null - } ?: exception.toString() - errorMessageHandler.invoke(message) - } - } - interface OnErrorListener { val onError: MutableSharedFlow } diff --git a/anki-common/src/main/kotlin/com/ichi2/anki/LaunchCatching.kt b/anki-common/src/main/kotlin/com/ichi2/anki/LaunchCatching.kt new file mode 100644 index 000000000000..1d67b86a0539 --- /dev/null +++ b/anki-common/src/main/kotlin/com/ichi2/anki/LaunchCatching.kt @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// SPDX-FileCopyrightText: Copyright (c) 2023 Brayan Oliveira <69634269+brayandso@users.noreply.github.com> + +package com.ichi2.anki + +import com.ichi2.anki.libanki.exception.InvalidSearchException +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import net.ankiweb.rsdroid.BackendException +import timber.log.Timber +import kotlin.coroutines.CoroutineContext +import kotlin.coroutines.EmptyCoroutineContext + +/** + * Runs a suspend function that catches any uncaught errors and reports them to the user. + * Errors from the backend contain localized text that is often suitable to show to the user as-is. + * Other errors should ideally be handled in the block. + * + * @param context Coroutine context passed to [launch] + * @param errorMessageHandler Called after an exception is caught and logged, input is either + * `Exception.localizedMessage` or `Exception.toString()` + * @param block code to execute inside [launch] + */ +fun CoroutineScope.launchCatching( + context: CoroutineContext = EmptyCoroutineContext, + errorMessageHandler: suspend (String) -> Unit, + block: suspend CoroutineScope.() -> Unit, +): Job = + launch(context) { + try { + block() + } catch (cancellationException: CancellationException) { + // CancellationException should be re-thrown to propagate it to the parent coroutine + throw cancellationException + } catch (exception: Exception) { + Timber.w(exception) + val message = + when (exception) { + is BackendException, is InvalidSearchException -> exception.localizedMessage + else -> null + } ?: exception.toString() + errorMessageHandler.invoke(message) + } + } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/CoroutineHelpersTest.kt b/anki-common/src/test/kotlin/com/ichi2/anki/LaunchCatchingTest.kt similarity index 97% rename from AnkiDroid/src/test/java/com/ichi2/anki/CoroutineHelpersTest.kt rename to anki-common/src/test/kotlin/com/ichi2/anki/LaunchCatchingTest.kt index a80654105e2d..4a4c0a0dc0d5 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/CoroutineHelpersTest.kt +++ b/anki-common/src/test/kotlin/com/ichi2/anki/LaunchCatchingTest.kt @@ -9,7 +9,7 @@ import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test -class CoroutineHelpersTest { +class LaunchCatchingTest { @Test fun `launchCatching does not include class names for InvalidSearchException`() = runTest {