test: Phase 3 — Mockito migration, fakes infrastructure, and coverage gaps#3539
Open
ksalhab89 wants to merge 21 commits intoquran:mainfrom
Open
test: Phase 3 — Mockito migration, fakes infrastructure, and coverage gaps#3539ksalhab89 wants to merge 21 commits intoquran:mainfrom
ksalhab89 wants to merge 21 commits intoquran:mainfrom
Conversation
- Add sqldelight-sqlite-driver for in-memory database testing - Test all DAO operations: bookmarks, toggles, recent pages - Test Flow emissions and change notifications - Use real BookmarksDatabase with proper column adapters - All tests use Turbine for Flow testing and Truth for assertions Phase 2 Week 1 complete - exceeded 15 test target with 16 tests
Document the 'fakes over mocks' pattern with: - Why shared fakes don't work (circular dependencies) - Correct pattern: local fakes in test source sets - Concrete examples for QuranSettings - Migration strategy for Week 3-4 presenter tests - Key principles and best practices This guide will serve as reference for implementing presenter tests in Phase 2 Week 3-4.
Tests cover: - Screen binding/unbinding lifecycle - Page coordinates loading with shouldOverlayPageInfo setting - Ayah coordinates loading after page coordinates - Image downloading on first bind - Invalid page filtering - Error handling for coordinates - Disposable cleanup on unbind Uses Mockito for framework-dependent QuranSettings (private constructor). Other dependencies mocked to focus on presenter logic. All 9 tests passing.
Tests cover: - Basic playback when files are available - Streaming mode when files missing - Streaming override when files are available - Start/End ayah swapping when reversed - Permission callbacks (download granted, download success) - Lifecycle management (bind/unbind) Note: Download tests requiring Intent creation are deferred (need Robolectric setup). Core business logic is validated. Added test-utils dependency to app module for TestDataFactory access. All 8 tests passing.
Phase 1 Complete: - QuranInfo tests (21 tests) - BookmarksDaoImpl tests (16 tests) - Test infrastructure (TestDataFactory, RxSchedulerRule) - Fakes-over-mocks pattern established Phase 2 Complete: - QuranPagePresenter tests (9 tests) - AudioPresenter tests (8 tests) - Pragmatic mocking strategy documented - Total: 33 new tests, all passing Updated roadmap to show completed phases and document key decisions.
Replace @BeforeClass/@afterclass RxAndroidPlugins setup with RxSchedulerRule to ensure proper cleanup after each test. This prevents scheduler state from leaking between tests when running the full test suite. All 33 Phase 1 & 2 tests still passing.
Analyzed 10 pre-existing test files using Mockito to identify migration candidates for fakes-over-mocks strategy. High-Priority Candidates: - BookmarkModelTest (13 mocks) → in-memory SQLite - RecentPageModelTest (12 mocks) → in-memory SQLite - BookmarkPresenterTest (14 mocks) → FakeBookmarkModel Target: Reduce Mockito files from 10 to 6-7 (50% reduction) Strategy: - Week 1: Migrate database tests to in-memory SQLite - Week 2: Create FakeBookmarkModel and migrate presenter tests - Week 3: Evaluate remaining files, document decisions Documented patterns, metrics, and implementation guidance.
Removed Thread.sleep(600) from async timer test. The RxSchedulerRule's trampoline scheduler executes Completable.timer() immediately without waiting for the actual 500ms delay, making Thread.sleep unnecessary and potentially flaky. Changes: - Removed Thread.sleep(600) from line 148 - Added comment explaining trampoline behavior - All 126 tests pass with fix applied
Added test dependencies required for in-memory SQLite testing: - sqldelight-sqlite-driver: JVM SQLite driver for in-memory DBs - sqldelight-primitive-adapters: Int column adapters for BookmarksDatabase
…-memory SQLite Replace Mockito-based Java tests with real in-memory SQLite implementations: - BookmarkModelTest: 2 tests using JdbcSqliteDriver.IN_MEMORY - RecentPageModelTest: 6 tests using JdbcSqliteDriver.IN_MEMORY - Fix @BeforeClass test pollution → use RxSchedulerRule per test - Zero Mockito usage in these test files
BookmarkPresenterTest: Replace @BeforeClass RxAndroidPlugins with RxSchedulerRule to prevent scheduler state from leaking between test classes. TagBookmarkPresenterTest: Remove spy() and unnecessary MockitoAnnotations, keeping only minimal mock() for Android framework dependencies.
Add stateful fakes to replace Mockito mocks in future tests: - FakeBookmarksDBAdapter: In-memory bookmark database adapter - FakeBookmarkModel / FakeRecentPageModel: Business logic fakes - FakeQariUtil, FakeQuranFileUtils: Utility fakes - FakeTranslationModel, FakeTranslationsDBAdapter, FakeTranslationListPresenter: Translation fakes - FakeContentResolver: Content resolver fake - UI fakes: FakeQuranPageScreen, FakePagerActivity, FakeTagBookmarkDialog, FakeCoordinatesModel, FakeQuranPageLoader, FakeAudioExtensionDecider, FakeQuranDisplayData - Test helpers: ContextTestHelpers, ResourcesTestHelpers, DatabaseHandlerTestHelpers
- TagBookmarkPresenterTest: Replace @BeforeClass RxAndroidPlugins with RxSchedulerRule to match all other test files and prevent scheduler state from leaking between test classes - RecentPageModelTest: Make SAMPLE_RECENT_PAGES immutable (listOf) to prevent accidental future mutation of companion object state - FakeBookmarkModel/FakeRecentPageModel: Document why passing a Mockito mock to the real constructor is safe (constructor stores but never calls methods on the adapter)
…fakes FakeBookmarkModel and FakeRecentPageModel were passing mock(BookmarksDBAdapter) to the real superclass constructor. Replace with a real BookmarksDBAdapter backed by an in-memory SQLite database (same JdbcSqliteDriver.IN_MEMORY pattern used in BookmarkModelTest). The fakes are now true fakes with zero Mockito dependency.
…RecentPageModel - AudioUtilsTest: extract shared audioUtils() helper, remove dead pageProviderMock vars; document QuranFileUtils/QariUtil as constructor fillers until Robolectric added - TagBookmarkPresenterTest: replace mock(BookmarksDBAdapter) with inMemoryBookmarksAdapter() and mock(RecentPageModel) with FakeRecentPageModel() across all 3 anonymous subclasses; retain mock(TagBookmarkDialog) — extends DialogFragment, needs Robolectric to instantiate - FakeRecentPageModel: fix JVM init-order NPE: RecentPageModel.init calls getRecentPagesObservable() before subclass fields are initialized; switch recentPages and getRecentPagesObservableCalls to lazy-init backing vars so they survive the pre-init window safely
…PageProvider
A4: FakePageProvider implements PageProvider (17 abstract methods)
Enables QariUtil(FakePageProvider()) without mocks
AudioUtilsTest migrated to Robolectric — 0 Mockito imports
A2: Extract interfaces from 3 non-open concrete classes (DIP)
TranslationModel → interface + TranslationModelImpl
TranslationsDBAdapter → interface + TranslationsDBAdapterImpl
TranslationListPresenter → interface + TranslationListPresenterImpl
Metro DI bindings added in DatabaseModule + PagerActivityModule
FakeTranslationModel, FakeTranslationsDBAdapter implement interfaces
FakeTranslationListPresenter uses production interface
BaseTranslationPresenterTest — 0 Mockito imports
A1: Robolectric for Android context dependencies
BookmarkImportExportModelTest.java → .kt (FakeBookmarkModel, real context)
ArabicDatabaseUtilsTest: real Robolectric context replaces mock(Context)
2 remaining mocks: DatabaseHandler (private ctor), QuranFileUtils (complex ctor)
…tial QuranImportPresenter migration ArabicDatabaseUtilsTest: replace @mock QuranFileUtils with a real instance constructed via Robolectric context + FakePageProvider + QuranScreenInfo (same pattern established in AudioUtilsTest). DatabaseHandler mock remains — it has a private constructor and is non-open; it is returned from an override and never interacted with. QuranImportPresenterTest (A3): add Robolectric runner + Config; replace mock(Context)/mock(Uri)/mock(BookmarkModel) with real Robolectric context, Uri.parse(), and FakeBookmarkModel(). Use ShadowContentResolver for testParseExternalFile. Three tests still require Mockito ContentResolver + Context because ShadowContentResolver 4.16.1 does not shadow openFileDescriptor and returns a non-null sentinel for unregistered URIs rather than null; ParcelFileDescriptor mock also remains for the null-fd NPE path. All 4 QuranImportPresenterTest and 6 ArabicDatabaseUtilsTest tests pass.
…ode quality Fix pre-existing flaky failures (BookmarkModelTest, RecentPageModelTest, TagBookmarkPresenterTest) caused by Robolectric's sandbox classloader intercepting the SQLite JDBC driver before plain JUnit tests could access it via DriverManager. Fix: add @RunWith(RobolectricTestRunner::class) to all SQLite-backed tests so every test uses the same classloader context. Full suite now passes in back-to-back runs. Fix LongArray equality semantics: UpdateTagsCall (FakeBookmarkModel) and TagBookmarksCall (FakeBookmarksDBAdapter) used LongArray in data classes which breaks equals/hashCode. Changed to List<Long> and removed the contentEquals() workaround in assertion methods. Code quality: - Remove dead FakeContentResolver (never used in any test) - Rename getGetBookmarkTagIdsCallCount() -> getBookmarkTagIdsCallCount() - Rename getGetRecentPagesObservableCallCount() -> getRecentPagesObservableCallCount() - ArrayList<Bookmark>() -> mutableListOf() in ArabicDatabaseUtilsTest - Anonymous HashMap init block -> mapOf() in BaseTranslationPresenterTest - ArrayList() -> emptyList() in BaseTranslationPresenterTest - Add comments explaining the deprecated Display API constraint in tests
…ad test files - Add @RunWith(RobolectricTestRunner) to AudioPresenterTest (needed for Intent creation) - Add 5 new tests covering all download intent paths: - Path A: aya position file missing -> handleRequiredDownload - Path B: gapless database file missing -> handleRequiredDownload - Path C: basmallah download -> single-verse intent (start == end) - Path D: full range download -> start/end/isGapless extras verified - Path E: onPostNotificationsPermissionResponse -> proceedWithDownload (bypass) - Delete 5 dead files: FakeQariUtil, FakeQuranFileUtils, ExampleUsage (unused fakes/stubs), BookmarkModelTest.java, RecentPageModelTest.java (commented-out stubs with Kotlin equivalents)
…add coverage - Extract inMemoryBookmarksAdapter() to DatabaseTestHelpers.kt (3 duplicates removed) - Fix AudioPresenterTest: getBooleanExtra default true→false (masks bug when extra absent) - Fix BookmarkModelTest: !! force-unwrap → requireNotNull() (idiomatic Kotlin) - Add @RunWith/@config to BaseTranslationPresenterTest (was missing Robolectric context) - Add getVerses error path tests to BaseTranslationPresenterTest (covers getVerses$2 lambda) - Add FakeTranslationModel error injection support (setArabicError/setTranslationError) - Add export path tests to BookmarkImportExportModelTest (covers exportBookmarks* methods)
QariItem gained opusUrl parameter, shifting positional args in AudioUpdaterTest. GaplessAudioInfoCommand and GappedAudioInfoCommand gained allowedExtensions parameter, breaking calls in audio info command tests.
ksalhab89
force-pushed
the
feature/testing-phase2
branch
from
814b349 to
5fcf46f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 3 of the testing infrastructure. Migrates remaining Mockito-heavy tests to fakes and Robolectric, closes coverage gaps, and fixes pre-existing test breaks from upstream API changes.
Changes
Mockito Migration
Migrated all feasible tests away from Mockito mocks to real fakes or Robolectric:
BookmarkModel,RecentPageModel→ in-memory SQLite + fakesTagBookmarkPresenter→FakeBookmarkModel,FakeRecentPageModelQuranImportPresenter→ Robolectric +ShadowContentResolverBaseTranslationPresenter→FakeTranslationModelArabicDatabaseUtils,AudioUtils,BookmarkImportExportModel→ Robolectric real contextRemaining Mockito usage is justified:
verify()interaction tests or non-open classes.Fakes Created
FakeBookmarkModel,FakeRecentPageModel,FakeTranslationModel,FakeTranslationsDBAdapter,FakeTranslationListPresenter,FakePageProvider,FakeBookmarksDBAdapterInterface Extractions (for testability)
TranslationModel,TranslationsDBAdapter,TranslationListPresenterCoverage Gaps Closed
AudioPresenterdownload paths A–E (aya position, gapless DB, basmallah, full range, permission flow)BaseTranslationPresenter.getVerses()coroutine exception paths (was 0%)BookmarkImportExportModelexport path (was 0%)Code Quality
inMemoryBookmarksAdapter()toDatabaseTestHelpers.kt(removes 3 duplicates)!!force-unwrap →requireNotNull()inBookmarkModelTestgetBooleanExtrawrong default inAudioPresenterTest(masked bug when extra absent)@RunWith/@ConfigtoBaseTranslationPresenterTestPre-existing Test Fixes (upstream API changes)
AudioUpdaterTest—QariItem.opusUrladded upstream; updated to named paramsGaplessAudioInfoCommandTest/GappedAudioInfoCommandTest—allowedExtensionsparam added to download query methodsPhase 3 Commits (above Phase 2)
Test Plan
./gradlew :app:testMadaniDebugUnitTest— 146 tests, 0 failures, 0 flaky (2 consecutive runs)./gradlew :common:data:test :common:bookmark:test— pass./gradlew :feature:audio:testReleaseUnitTest :common:audio:testReleaseUnitTest— pass./gradlew :app:lintMadaniDebug— 0 errors