Expand Compose UI test coverage for market list, detail, and app navigation

[masoudkarimi]

Summary

• Add Compose UI tests for favorite market list states, item actions, swipe-to-remove, pull refresh, and multi-item rendering.
• Add paged market list tests for loaded data, refresh errors, append loading/error states, item clicks, and favorite clicks.
• Add market detail screen tests for favorite actions, stat rows, loading state behavior, and empty chart data.
• Add app-level instrumentation test setup with a custom runner, test application, Koin test module, and fake market repository.
• Add a MainActivity instrumentation test that navigates to the Favorite screen and verifies the empty favorite state.
• Add a test tag for the paging append loading indicator.
• Handle empty chart data in QuadLineChart to avoid division-by-zero and show a clear empty state.
• Correct favorite item swipe direction for dismiss behavior.

Verification

• ./gradlew :app:connectedDebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=ir.composenews.ui.MainActivityTest

Notes

• gradle/gradle-daemon-jvm.properties is untracked local Gradle state and should not be included in the PR.

[Kaaveh]

@claude

[claude]

Claude finished @Kaaveh's task in 2m 28s —— View job

---

Code Review

• Gather context and understand the PR
• Read changed files
• Review test infrastructure (TestApplication, TestModule, InstrumentationTestRunner)
• Review MarketListScreen tests
• Review PagedMarketListScreen tests
• Review MarketDetailScreen tests
• Review production code changes (QuadLineChart, MarketItem, MarketListScreen)
• Post review findings

---

Good test coverage expansion overall — the test pyramid coverage for these screens is a clear improvement. A few issues to address, ranging from a build-config bug to test quality gaps.

Findings

🔴 High — Test artifacts in wrong dependency scope (app/build.gradle.kts:127-128)

compose.ui.test.manifest and compose.ui.test.junit4 are declared as implementation, which means they ship in the production APK. They should be:

// app/build.gradle.kts
debugImplementation(compose.ui.test.manifest)   // was: implementation
androidTestImplementation(compose.ui.test.junit4) // was: implementation

Fix this →

---

🔴 High — Empty test with no assertions (MarketDetailScreenTest.kt:177-205)

givenLoadedMarketState_whenRendered_thenChartIsDisplayed sets up content but makes zero assertions. It passes vacuously and adds no test value. Either add a meaningful assertion (e.g. checking a chart component node exists via a testTag) or remove the test.

Fix this →

---

🟡 Medium — TestApplication may fail on repeated runs due to missing stopKoin()

If two instrumentation test runs share a process (common with orchestrated suites), the second call to startKoin {} in TestApplication.onCreate() will throw KoinApplicationAlreadyStartedException. Override onTerminate() to call stopKoin():

override fun onTerminate() {
    stopKoin()
    super.onTerminate()
}

Fix this →

---

🟡 Medium — buildMarketModel duplicated across three test files

An identical buildMarketModel helper appears in MarketListScreenTest.kt, PagedMarketListScreenTest.kt, and MarketDetailScreenTest.kt. This should be extracted to a shared test utility in core:test, similar to how the module already provides DispatcherProvider.

---

🟡 Medium — Duplicate test that only partially validates its claim (MarketDetailScreenTest.kt:79-104)

givenLoadedMarketStateWithFavorite_whenRendered_thenFavoritedMarketNameIsVisible only asserts "Bitcoin" is shown — identical to givenLoadedMarketState_whenRendered_thenMarketNameIsVisible. Since the test name promises "favorite" behavior, it should additionally assert the favorited icon is present:

composeTestRule.onNodeWithContentDescription("Favorited").assertExists()

---

🟡 Medium — assertExists() vs assertIsDisplayed() in stat row test (MarketDetailScreenTest.kt:152-159)

Most assertions in givenLoadedMarketState_whenRendered_thenStatRowsAreVisible use assertExists(), which only checks the semantics tree — a node off-screen would pass. assertIsDisplayed() would give a stronger guarantee.

---

🟡 Medium — TestablePagedMarketListScreen is implicitly public (PagedMarketListScreenTest.kt:33-44)

This top-level composable leaks into the package's public API. Composable functions cannot be private, but they can be internal:

@Composable
internal fun TestablePagedMarketListScreen(...)

---

🟡 Medium — Unused import (PagedMarketListScreenTest.kt:13)

import androidx.compose.ui.test.hasProgressBarRangeInfo is imported but never referenced.

Fix this →

---

🟢 Minor — Callback captures outer scope instead of lambda param (MarketListScreenTest.kt:129)

onFavoriteClick = { clickedMarket = market } // captures outer `market`

Should use the lambda parameter for semantic correctness:

onFavoriteClick = { clickedMarket = it }

---

🟢 Minor — Typo in variable names (PagedMarketListScreenTest.kt)

pagedDate (lines 98, 115, 133, 150, 162) should be pagingData to match the surrounding naming convention.

---

🟢 Minor — Stray comment (MarketListScreenTest.kt:217-219)

// Also we can use
// swipeLeft()

This leftover comment should be removed.

---

🟢 Minor — Text empty state missing typography style (QuadLineChart.kt:82-85)

The "No chart data" Text has no explicit style, falling back to default typography. Applying style = MaterialTheme.typography.bodyMedium (or similar) would keep it consistent with the design system.

---

Positive Notes

• The QuadLineChart division-by-zero fix for empty data is clean and the early-return with an empty state is the right approach.
• The MarketItem swipe direction fix (StartToEnd → EndToStart) is correct.
• The testTag("Paging Progress Bar") addition enabling paged append-loading tests is a good testability improvement.
• Using Dispatchers.Unconfined in DispatcherProvider for tests is a pragmatic choice for instrumentation tests.
• The TestablePagedMarketListScreen wrapper is a clean solution to the LazyPagingItems collection problem in tests.

[masoudkarimi]

Hi @Kaaveh ,

I noticed that we currently use the core:test module to share test-only utilities across modules. For this kind of shared test code, Gradle/AGP test fixtures seem like a better fit because they expose helpers only to test source sets instead of making them part of the module’s main API surface.

I’m planning to convert the shared test-only utilities in core:test to a testFixtures source set and update consumers to depend on it via testImplementation(testFixtures(...)).

Are you OK with this direction?

References:

• Gradle test fixtures: https://docs.gradle.org/current/userguide/java_testing.html#sec:java_test_fixtures
• Android Gradle Plugin testFixtures DSL: https://developer.android.com/reference/tools/gradle-api/8.13/com/android/build/api/dsl/TestFixtures

[masoudkarimi]

@claude

[Kaaveh]

@claude

[claude]

Claude finished @Kaaveh's task in 2m 48s —— View job

---

Response to masoudkarimi's testFixtures proposal

• Gather context and understand the request
• Read changed files (core/test build config, testFixtures files)
• Review testFixtures implementation
• Post findings and response

---

@masoudkarimi — the direction is correct and the implementation is mostly there. Converting core:test to testFixtures is the right call: it scopes shared test utilities behind testImplementation(testFixtures(...)) so they can never accidentally appear in the production APK. The MarketModelFixtures fixture directly resolves the duplicate buildMarketModel finding from the earlier review.

That said, there are a few issues in the current implementation worth addressing:

🔴 High — testFixtures files are Java, not Kotlin

All three files under src/testFixtures/java/ were written as .java, which is inconsistent with the rest of the codebase and introduces unsafe Kotlin coroutine interop. The files should be .kt under src/testFixtures/kotlin/.

The Java implementation of MainCoroutineListener is particularly risky:

// Current — Java with raw Continuation interop
@Override
public Object beforeSpec(Spec spec, Continuation<? super Unit> continuation) {
    TestDispatchers.setMain(Dispatchers.INSTANCE, testDispatcher);
    return Unit.INSTANCE;
}

vs. the clean original Kotlin that was deleted:

// Original Kotlin — correct
override suspend fun beforeSpec(spec: Spec) {
    Dispatchers.setMain(testDispatcher)
}

The Java version also calls TestDispatchers.setMain (an internal API) instead of Dispatchers.setMain from kotlinx.coroutines.test. This could break with any coroutines library update.

Fix this →

---

🟡 Medium — SuspendSpec deleted but not migrated

SuspendSpec.kt was deleted from src/main/java/ but has no replacement in src/testFixtures/. No test currently imports it, so this may be intentional — but if it's being intentionally dropped, a note confirming that would help reviewers.

---

🟡 Medium — DispatcherProvider belongs in a production module, not core:test

domain/market/build.gradle.kts has both:

api(core.test)                          // leaks test module into production graph
testImplementation(testFixtures(core.test))

The reason api(core.test) is needed is that DispatcherProvider (the interface used by ViewModels) lives in core:test. Moving DispatcherProvider and PlatformDispatcherProvider to core:base would let you remove this api(core.test) dependency entirely, keeping test infrastructure strictly out of the production graph. That said, this is a larger refactor and may be out of scope for this PR.

---

🟡 Medium — app/build.gradle.kts uses implementation(core.test) in main scope

implementation(core.test)   // line 109 — ships test module in production APK

This is a pre-existing issue, but the testFixtures migration is a good moment to change it to testImplementation or androidTestImplementation (or remove it if the dependency is covered by a transitive path).

---

Bottom line

The testFixtures direction is correct and should be approved conceptually. The immediate blockers are the Java-vs-Kotlin issue (the coroutine API misuse is a correctness risk) and the dropped Dispatchers.setMain call. The DispatcherProvider-in-production-module concern is real but can be a follow-up.

[Kaaveh]

@claude

[claude]

Claude finished @Kaaveh's task in 3m 34s —— View job

---

Changes Applied

• Gather context — checked current file state vs review findings
• Fix app/build.gradle.kts: move test dependencies to correct scopes
• Convert testFixtures Java files to Kotlin
• Commit and push

---

What was fixed in this pass

app/build.gradle.kts

// Before
implementation(compose.ui.test.manifest)
implementation(compose.ui.test.junit4)

// After
debugImplementation(compose.ui.test.manifest)
androidTestImplementation(compose.ui.test.junit4)

These were shipping in the production APK; now they're correctly scoped to debug and androidTest configurations.

core/test testFixtures — Java → Kotlin

All three Java files have been replaced with idiomatic Kotlin equivalents:

MainCoroutineListener.kt — replaces the raw Continuation interop and TestDispatchers internal API with the correct public Kotlin API:

class MainCoroutineListener(
    val testDispatcher: TestDispatcher = UnconfinedTestDispatcher(),
) : BeforeSpecListener, AfterSpecListener {
    override suspend fun beforeSpec(spec: Spec) { Dispatchers.setMain(testDispatcher) }
    override suspend fun afterSpec(spec: Spec) { Dispatchers.resetMain() }
}

TestDispatcherProvider.kt — clean Kotlin class implementing the DispatcherProvider interface directly as properties, no Java getter bridging:

class TestDispatcherProvider(testScheduler: TestCoroutineScheduler) : DispatcherProvider {
    override val ui: TestDispatcher = UnconfinedTestDispatcher(testScheduler)
    override val io: TestDispatcher = UnconfinedTestDispatcher(testScheduler)
    override val bg: TestDispatcher = UnconfinedTestDispatcher(testScheduler)
}

MarketModelFixtures.kt — idiomatic Kotlin object with the same overloads as the Java version to keep all callers unchanged.

---

Already resolved in earlier commits

• stopKoin() in TestApplication.onTerminate() ✅
• Unused import, pagedDate → pagingData, internal visibility in PagedMarketListScreenTest ✅
• Stray comments and callback lambda fix in MarketListScreenTest ✅
• Chart test assertion, duplicate test removal, assertIsDisplayed() in MarketDetailScreenTest ✅
• DispatcherProvider moved to core:base ✅
• implementation(core.test) removed from production scope ✅
• bodyMedium style on empty chart text ✅
  | Branch: compose-tests