diff --git a/.claude/agents/test-flow-architect.md b/.claude/agents/test-flow-architect.md index 2ed69e343a8..870b44c5b0e 100644 --- a/.claude/agents/test-flow-architect.md +++ b/.claude/agents/test-flow-architect.md @@ -145,7 +145,7 @@ Steps: can audit. 2. Invoke the `android-testing` skill and pass it the approved plan. 3. Generate Robot additions and Test classes in the correct source set - (`androidInstrumentedTest` for UI flows, `commonTest` for integration + (`src/androidTest` for UI flows, `commonTest` for integration tests). Reference the claimed program UID(s) as constants in the test intents (e.g. in `EventIntents.kt`). 4. Run lint and the targeted tests: diff --git a/.claude/skills/android-testing/SKILL.md b/.claude/skills/android-testing/SKILL.md index 768b797deae..705d9903013 100644 --- a/.claude/skills/android-testing/SKILL.md +++ b/.claude/skills/android-testing/SKILL.md @@ -1,441 +1,129 @@ --- name: android-testing description: > - Guidelines for writing unit tests (mockito-kotlin, Turbine, runTest) and - UI instrumented tests (Robot pattern, CoroutineTracker, Compose test tags) - for the DHIS2 Android KMP project. Load this when creating, fixing, or - reviewing any test in the codebase. + Guidelines for writing tests in the DHIS2 Android KMP project — host unit tests + (commonTest / androidHostTest, mockito-kotlin, Turbine, runTest) and instrumented + device tests (src/androidTest, Robot pattern, Compose test tags). Routes to + references/unit-testing.md and references/instrumented-testing.md. Load this when + creating, fixing, or reviewing any test in the codebase. --- -# DHIS2 Android Testing Guidelines +# DHIS2 Android Testing -## Testing Stack +Start here, then open the reference you need. This file is the router and the +invariants; the details live in `references/`. -- **Unit Tests**: `mockito-kotlin` (`mock()`, `whenever()`, `verify()`), JUnit / `kotlin.test` -- **Flow tests**: Turbine (`app.cash.turbine`) + `kotlinx-coroutines-test` -- **UI Tests**: Compose Testing + Espresso with Robot pattern -- **Test locations**: - - `commonTest/` — platform-agnostic unit tests (`kotlin.test` annotations: `@Test`, `@BeforeTest`) - - `androidUnitTest/` — Android-specific unit tests (JUnit `@Test`) - - `androidInstrumentedTest/` / `androidTest/` — UI/instrumented tests +## Which test am I writing? -## Run Commands +| What you are testing | Module type | Source set | Gradle task | +| --- | --- | --- | --- | +| Domain logic, use case, ViewModel | KMP | `src/commonTest/kotlin` | `:mod:testAndroidHostTest` | +| Anything touching `org.hisp.dhis.android.core.*` (i.e. mocks `D2`) | KMP | `src/androidHostTest/kotlin` | `:mod:testAndroidHostTest` | +| Anything in `app` | AGP | `src/test/java` | `:app:testDhis2DebugUnitTest` | +| Anything in another AGP module | AGP | `src/test/java` | `:mod:testDebugUnitTest` | +| UI flow on a device | AGP | `src/androidTest/java` | BrowserStack matrix | -```bash -# Shortcut: lint + all unit tests (mirrors CI) -./run_tests.sh - -# All unit tests (legacy + KMP host + KMP debug) -./gradlew testDebugUnitTest testDhis2DebugUnitTest testAndroidHostTest - -# Desktop targets in KMP modules -./gradlew desktopTest - -# Single KMP module test class (commonTest + androidUnitTest source sets) -./gradlew :login:testAndroidHostTest --tests "org.dhis2.mobile.login.main.ui.viewmodel.LoginViewModelTest" - -# Single KMP module test class (androidUnitTest source set only) -./gradlew :login:testAndroidDebugUnitTest --tests "org.dhis2.mobile.login.main.ui.viewmodel.LoginViewModelTest" - -# Single legacy Android module test class -./gradlew :form:testDebugUnitTest --tests "org.dhis2.form.ui.FormViewModelTest" - -# Single test method (commonTest + androidUnitTest source sets) -./gradlew :login:testAndroidHostTest --tests "org.dhis2.mobile.login.main.ui.viewmodel.LoginViewModelTest.initial screen is set correctly when starting" -``` - -## Critical Rule: No Hard-Coded Delays - -ViewModels use `launchUseCase { }` which wraps `CoroutineTracker`. Espresso's -`IdlingResource` automatically waits for tracked coroutines. `Thread.sleep()` and -hard-coded timeouts are **forbidden**. - -```kotlin -// ✅ CORRECT — IdlingResource waits automatically -@Test -fun shouldLoadData() { - exampleRobot(composeTestRule) { - clickLoadButton() - verifyDataDisplayed() // no delay needed - } -} - -// ❌ WRONG -@Test -fun shouldLoadData() { - clickLoadButton() - Thread.sleep(2000) // FORBIDDEN - verifyDataDisplayed() -} -``` - -## Mocking: mockito-kotlin only - -Use `mock()`, `whenever()`, `verify()` from `org.mockito.kotlin`. Do **not** use MockK. - -```kotlin -import org.mockito.kotlin.mock -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever -``` - -## Unit Test Patterns - -### Use Case test - -```kotlin -class SavePinUseCaseTest { - private val repository: SessionRepository = mock() - private val useCase = SavePinUseCase(repository) - - @Test - fun `should return success when pin is saved`() = runTest { - whenever(repository.savePin("1234")).thenReturn(Unit) - - val result = useCase("1234") - - assertTrue(result.isSuccess) - verify(repository).savePin("1234") - } -} -``` - -### ViewModel test - -```kotlin -class ExampleViewModelTest { - private val useCase: GetDataUseCase = mock() - private val testDispatcher = UnconfinedTestDispatcher() - private lateinit var viewModel: ExampleViewModel - - @BeforeTest - fun setUp() { - Dispatchers.setMain(testDispatcher) - } - - @AfterTest - fun tearDown() { - Dispatchers.resetMain() - } - - @Test - fun `should emit success state when use case succeeds`() = runTest { - whenever(useCase()).thenReturn(Result.success(flowOf(listOf(item)))) - - viewModel = ExampleViewModel(useCase) - - viewModel.uiState.test { - assertEquals(UiState.Success(listOf(item)), awaitItem()) - } - } -} -``` - -### Repository test - -```kotlin -class ExampleRepositoryTest { - // D2 chains calls across multiple intermediate objects — RETURNS_DEEP_STUBS is required - // so that d2.someModule().someRepository().blockingGet() doesn't NPE on the intermediates. - private val d2: D2 = mock(defaultAnswer = Mockito.RETURNS_DEEP_STUBS) - private val domainErrorMapper: DomainErrorMapper = mock() - private val repository = ExampleRepositoryImpl(d2, domainErrorMapper) - - @Test - fun `should map D2Error to domain error`() = runTest { - val d2Error = D2Error.builder().errorCode(D2ErrorCode.API_RESPONSE_PROCESS_ERROR).build() - whenever(d2.exampleModule().examples().blockingGet()).thenThrow(d2Error) - whenever(domainErrorMapper.mapToDomainError(d2Error)).thenReturn(DomainException("error")) - - val result = runCatching { repository.getData() } - - assertTrue(result.isFailure) - verify(domainErrorMapper).mapToDomainError(d2Error) - } -} -``` - -## UI Tests: Robot Pattern - -All UI tests go in `androidInstrumentedTest/`. Always use the Robot pattern. Tests extend -`BaseTest`, which provides `mockWebServerRobot` — a helper that stubs HTTP responses from the -DHIS2 server so tests run fully offline against a local `MockWebServer`. Register stubs -**before** launching the robot body. +KMP modules: `login`, `sync`, `aggregates`, `commonskmm`, `tracker`. +AGP modules: `app`, `form`, `commons`, `compose-table`, `stock-usecase`, +`dhis_android_analytics`, `dhis2_android_maps`, `dhis2-mobile-program-rules`. -```kotlin -fun exampleRobot(rule: ComposeTestRule, body: ExampleRobot.() -> Unit) = - ExampleRobot(rule).apply { body() } +Both KMP source sets run under the **same** task — the split is about what the test +can see, not how it is run. The SDK is an `androidMain` dependency, so a test that +mocks `D2` must be in `androidHostTest`; `androidHostTest` depends on `commonTest`, +so `commonTest` dependencies are already on its classpath. -class ExampleRobot(val rule: ComposeTestRule) : BaseRobot() { - fun typeUsername(username: String) { - rule.waitUntilExactlyOneExists(hasTestTag(USERNAME_TAG), TIMEOUT) - rule.onNodeWithTag(USERNAME_TAG).performClick() - rule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) - } +## These do not exist in this repo - fun clickSubmitButton() { - rule.waitUntilExactlyOneExists(hasTestTag(SUBMIT_TAG), TIMEOUT) - rule.onNodeWithTag(SUBMIT_TAG).performClick() - } -} +Guidance elsewhere on the internet (and older copies of this file) will tell you to +use these. They are wrong **here** — a test placed in one of them is never compiled +and never run, and nothing fails to tell you so: -class ExampleTest : BaseTest() { - @get:Rule val rule = createComposeRule() +- `androidUnitTest/` — does not exist. Use `androidHostTest/`. +- `androidInstrumentedTest/` — does not exist. Use `src/androidTest/`. +- `src/desktopTest/` — no module has one, despite the desktop targets. +- `testAndroidDebugUnitTest` — **not a task.** Use `testAndroidHostTest`. - @Test - fun shouldPerformSuccessfulAction() { - // Stub the network response before any UI interaction - mockWebServerRobot.addResponse(GET, "/api/endpoint", MOCK_RESPONSE, 200) - exampleRobot(rule) { - typeUsername("user") - clickSubmitButton() - verifySuccessMessageDisplayed() - } - // Call cleanDatabase() after any test that writes to the local DB — - // it clears all DHIS2 SDK tables so state doesn't leak into the next test. - cleanDatabase() - } -} -``` - -### What belongs in a robot, and which robot - -- **One robot per screen or dialog.** A dialog reached from several screens gets its - own robot rather than duplicated methods in each caller's robot — - `OrgUnitSelectorRobot` is the existing example. -- **Waiting for a screen belongs to that screen's robot**, not the test. If a step - navigates to a new Activity, expose e.g. `waitForFormToOpen()` on the destination's - robot; the test then reads as intent, and the reason for the wait is documented once - instead of repeated at every call site. -- **Keep framework plumbing out of the test class.** Reaching into - `supportFragmentManager` / `ActivityLifecycleMonitorRegistry` from a `@Test` bypasses - the pattern; assert on what the dialog renders through its robot instead. - -## Test Tags - -Export constants from the screen file. Format: `{SCREEN}_{COMPONENT}_TAG`. - -```kotlin -const val LOGIN_BUTTON_TAG = "LOGIN_BUTTON_TAG" -const val USERNAME_INPUT_TAG = "USERNAME_INPUT_TAG" - -@Composable -fun LoginScreen() { - InputField(modifier = Modifier.testTag(USERNAME_INPUT_TAG)) - Button( - onClick = { /* submit */ }, - modifier = Modifier.testTag(LOGIN_BUTTON_TAG), - ) { - Text("Log in") - } -} -``` - -### Never assume a test tag exists — verify it is emitted first - -A matcher built on a tag that the UI never renders fails silently: it just -times out with no hint that the tag was the problem. Before you write -`hasTestTag("FOO")`, confirm `FOO` is actually set on a node — grep the screen -(and the design-system component source) for `testTag("FOO")`, or dump the tree -with `composeTestRule.onRoot().printToLog("TREE")` and read what's really there. - -This bites hardest with tags that come from the design-system library rather -than app code (e.g. a list-card item tag). If you can't confirm a tag is -emitted, match on **confirmable text or semantics** instead — text you can see -on screen is always safer than a tag you're guessing at. - -### Prefix matchers: check no longer tag shares the prefix +`commonskmm`, `login` and `sync` do declare an `androidDeviceTest` source set, but +no test has been written in one yet. -When matching on a tag *prefix* rather than an exact tag, verify that no longer tag -starts with the same string. `OrgBottomSheet` declares both `ORG_TREE_ITEM_` and -`ORG_TREE_ITEM_CHECKBOX_`, so a `startsWith("ORG_TREE_ITEM_")` matcher also selects -every checkbox. Harmless for an existence check, wrong if you meant "a tree row". +## Run commands -### Merged vs unmerged semantics tree - -Any node with `mergeDescendants = true` collapses its whole subtree in the **merged** -tree — the default for every query. `Modifier.clickable` sets it, so a clickable -`LazyColumn` or a design-system list card merges everything inside it. - -| Symptom | Cause | Fix | -| --- | --- | --- | -| `onAllNodesWithTag(X)` → 0 nodes, but X is on screen | tag absorbed by a merging ancestor | add `useUnmergedTree = true` | -| `performScrollTo()` → "no parent layout with a Scroll SemanticsAction" | matched the scroller, not the child | query unmerged, or scroll the container with `performScrollToNode` | -| a child-counting assertion passes suspiciously | nodes inside a merged subtree report `children == []` — a vacuous pass | read the merged node's aggregated text instead | - -**What makes this look self-contradictory:** *clickable* descendants (radio buttons, -icon buttons) set their own `mergeDescendants` and survive merging, so some merged -lookups work on the same screen where others return nothing. - -**`waitUntilAtLeastOneExists` always searches the merged tree** — it has no unmerged -option. For a tag that exists only unmerged, spell the wait out: - -```kotlin -composeTestRule.waitUntil(TIMEOUT) { - composeTestRule.onAllNodesWithTag(TAG, useUnmergedTree = true) - .fetchSemanticsNodes().isNotEmpty() -} -``` - -An unmerged node carries only its **own** text, so to find a field by its label you -must recurse the subtree — the label often sits several levels below the tagged node. -`BaseRobot` provides `texts()` (own text, merged nodes) and `subtreeTexts()` -(recursive, unmerged) for exactly this split. - -## DHIS2 Design System Inputs - -DHIS2 input components are composite. Click the wrapper to focus, then target the -inner field. Use `performTextInput()`, never `performTextReplacement()`. +```bash +# lint + all unit tests (what CI runs) +./run_tests.sh -The inner-field tag depends on the component. Most text-style inputs follow the -pattern `INPUT__FIELD` — e.g. `InputText` uses `INPUT_TEXT_FIELD`, -`InputEmail` uses `INPUT_EMAIL_FIELD`, `InputNumber` uses `INPUT_NUMBER_FIELD`, -`InputPhoneNumber` uses `INPUT_PHONE_NUMBER_FIELD`, and so on. Non-text inputs -(checkboxes, dropdowns, dialogs, pickers, org-unit, coordinate, etc.) use their -own tag schemes. +# all unit tests +./gradlew testDebugUnitTest testDhis2DebugUnitTest testAndroidHostTest -To find the exact testTag for any design-system component, check the API docs: - -— or open the component source in -`../dhis2-mobile-ui/designsystem/src/commonMain/kotlin/org/hisp/dhis/mobile/ui/designsystem/component/.kt` -and grep for `testTag(`. +# one KMP test class (either source set) +./gradlew :login:testAndroidHostTest --tests "org.dhis2.mobile.login.main.ui.viewmodel.LoginViewModelTest" -```kotlin -// ✅ CORRECT — InputText example -rule.onNodeWithTag(USERNAME_TAG).performClick() -rule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) +# one test method +./gradlew :login:testAndroidHostTest --tests "org.dhis2.mobile.login.main.ui.viewmodel.LoginViewModelTest.initial screen is set correctly when starting" -// ❌ WRONG -rule.onNodeWithTag(USERNAME_TAG).performTextReplacement(username) +# one AGP module test class +./gradlew :form:testDebugUnitTest --tests "org.dhis2.form.ui.FormViewModelTest" ``` -## Instrumented Test State: Fixtures & Assertions - -### Assert through the UI, not the SDK - -In an instrumented test, verify what the **user sees** — visible text, tags, -semantics — not the SDK's internal state. Reaching into -`D2Manager.getD2()…blockingGet()` to check a status couples the test to the -database layer instead of the screen, and no other test in this codebase does -it. If the UI shows "Event completed", assert on that; don't probe -`event.status()`. +## Confirm your test actually ran -```kotlin -// ✅ CORRECT — assert what's on screen -programEventsRobot(composeTestRule) { - checkEventIsComplete(eventDate) -} +A green build does **not** mean your test ran. A class that was never collected — +wrong source set, wrong annotation — produces no result file at all, and the build +succeeds. -// ❌ WRONG — probing SDK state from an instrumented test -val status = D2Manager.getD2().eventModule().events().uid(uid).blockingGet()?.status() -assertTrue(status == EventStatus.COMPLETED) +```bash +ls build/test-results//TEST-.xml ``` -The SDK is still fine for **seeding** a fixture (see below) — the rule is about -*assertions*: check the UI, not the database. - -### Seed fixtures at runtime — don't hardcode demo UIDs - -The test DB is a snapshot; a specific demo event UID like `"ohAH6BXIMad"` can -disappear or change the moment the snapshot is regenerated, breaking the test -for reasons unrelated to the app. Instead, **create the fixture you need at the -start of the test** via the SDK, in an intent helper, and use the UID it -returns: +Check the file exists and its root element says `tests="N"` with the N you expect. +No file means the test never ran. Do this before reporting a test as passing. + +## Invariants + +1. **mockito-kotlin only** — `mock()`, `whenever()`, `verify()`. Never MockK. +2. **JUnit4 / `kotlin.test` annotations only.** JUnit Jupiter is excluded from every + test configuration; an `org.junit.jupiter.api.Test` fails the build. +3. **`D2` must be mocked with `RETURNS_DEEP_STUBS`** — its call chains NPE otherwise. +4. **`DomainError` and `D2Error` need `thenAnswer { throw ... }`**, not `doThrow` — + mockito rejects them as invalid checked exceptions. +5. **No `Thread.sleep()` or hard-coded delays**, in any test. +6. **ViewModels use `launchUseCase { }`**, not `viewModelScope.launch` — it wraps + `CoroutineTracker`, which drives Espresso's `IdlingResource`. +7. **Pass one dispatcher everywhere** — `Dispatcher(testDispatcher, testDispatcher, + testDispatcher)` — and install it with `Dispatchers.setMain`, or the schedulers + diverge. +8. **Test both SDK error shapes** — the blocking RxJava operators rewrap `D2Error` + in a `RuntimeException`. +9. **Never resolve a UI string inline in domain/data code** — it cannot resolve in a + host test. Inject a `*ResourceProvider`. +10. **Assert through the UI in instrumented tests**, never by probing SDK state. + +## References + +- **[references/unit-testing.md](references/unit-testing.md)** — host (JVM) tests: + source sets and Turbine availability per module, coroutine/dispatcher setup, + Turbine and `StateFlow` semantics, mocking, SDK error mapping, `DomainError` + pitfalls, resource providers, multi-step use cases, and a symptom → cause → fix + troubleshooting table. +- **[references/instrumented-testing.md](references/instrumented-testing.md)** — + device tests: Robot pattern, test tags, merged vs unmerged semantics, + design-system inputs, fixtures and cleanup, and the landscape/CI-matrix rules. + +## Keeping this true + +If a test problem costs you more than about 30 minutes, add a row to the +troubleshooting table in `references/unit-testing.md` (or the relevant section of +`references/instrumented-testing.md`) **in the same PR that fixes it**. The tables +are the point of this skill; a lesson learned and not written down will be paid for +again. + +To check that no document has drifted back to teaching the nonexistent source sets, +run the gate — it allows lines that *deny* the names and flags every other use: -```kotlin -// In EventIntents.kt — create a fresh event, return its UID + display date -fun createFreshFlowAEvent(): FreshFlowAEvent { - val uid = d2.eventModule().events().blockingAdd( - EventCreateProjection.builder() - .program(FLOW_A_PROGRAM_UID) // anchor to the stable program… - .programStage(FLOW_A_STAGE_UID) - .organisationUnit(FLOW_A_ORG_UNIT_UID) - .build(), - ) - d2.eventModule().events().uid(uid).setEventDate(now) - return FreshFlowAEvent(uid, displayDate) // …generate the fragile event yourself -} +```bash +.claude/skills/android-testing/check-stale-tokens.sh ``` -You still depend on the **program** existing (a big, stable structural object), -but you generate the **event** (the fragile row) yourself — so a DB refresh -can't pull the rug out. Prefer this over hardcoded demo UIDs for any test that -needs a specific event/enrollment to act on. - -### Tests run isolated per class — but state leaks within one run - -CI runs each test class in its own instrumentation process, so each class -starts from the fresh DB snapshot. But within a **single** `connectedAndroidTest` -invocation that spans multiple classes, SDK writes persist across tests — a -fixture one test seeds (or a status it changes) is still there for the next -test. Two consequences: - -- A multi-class local run can fail a later test that a single-class run passes - (stale state, not a real bug). Reproduce CI by running one class at a time. -- When a seeded fixture coexists with demo data, clean up with - `cleanDatabase()` where the next test needs a pristine list. - -## Write for the CI device matrix — including landscape - -Tests run on the BrowserStack device matrix (multiple devices **and -orientations**), not just your local emulator. A test that passes locally in -portrait can fail on CI in landscape — almost always because landscape has far -less vertical height, so a node that was on-screen in portrait is now scrolled -out of the viewport. Compose reports such a node as **present but not -displayed**, so `assertIsDisplayed()` fails (and `performClick()` may miss) -even though the element exists and the app is fine. - -Make assertions orientation-independent: - -- **Scroll the target into view before asserting or clicking.** Call - `performScrollTo()` (Compose) / `scrollTo()` (Espresso) on the node first. - - ```kotlin - // ✅ robust in any orientation — bring it on-screen, then assert - composeTestRule.onNodeWithText(orgUnit).performScrollTo().assertIsDisplayed() - - // ❌ portrait-only — fails in landscape when the node is below the fold - composeTestRule.onNodeWithText(orgUnit).assertIsDisplayed() - ``` - -- **`performScrollTo()` only works if the node has a Compose scroll ancestor** - (`verticalScroll`, `LazyColumn`, …). It is not a free safety net: with no such - ancestor it throws *"no parent layout with a Scroll SemanticsAction"* on **every** - device, portrait included. `BottomSheetDialogContent` has no scroll container — the - sheet's drag is View-level `BottomSheetBehavior`, invisible to Compose semantics — so - nothing inside a bottom sheet can be scrolled to. Check the component before - applying the rule above; where there is no scroller, use `assertExists()`. -- **Match the assertion to the claim.** "Did this screen/dialog open" is an - *existence* claim → `assertExists()`. "Can the user see or act on this" is a - *visibility* claim → `assertIsDisplayed()`, scrolled into view first. -- **When you only need to prove a node is in the tree** (not that it's - visible right now), use `assertExists()` instead of `assertIsDisplayed()`. -- **Don't assume layout positions.** Toolbars, FABs, and bottom sheets reflow - in landscape; the soft keyboard can also go fullscreen (extract mode) and - cover the form. Target nodes by tag/text and scroll to them rather than - relying on where they sit in portrait. -- **"Green locally" ≠ "green on CI".** Don't declare a flow done on a local - portrait run alone — the matrix exercises orientations your emulator didn't. - -## Common Mistakes to Avoid - -- Using `Thread.sleep()` or any hard-coded delays -- Using MockK (`mockk()`, `every {}`, `coEvery {}`) — use mockito-kotlin instead -- Using `performTextReplacement()` on DHIS2 design system components -- Not exporting test tag constants from screen files -- Not extending `BaseRobot` for robot classes -- Not cleaning up after tests (`cleanDatabase()`, clear preferences) -- Testing implementation details instead of user flows -- Probing SDK state (`D2Manager…blockingGet()`) to assert in an instrumented - test instead of checking what's on screen -- Building a matcher on a test tag you haven't confirmed is emitted (especially - design-system tags) — verify or match on text instead -- Matching a tag *prefix* without checking whether a longer tag shares it -- Querying the merged tree for a tag inside a merging container (returns 0 nodes), - or using `waitUntilAtLeastOneExists` for an unmerged-only tag -- Adding `performScrollTo()` to a node with no Compose scroll ancestor — it throws - rather than hardening the assertion -- Hardcoding demo fixture UIDs instead of seeding the fixture at runtime via - the SDK -- Asserting `assertIsDisplayed()` / clicking without `performScrollTo()` first — - fails in landscape on the CI matrix when the node is below the fold +The testing guidance lives here and only here. `.github/agents/testing.agent.md` and +the Testing section of `AGENTS.md` are deliberately thin pointers — put new +guidance in this skill, not in those. diff --git a/.claude/skills/android-testing/check-stale-tokens.sh b/.claude/skills/android-testing/check-stale-tokens.sh new file mode 100755 index 00000000000..5bb9996ab39 --- /dev/null +++ b/.claude/skills/android-testing/check-stale-tokens.sh @@ -0,0 +1,25 @@ +#!/bin/bash +# Fails if any document teaches a source set or Gradle task that does not exist here. +# +# Lines that DENY the names are allowed -- SKILL.md has to be able to say +# " does not exist" without tripping this check. Everything else is a hit. +# +# This script lives in .sh deliberately: the scan covers .md/.kts/.yml, so keeping +# the patterns here stops the gate from matching its own source. +set -uo pipefail +cd "$(dirname "$0")/../../.." + +hits=$(grep -rn "androidUnitTest\|androidInstrumentedTest\|testAndroidDebugUnitTest" \ + --include="*.md" --include="*.kts" --include="*.yml" . \ + | grep -v "/build/" \ + | grep -vE "does not exist|not a task" || true) + +if [ -n "$hits" ]; then + echo "Stale source-set or task names still taught as guidance:" + echo "$hits" + echo + echo "Use: androidHostTest/ (not androidUnitTest/), src/androidTest/ (not" + echo "androidInstrumentedTest/), testAndroidHostTest (not testAndroidDebugUnitTest)." + exit 1 +fi +echo "OK: no document teaches a nonexistent source set or task." diff --git a/.claude/skills/android-testing/references/instrumented-testing.md b/.claude/skills/android-testing/references/instrumented-testing.md new file mode 100644 index 00000000000..8e233e4d4a0 --- /dev/null +++ b/.claude/skills/android-testing/references/instrumented-testing.md @@ -0,0 +1,325 @@ +# Instrumented (device) tests + +Compose Testing + Espresso, Robot pattern, run on the BrowserStack device matrix. + +**Where they live:** `src/androidTest/java` — in `app`, `commons`, `compose-table` and +`form`. Those four are the only modules with instrumented tests today. + +`commonskmm`, `login` and `sync` declare an `androidDeviceTest` source set in their +`build.gradle.kts`, but **no test has been written in one yet**. If you are adding the +first device test to a KMP module, that is the source set to use — expect to wire up +the harness (`BaseTest`, MockWebServer, fixtures) from scratch, none of it is there. + +For host (JVM) unit tests, see [unit-testing.md](unit-testing.md). + +--- + +## Critical Rule: No Hard-Coded Delays + +ViewModels use `launchUseCase { }` which wraps `CoroutineTracker`. Espresso's +`IdlingResource` automatically waits for tracked coroutines. `Thread.sleep()` and +hard-coded timeouts are **forbidden**. + +```kotlin +// ✅ CORRECT — IdlingResource waits automatically +@Test +fun shouldLoadData() { + exampleRobot(composeTestRule) { + clickLoadButton() + verifyDataDisplayed() // no delay needed + } +} + +// ❌ WRONG +@Test +fun shouldLoadData() { + clickLoadButton() + Thread.sleep(2000) // FORBIDDEN + verifyDataDisplayed() +} +``` + +## UI Tests: Robot Pattern + +All UI tests go in `src/androidTest/`. Always use the Robot pattern. Tests extend +`BaseTest`, which provides `mockWebServerRobot` — a helper that stubs HTTP responses from the +DHIS2 server so tests run fully offline against a local `MockWebServer`. Register stubs +**before** launching the robot body. + +```kotlin +fun exampleRobot(rule: ComposeTestRule, body: ExampleRobot.() -> Unit) = + ExampleRobot(rule).apply { body() } + +class ExampleRobot(val rule: ComposeTestRule) : BaseRobot() { + fun typeUsername(username: String) { + rule.waitUntilExactlyOneExists(hasTestTag(USERNAME_TAG), TIMEOUT) + rule.onNodeWithTag(USERNAME_TAG).performClick() + rule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) + } + + fun clickSubmitButton() { + rule.waitUntilExactlyOneExists(hasTestTag(SUBMIT_TAG), TIMEOUT) + rule.onNodeWithTag(SUBMIT_TAG).performClick() + } +} + +class ExampleTest : BaseTest() { + @get:Rule val rule = createComposeRule() + + @Test + fun shouldPerformSuccessfulAction() { + // Stub the network response before any UI interaction + mockWebServerRobot.addResponse(GET, "/api/endpoint", MOCK_RESPONSE, 200) + exampleRobot(rule) { + typeUsername("user") + clickSubmitButton() + verifySuccessMessageDisplayed() + } + // Call cleanDatabase() after any test that writes to the local DB — + // it clears all DHIS2 SDK tables so state doesn't leak into the next test. + cleanDatabase() + } +} +``` + +### What belongs in a robot, and which robot + +- **One robot per screen or dialog.** A dialog reached from several screens gets its + own robot rather than duplicated methods in each caller's robot — + `OrgUnitSelectorRobot` is the existing example. +- **Waiting for a screen belongs to that screen's robot**, not the test. If a step + navigates to a new Activity, expose e.g. `waitForFormToOpen()` on the destination's + robot; the test then reads as intent, and the reason for the wait is documented once + instead of repeated at every call site. +- **Keep framework plumbing out of the test class.** Reaching into + `supportFragmentManager` / `ActivityLifecycleMonitorRegistry` from a `@Test` bypasses + the pattern; assert on what the dialog renders through its robot instead. + +## Test Tags + +Export constants from the screen file. Format: `{SCREEN}_{COMPONENT}_TAG`. + +```kotlin +const val LOGIN_BUTTON_TAG = "LOGIN_BUTTON_TAG" +const val USERNAME_INPUT_TAG = "USERNAME_INPUT_TAG" + +@Composable +fun LoginScreen() { + InputField(modifier = Modifier.testTag(USERNAME_INPUT_TAG)) + Button( + onClick = { /* submit */ }, + modifier = Modifier.testTag(LOGIN_BUTTON_TAG), + ) { + Text("Log in") + } +} +``` + +### Never assume a test tag exists — verify it is emitted first + +A matcher built on a tag that the UI never renders fails silently: it just +times out with no hint that the tag was the problem. Before you write +`hasTestTag("FOO")`, confirm `FOO` is actually set on a node — grep the screen +(and the design-system component source) for `testTag("FOO")`, or dump the tree +with `composeTestRule.onRoot().printToLog("TREE")` and read what's really there. + +This bites hardest with tags that come from the design-system library rather +than app code (e.g. a list-card item tag). If you can't confirm a tag is +emitted, match on **confirmable text or semantics** instead — text you can see +on screen is always safer than a tag you're guessing at. + +### Prefix matchers: check no longer tag shares the prefix + +When matching on a tag *prefix* rather than an exact tag, verify that no longer tag +starts with the same string. `OrgBottomSheet` declares both `ORG_TREE_ITEM_` and +`ORG_TREE_ITEM_CHECKBOX_`, so a `startsWith("ORG_TREE_ITEM_")` matcher also selects +every checkbox. Harmless for an existence check, wrong if you meant "a tree row". + +### Merged vs unmerged semantics tree + +Any node with `mergeDescendants = true` collapses its whole subtree in the **merged** +tree — the default for every query. `Modifier.clickable` sets it, so a clickable +`LazyColumn` or a design-system list card merges everything inside it. + +| Symptom | Cause | Fix | +| --- | --- | --- | +| `onAllNodesWithTag(X)` → 0 nodes, but X is on screen | tag absorbed by a merging ancestor | add `useUnmergedTree = true` | +| `performScrollTo()` → "no parent layout with a Scroll SemanticsAction" | matched the scroller, not the child | query unmerged, or scroll the container with `performScrollToNode` | +| a child-counting assertion passes suspiciously | nodes inside a merged subtree report `children == []` — a vacuous pass | read the merged node's aggregated text instead | + +**What makes this look self-contradictory:** *clickable* descendants (radio buttons, +icon buttons) set their own `mergeDescendants` and survive merging, so some merged +lookups work on the same screen where others return nothing. + +**`waitUntilAtLeastOneExists` always searches the merged tree** — it has no unmerged +option. For a tag that exists only unmerged, spell the wait out: + +```kotlin +composeTestRule.waitUntil(TIMEOUT) { + composeTestRule.onAllNodesWithTag(TAG, useUnmergedTree = true) + .fetchSemanticsNodes().isNotEmpty() +} +``` + +An unmerged node carries only its **own** text, so to find a field by its label you +must recurse the subtree — the label often sits several levels below the tagged node. +`BaseRobot` provides `texts()` (own text, merged nodes) and `subtreeTexts()` +(recursive, unmerged) for exactly this split. + +## DHIS2 Design System Inputs + +DHIS2 input components are composite. Click the wrapper to focus, then target the +inner field. Use `performTextInput()`, never `performTextReplacement()`. + +The inner-field tag depends on the component. Most text-style inputs follow the +pattern `INPUT__FIELD` — e.g. `InputText` uses `INPUT_TEXT_FIELD`, +`InputEmail` uses `INPUT_EMAIL_FIELD`, `InputNumber` uses `INPUT_NUMBER_FIELD`, +`InputPhoneNumber` uses `INPUT_PHONE_NUMBER_FIELD`, and so on. Non-text inputs +(checkboxes, dropdowns, dialogs, pickers, org-unit, coordinate, etc.) use their +own tag schemes. + +To find the exact testTag for any design-system component, check the API docs: + +— or open the component source in +`../dhis2-mobile-ui/designsystem/src/commonMain/kotlin/org/hisp/dhis/mobile/ui/designsystem/component/.kt` +and grep for `testTag(`. + +```kotlin +// ✅ CORRECT — InputText example +rule.onNodeWithTag(USERNAME_TAG).performClick() +rule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) + +// ❌ WRONG +rule.onNodeWithTag(USERNAME_TAG).performTextReplacement(username) +``` + +## Instrumented Test State: Fixtures & Assertions + +### Assert through the UI, not the SDK + +In an instrumented test, verify what the **user sees** — visible text, tags, +semantics — not the SDK's internal state. Reaching into +`D2Manager.getD2()…blockingGet()` to check a status couples the test to the +database layer instead of the screen, and no other test in this codebase does +it. If the UI shows "Event completed", assert on that; don't probe +`event.status()`. + +```kotlin +// ✅ CORRECT — assert what's on screen +programEventsRobot(composeTestRule) { + checkEventIsComplete(eventDate) +} + +// ❌ WRONG — probing SDK state from an instrumented test +val status = D2Manager.getD2().eventModule().events().uid(uid).blockingGet()?.status() +assertTrue(status == EventStatus.COMPLETED) +``` + +The SDK is still fine for **seeding** a fixture (see below) — the rule is about +*assertions*: check the UI, not the database. + +### Seed fixtures at runtime — don't hardcode demo UIDs + +The test DB is a snapshot; a specific demo event UID like `"ohAH6BXIMad"` can +disappear or change the moment the snapshot is regenerated, breaking the test +for reasons unrelated to the app. Instead, **create the fixture you need at the +start of the test** via the SDK, in an intent helper, and use the UID it +returns: + +```kotlin +// In EventIntents.kt — create a fresh event, return its UID + display date +fun createFreshFlowAEvent(): FreshFlowAEvent { + val uid = d2.eventModule().events().blockingAdd( + EventCreateProjection.builder() + .program(FLOW_A_PROGRAM_UID) // anchor to the stable program… + .programStage(FLOW_A_STAGE_UID) + .organisationUnit(FLOW_A_ORG_UNIT_UID) + .build(), + ) + d2.eventModule().events().uid(uid).setEventDate(now) + return FreshFlowAEvent(uid, displayDate) // …generate the fragile event yourself +} +``` + +You still depend on the **program** existing (a big, stable structural object), +but you generate the **event** (the fragile row) yourself — so a DB refresh +can't pull the rug out. Prefer this over hardcoded demo UIDs for any test that +needs a specific event/enrollment to act on. + +### Tests run isolated per class — but state leaks within one run + +CI runs each test class in its own instrumentation process, so each class +starts from the fresh DB snapshot. But within a **single** `connectedAndroidTest` +invocation that spans multiple classes, SDK writes persist across tests — a +fixture one test seeds (or a status it changes) is still there for the next +test. Two consequences: + +- A multi-class local run can fail a later test that a single-class run passes + (stale state, not a real bug). Reproduce CI by running one class at a time. +- When a seeded fixture coexists with demo data, clean up with + `cleanDatabase()` where the next test needs a pristine list. + +## Write for the CI device matrix — including landscape + +Tests run on the BrowserStack device matrix (multiple devices **and +orientations**), not just your local emulator. A test that passes locally in +portrait can fail on CI in landscape — almost always because landscape has far +less vertical height, so a node that was on-screen in portrait is now scrolled +out of the viewport. Compose reports such a node as **present but not +displayed**, so `assertIsDisplayed()` fails (and `performClick()` may miss) +even though the element exists and the app is fine. + +Make assertions orientation-independent: + +- **Scroll the target into view before asserting or clicking.** Call + `performScrollTo()` (Compose) / `scrollTo()` (Espresso) on the node first. + + ```kotlin + // ✅ robust in any orientation — bring it on-screen, then assert + composeTestRule.onNodeWithText(orgUnit).performScrollTo().assertIsDisplayed() + + // ❌ portrait-only — fails in landscape when the node is below the fold + composeTestRule.onNodeWithText(orgUnit).assertIsDisplayed() + ``` + +- **`performScrollTo()` only works if the node has a Compose scroll ancestor** + (`verticalScroll`, `LazyColumn`, …). It is not a free safety net: with no such + ancestor it throws *"no parent layout with a Scroll SemanticsAction"* on **every** + device, portrait included. `BottomSheetDialogContent` has no scroll container — the + sheet's drag is View-level `BottomSheetBehavior`, invisible to Compose semantics — so + nothing inside a bottom sheet can be scrolled to. Check the component before + applying the rule above; where there is no scroller, use `assertExists()`. +- **Match the assertion to the claim.** "Did this screen/dialog open" is an + *existence* claim → `assertExists()`. "Can the user see or act on this" is a + *visibility* claim → `assertIsDisplayed()`, scrolled into view first. +- **When you only need to prove a node is in the tree** (not that it's + visible right now), use `assertExists()` instead of `assertIsDisplayed()`. +- **Don't assume layout positions.** Toolbars, FABs, and bottom sheets reflow + in landscape; the soft keyboard can also go fullscreen (extract mode) and + cover the form. Target nodes by tag/text and scroll to them rather than + relying on where they sit in portrait. +- **"Green locally" ≠ "green on CI".** Don't declare a flow done on a local + portrait run alone — the matrix exercises orientations your emulator didn't. + + +## Common mistakes — instrumented + +- Using `Thread.sleep()` or any hard-coded delays +- Using `performTextReplacement()` on DHIS2 design system components +- Not exporting test tag constants from screen files +- Not extending `BaseRobot` for robot classes +- Not cleaning up after tests (`cleanDatabase()`, clear preferences) +- Testing implementation details instead of user flows +- Probing SDK state (`D2Manager…blockingGet()`) to assert in an instrumented + test instead of checking what's on screen +- Building a matcher on a test tag you haven't confirmed is emitted (especially + design-system tags) — verify or match on text instead +- Matching a tag *prefix* without checking whether a longer tag shares it +- Querying the merged tree for a tag inside a merging container (returns 0 nodes), + or using `waitUntilAtLeastOneExists` for an unmerged-only tag +- Adding `performScrollTo()` to a node with no Compose scroll ancestor — it throws + rather than hardening the assertion +- Hardcoding demo fixture UIDs instead of seeding the fixture at runtime via + the SDK +- Asserting `assertIsDisplayed()` / clicking without `performScrollTo()` first — + fails in landscape on the CI matrix when the node is below the fold diff --git a/.claude/skills/android-testing/references/unit-testing.md b/.claude/skills/android-testing/references/unit-testing.md new file mode 100644 index 00000000000..f8046749450 --- /dev/null +++ b/.claude/skills/android-testing/references/unit-testing.md @@ -0,0 +1,270 @@ +# Host (JVM) unit tests + +Everything here runs on the JVM, with no device and no Robolectric. For device +tests see [instrumented-testing.md](instrumented-testing.md). + +--- + +## 1. Where a unit test lives + +| Module | Source set | Gradle task | +| --- | --- | --- | +| KMP (`login`, `sync`, `aggregates`, `commonskmm`, `tracker`) — pure logic | `src/commonTest/kotlin` | `:mod:testAndroidHostTest` | +| KMP — anything touching `org.hisp.dhis.android.core.*` | `src/androidHostTest/kotlin` | `:mod:testAndroidHostTest` | +| `app` | `src/test/java` | `:app:testDhis2DebugUnitTest` | +| Other AGP modules (`form`, `commons`, `compose-table`, `stock-usecase`, `dhis_android_analytics`, `dhis2_android_maps`, `dhis2-mobile-program-rules`) | `src/test/java` | `:mod:testDebugUnitTest` | + +Both KMP source sets are driven by the **same** task. `androidHostTest` depends on +`commonTest`, so a dependency declared in `commonTest` is already on the +`androidHostTest` compile classpath — you do not need to declare it twice. +(`tracker` declares Turbine in both; that is redundant, not a requirement.) + +The split is about **what the test can see**, not which task runs it: the DHIS2 SDK +is an `androidMain` dependency, so a test that mocks `D2` must live in +`androidHostTest`. Pure domain and ViewModel tests belong in `commonTest`. + +**Turbine is not available everywhere.** Verified per module: + +| Module | Turbine declared in | Usable from `commonTest`? | +| --- | --- | --- | +| `login` | `commonTest` | yes | +| `aggregates` | `commonTest` | yes | +| `tracker` | `commonTest` + `androidHostTest` | yes | +| `sync` | `androidHostTest` **only** | **no** — add `implementation(libs.test.turbine)` to `commonTest` | +| `commonskmm` | nowhere | **no** — add it to the source set you need | + +## 1b. Canonical files to copy from + +Rather than reproduce skeletons that drift, start from a real test in this repo: + +| Kind | File | +| --- | --- | +| Use case | `login/src/commonTest/.../pin/domain/usecase/ValidatePinUseCaseTest.kt` | +| ViewModel (Turbine + `StateFlow`) | `login/src/commonTest/.../main/ui/viewmodel/LoginViewModelTest.kt` | +| Repository mocking `D2` | `login/src/androidHostTest/.../main/data/LoginRepositoryImplTest.kt` | +| JUnit4 `@get:Rule` in `commonTest` | `aggregates/src/commonTest/.../GetDataValueInputTest.kt` | + +The repository one is the densest: it shows `setMain`/`resetMain`, the `Dispatcher` +triple, `RETURNS_DEEP_STUBS` and `thenAnswer { throw ... }` together. + +## 2. Framework: JUnit4 + kotlin.test + +Prefer `kotlin.test` annotations (`@Test`, `@BeforeTest`, `@AfterTest`, +`assertEquals`, `assertTrue`, `assertFailsWith`) — they compile in `commonTest` and +`androidHostTest` alike. JUnit4 annotations (`org.junit.Test`, `@Before`, `@Rule`) +are accepted, and are required when you need a `@get:Rule` (see `KoinTestRule` in +`aggregates/src/commonTest/.../GetDataValueInputTest.kt`). + +**JUnit Jupiter is banned.** `org.junit.jupiter` is excluded from every test +configuration in the root `build.gradle.kts`, and `useJUnitPlatform()` is set +nowhere. Before that exclusion existed, a Jupiter `@Test` was **silently never +collected** — the class compiled, the build went green, and the test never ran. +Now it fails the build instead. Use `kotlin.test.assertFailsWith { }`, never +`org.junit.jupiter.api.assertThrows`. + +## 3. Coroutines + +**Canonical file to copy from:** +`login/src/androidHostTest/kotlin/org/dhis2/mobile/login/main/data/LoginRepositoryImplTest.kt` +— it shows `setMain`/`resetMain`, the `Dispatcher` triple, `RETURNS_DEEP_STUBS`, and +`thenAnswer { throw ... }` in one place. + +A `TestDispatcher` held as a class field does **not** share `runTest`'s +`TestCoroutineScheduler`. Symptom: + +``` +IllegalStateException: Detected use of different schedulers +``` + +Two fixes, either is fine: + +```kotlin +// A — install the field dispatcher as Main; runTest then reuses its scheduler +private val testDispatcher = StandardTestDispatcher() + +@BeforeTest fun setUp() { Dispatchers.setMain(testDispatcher) } +@AfterTest fun tearDown() { Dispatchers.resetMain() } + +// B — build the dispatcher inside runTest, from the test's own scheduler +runTest { + val dispatcher = StandardTestDispatcher(testScheduler) +} +``` + +- `StandardTestDispatcher` queues work — you must `advanceUntilIdle()` or + `runCurrent()` to let it run. Use it when ordering matters. +- `UnconfinedTestDispatcher` runs eagerly at the launch point. Use it when you just + want the coroutine to have finished by the next line. + +KMP classes take `org.dhis2.mobile.commons.coroutine.Dispatcher`, a data class of +three dispatchers (`io`, `main`, `default`). In a test, pass the same test +dispatcher for all three so everything shares one clock: + +```kotlin +dispatcher = Dispatcher(testDispatcher, testDispatcher, testDispatcher) +``` + +## 4. Turbine and `StateFlow` + +`StateFlow` replays its current value to every new collector, so **the first +`awaitItem()` is the state at subscription time**, not the first change. If you +subscribe after the work has already run, that first item is also the last one. + +```kotlin +viewModel.uiState.test { + assertEquals(UiState.Loading, awaitItem()) // replayed initial value + assertEquals(UiState.Success(data), awaitItem()) + cancelAndIgnoreRemainingEvents() +} +``` + +- `StateFlow` **conflates**: intermediate states can be dropped if the producer + outruns the collector. Don't assert on a state the flow only passes through. +- `expectMostRecentItem()` when you only care about where it settled. +- `cancelAndIgnoreRemainingEvents()` at the end of a block that doesn't drain. +- Check §1 before reaching for Turbine — two modules don't have it yet. + +## 5. Mocking: mockito-kotlin only + +`mock()`, `whenever()`, `verify()` from `org.mockito.kotlin`. **Never MockK** — +no `mockk()`, `every {}`, `coEvery {}`. + +**`D2` needs `RETURNS_DEEP_STUBS`.** Its API is a chain of intermediate objects +(`d2.userModule().accountManager().getAccounts()`), and a plain mock returns `null` +at the first link, so the chain NPEs before your stub is reached. + +```kotlin +private val d2: D2 = Mockito.mock(D2::class.java, Mockito.RETURNS_DEEP_STUBS) +``` + +**`doThrow` fails on `DomainError`.** `DomainError` is declared +`sealed class DomainError : Throwable()`. Mockito checks a throwable against the +method's declared checked exceptions and rejects it: + +``` +Checked exception is invalid for this method +``` + +Stub with `thenAnswer` instead — it bypasses that check: + +```kotlin +// ❌ fails at runtime +whenever(repository.logout()).doThrow(domainError) + +// ✅ +whenever(repository.logout()).thenAnswer { throw domainError } +``` + +The same applies to `D2Error`, which is also a checked exception. + +## 6. SDK errors: two shapes, test both + +The SDK's blocking RxJava operators (`blockingGet()`, `blockingAdd()`, …) rewrap +the checked `D2Error` in a `RuntimeException`. So an inline +`catch (d2Error: D2Error)` in a repository **misses the blocking-call case +entirely** — the error sails past the mapper and reaches the ViewModel unmapped. + +Map with `withDomainErrors { }` / `withDomainErrorsAsResult { }` from +`commonskmm/src/androidMain/kotlin/org/dhis2/mobile/commons/error/DomainErrorMapperExtensions.kt`. +They unwrap the cause chain (`Throwable.asD2Error()` walks causes up to a depth of +5, with a self-reference guard) before mapping, so both shapes are handled. + +They are extensions on `DomainErrorMapper`, so the call reads +`domainErrorMapper.withDomainErrors { ... }`. `SyncDataSetRepositoryImpl` is a +worked example. + +Whichever you use, a repository test must cover **both** shapes: + +```kotlin +whenever(...).thenAnswer { throw d2Error } // direct +whenever(...).thenAnswer { throw RuntimeException(d2Error) } // blocking-wrapped +``` + +A test that only covers the direct shape will pass against a repository that is +broken for every blocking call it makes. + +## 7. `DomainError` in production code: `catch (e: Exception)` does not catch it + +Because `DomainError` extends `Throwable` rather than `Exception`, this is a silent +hole: + +```kotlin +try { + sessionRepository.logout() // throws DomainError + Result.success(Unit) +} catch (e: Exception) { // ← does not match DomainError + Result.failure(e) +} +``` + +The error escapes the `try` entirely and propagates to the caller, which was +promised a `Result`. Catch `Throwable` (rethrowing `CancellationException`) when a +`DomainError` can reach the block. `ForgotPinUseCase` and `SavePinUseCase` in +`login` currently have this shape — treat them as the counter-example, not the +pattern. + +## 8. Compose resources cannot resolve in a host test + +There is no Robolectric in this project's host tests, so `getString` reaches +`Resources.getSystem()` and a `Res.string.*` lookup has nothing to resolve against. +This is a **design rule, not a test trick**: + +> A branch you need to test must not resolve a UI string inline. + +Put the string behind an injectable provider, which the test then mocks: +`D2ErrorMessageProvider`, `StringResourceProvider` (both +`commonskmm/src/commonMain/.../resources/`), `CredentialsResourceProvider` +(`login/src/commonMain/.../ui/provider/`). + +Review heuristic: **`getString(` under `domain/` or `data/` is a smell.** If you hit +an unresolved-resource failure while testing, the fix is usually in the production +class, not the test. + +## 9. Multi-step use cases: one test per failing step + +When a repository reports failure as a returned `Result` rather than by throwing, a +use case that inspects only its **own** final `Result` silently swallows the +intermediate failures: + +```kotlin +override suspend fun invoke(input: Unit): Result = try { + repo.stepA() // returns Result — failure ignored + repo.stepB() // returns Result — failure ignored + Result.success(Unit) +} catch (e: Exception) { Result.failure(e) } +``` + +Every step "succeeds" as far as the caller can tell. Write **one test per step**, +each stubbing that step to fail and asserting the use case surfaces it — a single +happy-path test plus a single "repository throws" test will not catch this. + +## 10. Adding tests, naming, and what not to test + +**First test in a module?** Check the module's `build.gradle.kts` has the test +dependencies for the source set you're using — `kotlin("test")`, +`libs.test.kotlinCoroutines`, `libs.test.mockitoKotlin`, and `libs.test.turbine` if +you assert on flows (§1). Copy the block from `login/build.gradle.kts`. + +**Naming.** New tests use backticked `GIVEN … WHEN … THEN …`: + +```kotlin +@Test +fun `GIVEN a stored pin WHEN validating a wrong pin THEN result is failure`() = runTest { +``` + +**What not to unit test:** composables (no Robolectric — those are instrumented +tests), generated code, Koin module wiring, and plain data-class accessors. A test +that only re-states a mock's stub verifies nothing. + +## 11. Troubleshooting + +| Symptom | Cause | Fix | +| --- | --- | --- | +| `IllegalStateException: Detected use of different schedulers` | field `TestDispatcher` doesn't share `runTest`'s scheduler | `Dispatchers.setMain(testDispatcher)` in `@BeforeTest`, or build the dispatcher from `testScheduler` inside `runTest` (§3) | +| No `TEST-*.xml` in `build/test-results//`, build still green | class was never collected — usually a Jupiter `@Test`, or the file is in a source set the task doesn't own | check the annotation import and the source set (§1, §2) | +| `Checked exception is invalid for this method` | `doThrow` with `DomainError` / `D2Error` | `whenever(...).thenAnswer { throw error }` (§5) | +| NPE partway through `d2.x().y().z()` | `D2` mocked without deep stubs | `Mockito.mock(D2::class.java, Mockito.RETURNS_DEEP_STUBS)` (§5) | +| `awaitItem()` times out | flow never emits — a dispatcher never ran (`StandardTestDispatcher` without `advanceUntilIdle()`), or the collector subscribed to the wrong flow | `advanceUntilIdle()`, or switch to `UnconfinedTestDispatcher` (§3) | +| First `awaitItem()` is the *old* state | `StateFlow` replays its current value at subscription | expect the initial value first, or use `expectMostRecentItem()` (§4) | +| Missing-resource error resolving `Res.string.*` | UI string resolved inline in domain/data code | inject a `*ResourceProvider` and mock it (§8) | +| Mapper never called for an SDK failure | the SDK's blocking operators wrapped `D2Error` in `RuntimeException` | map via `withDomainErrors`; test the wrapped shape too (§6) | diff --git a/.claude/skills/sentry-fix/SKILL.md b/.claude/skills/sentry-fix/SKILL.md index 1c6f297b091..94588fb4bd4 100644 --- a/.claude/skills/sentry-fix/SKILL.md +++ b/.claude/skills/sentry-fix/SKILL.md @@ -220,8 +220,9 @@ Load the `android-testing` skill for full patterns. At minimum write: **Placement**: - `commonTest/` — for classes in `commonMain` -- `androidUnitTest/` — for classes in `androidMain` -- Existing module test source set — for legacy Android modules (`form`, `commons`, `tracker`, `app`) +- `androidHostTest/` — for classes in `androidMain` (e.g. anything mocking `D2`) +- Existing module test source set (`src/test/java`) — for AGP modules (`form`, + `commons`, `app`, …). Note `tracker` is KMP, not legacy - **SDK** — `core/src/test/java/`, class named `Should`, mirroring the neighboring tests of the touched class - **Design system** — put pure-Kotlin tests next to the existing tests of the @@ -244,13 +245,13 @@ inside the worktree from Step 2). Fix any failures before moving on. ./gradlew ktlintCheck # 3. Run tests for the affected module -# KMP module (commonTest source set): +# KMP module — runs BOTH the commonTest and androidHostTest source sets: ./gradlew ::testAndroidHostTest -# KMP module (androidUnitTest source set): -./gradlew ::testAndroidDebugUnitTest +# app (it has flavours): +./gradlew :app:testDhis2DebugUnitTest -# Legacy Android module: +# Other AGP modules: ./gradlew ::testDebugUnitTest ``` diff --git a/.claude/skills/sentry-triage/references/repo-map.md b/.claude/skills/sentry-triage/references/repo-map.md index 4726efdf2af..dd483cbca0f 100644 --- a/.claude/skills/sentry-triage/references/repo-map.md +++ b/.claude/skills/sentry-triage/references/repo-map.md @@ -74,8 +74,9 @@ the evidence disagrees — and says so explicitly when that happens. - Conventions: `AGENTS.md` (launchUseCase, DomainErrorMapper, KMP placement, ktlint, testing rules). - Lint/tests: `./gradlew ktlintFormat ktlintCheck` + per-module test task - (`testDebugUnitTest` legacy modules · `testAndroidHostTest` KMP commonTest · - `testAndroidDebugUnitTest` KMP androidUnitTest). + (`testDebugUnitTest` AGP modules · `testDhis2DebugUnitTest` for `app` · + `testAndroidHostTest` for KMP modules, covering both their `commonTest` and + `androidHostTest` source sets). - Module mapping for `org.dhis2.*` frames: | Package prefix | Source root | @@ -123,8 +124,8 @@ the evidence disagrees — and says so explicitly when that happens. merge to `develop`; `main` only receives release PRs. - Module `:designsystem`, package root `org.hisp.dhis.mobile.ui.designsystem`; KMP source sets `commonMain`/`androidMain`/`desktopMain`/`iosMain`; Kotlin - tests run on the desktop target; Paparazzi snapshot tests live in - `androidUnitTest`. + tests run on the desktop target; Paparazzi snapshot tests live in that repo's + Android unit-test source set (a layout this app does not share). - Verify: ```bash ./gradlew ktlintFormat ktlintCheck diff --git a/.claude/skills/test-flow-planner/SKILL.md b/.claude/skills/test-flow-planner/SKILL.md index 27f9cde2d30..c1c89545c5a 100644 --- a/.claude/skills/test-flow-planner/SKILL.md +++ b/.claude/skills/test-flow-planner/SKILL.md @@ -290,7 +290,7 @@ order: ## Flows ### Flow A — `` -- **Source set**: androidInstrumentedTest | commonTest +- **Source set**: src/androidTest | commonTest - **Module path**: `app/src/androidTest/java/org/dhis2/usescases//` - **Claimed program**: `` `` (proposed / claimed) - **Program config changes**: Unit, -) { - ExampleRobot(composeTestRule).apply { robotBody() } -} - -// Robot class extending BaseRobot -class ExampleRobot(val composeTestRule: ComposeTestRule) : BaseRobot() { - fun typeUsername(username: String) { - composeTestRule.waitUntilExactlyOneExists(hasTestTag(USERNAME_TAG), TIMEOUT) - composeTestRule.onNodeWithTag(USERNAME_TAG).performClick() - composeTestRule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) - } - - fun clickSubmitButton() { - composeTestRule.waitUntilExactlyOneExists(hasTestTag(SUBMIT_TAG), TIMEOUT) - composeTestRule.onNodeWithTag(SUBMIT_TAG).performClick() - } - - fun verifySuccessMessageDisplayed() { - composeTestRule.waitUntilExactlyOneExists(hasTestTag(SUCCESS_TAG), TIMEOUT) - } -} - -// Test class -class ExampleTest : BaseTest() { - @get:Rule - val composeTestRule = createComposeRule() - - @Test - fun shouldPerformSuccessfulAction() { - mockWebServerRobot.addResponse(GET, "/api/endpoint", MOCK_RESPONSE, 200) - - exampleRobot(composeTestRule) { - typeUsername("user") - clickSubmitButton() - verifySuccessMessageDisplayed() - } - - cleanDatabase() - } -} -``` - -### Test Tags for Compose UI - -Always add test tags to interactive components. Export constants from the screen file. - -```kotlin -// In the screen composable file -const val LOGIN_BUTTON_TAG = "LOGIN_BUTTON_TAG" -const val USERNAME_INPUT_TAG = "USERNAME_INPUT_TAG" - -@Composable -fun LoginScreen() { - InputField(modifier = Modifier.testTag(USERNAME_INPUT_TAG)) - Button(modifier = Modifier.testTag(LOGIN_BUTTON_TAG)) { ... } -} -``` - -Format: `{SCREEN}_{COMPONENT}_TAG` (e.g., `LOGIN_BUTTON_TAG`, `HOME_MENU_TAG`) - -### DHIS2 Design System Components - -DHIS2 components are composite. Click the wrapper tag to focus, then target the -inner `"INPUT_TEXT_FIELD"` node. Always use `performTextInput()`, not -`performTextReplacement()`. - -```kotlin -// ✅ CORRECT -fun typeUsername(username: String) { - composeTestRule.onNodeWithTag(USERNAME_TAG).performClick() - composeTestRule.onAllNodesWithTag("INPUT_TEXT_FIELD")[0].performTextInput(username) -} - -// ❌ WRONG -fun typeUsername(username: String) { - composeTestRule.onNodeWithTag(USERNAME_TAG).performTextReplacement(username) -} -``` - -### Mock Server - -```kotlin -mockWebServerRobot.addResponse( - method = GET, - path = "/api/dataElements", - response = MOCK_DATA_ELEMENTS_JSON, - responseCode = 200, -) -``` - -## Unit Testing Guidelines - -### Mocking library: mockito-kotlin - -Use `mock()`, `whenever()`, `verify()` from `org.mockito.kotlin`. Do **not** use MockK. - -```kotlin -import org.mockito.kotlin.mock -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever -``` - -### Repository Tests - -```kotlin -class ExampleRepositoryTest { - private val d2: D2 = mock() - private val domainErrorMapper: DomainErrorMapper = mock() - private val repository = ExampleRepositoryImpl(d2, domainErrorMapper) - - @Test - fun `should map SDK data to domain models`() = runTest { - val sdkData = listOf() - whenever(d2.exampleModule().examples().blockingGet()).thenReturn(sdkData) - - val result = repository.getData() - - verify(d2.exampleModule().examples()).blockingGet() - } - - @Test - fun `should map D2Error to domain error`() = runTest { - val d2Error = D2Error.builder().errorCode(D2ErrorCode.API_RESPONSE_PROCESS_ERROR).build() - whenever(d2.exampleModule().examples().blockingGet()).thenThrow(d2Error) - whenever(domainErrorMapper.mapToDomainError(d2Error)).thenReturn(DomainException("Mapped error")) - - val result = runCatching { repository.getData() } - - assertTrue(result.isFailure) - verify(domainErrorMapper).mapToDomainError(d2Error) - } -} -``` - -### ViewModel Tests - -```kotlin -class ExampleViewModelTest { - private val getDataUseCase: GetDataUseCase = mock() - private val testDispatcher = UnconfinedTestDispatcher() - private lateinit var viewModel: ExampleViewModel - - @BeforeTest - fun setUp() { - Dispatchers.setMain(testDispatcher) - } - - @Test - fun `should emit success state when use case succeeds`() = runTest { - val data = listOf(ExampleData("test")) - whenever(getDataUseCase()).thenReturn(Result.success(flowOf(data))) - - viewModel = ExampleViewModel(getDataUseCase) - - assertEquals(UiState.Success(data), viewModel.uiState.value) - } -} -``` - -### Use Case Tests - -```kotlin -class GetDataUseCaseTest { - private val repository: ExampleRepository = mock() - private val useCase = GetDataUseCase(repository) - - @Test - fun `should filter invalid data`() = runTest { - val allData = listOf(ExampleData(isValid = true), ExampleData(isValid = false)) - whenever(repository.getData()).thenReturn(flowOf(allData)) - - val result = useCase(Unit) - - result.onSuccess { flow -> - flow.test { - val data = awaitItem() - assertTrue(data.all { it.isValid }) - awaitComplete() - } - } - } -} -``` - -## Test Organization - -``` -modulekmm/src/ -├── commonTest/kotlin/ # Shared unit tests (kotlin.test + mockito-kotlin + turbine) -│ ├── domain/ # Use case tests -│ └── data/ # Repository interface tests -├── androidUnitTest/kotlin/ # Android-specific unit tests -│ ├── data/ # Repository implementation tests -│ └── ui/ # ViewModel tests -└── androidInstrumentedTest/ # UI tests with Robot pattern - ├── robots/ # Robot classes - └── tests/ # Test classes -``` - -## Best Practices Checklist - -- Use `waitUntilExactlyOneExists()` before interacting with elements -- Robot methods use descriptive names (`clickLoginButton()`, not `click()`) -- Keep robots focused on actions; use separate methods for assertions -- Test user flows, not isolated components -- Mock all external dependencies (network, SDK, databases) -- Clean up after tests (`cleanDatabase()`, clear preferences) -- Use `performTextInput()` for DHIS2 design system inputs -- Export test tag constants from screen files - -## Common Mistakes to Avoid - -- Using `Thread.sleep()` or any hard-coded delays -- Using `performTextReplacement()` on DHIS2 design system components -- Using MockK (`mockk()`, `every {}`, `coEvery {}`) — use mockito-kotlin instead -- Forgetting to export test tag constants from screen files -- Not extending `BaseRobot` for robot classes -- Not cleaning up after tests -- Testing implementation details instead of user flows -- Forgetting to mock external dependencies before the test runs - -## Your Responsibilities - -When asked to create or fix tests: - -1. Identify test type: unit (use case / repository / ViewModel) or UI (instrumented) -2. Place tests in the correct source set (`commonTest`, `androidUnitTest`, or `androidInstrumentedTest`) -3. Use `mockito-kotlin` for all mocking — never MockK -4. For UI tests: apply the Robot pattern, export test tags, rely on `CoroutineTracker` -5. Ensure proper cleanup (database, preferences, mock server) -6. Run the relevant Gradle task to verify the test passes before finishing +You write, fix, and review tests for the DHIS2 Android Capture App (Kotlin +Multiplatform, Compose Multiplatform, DHIS2 Android SDK, Koin, MVVM). + +**The source of truth is `.claude/skills/android-testing/`.** Read it before +writing a test — `SKILL.md` routes you to the right source set and Gradle task, +`references/unit-testing.md` covers host tests, and +`references/instrumented-testing.md` covers device tests. This file is +deliberately a stub so the guidance cannot drift into a second, disagreeing copy. + +Invariants that hold everywhere in this repo: + +1. **mockito-kotlin only** (`mock()`, `whenever()`, `verify()`). Never MockK. +2. **No `Thread.sleep()` or hard-coded delays**, in any test. +3. **`D2` must be mocked with `RETURNS_DEEP_STUBS`** — its call chains NPE otherwise. +4. **JUnit4 / `kotlin.test` annotations only.** JUnit Jupiter is excluded from every + test configuration; a `org.junit.jupiter.api.Test` fails the build. +5. **Confirm the test actually ran** — check + `build/test-results//TEST-.xml` for `tests="N"`. A class that was + never collected produces no file at all, and the build still goes green. diff --git a/AGENTS.md b/AGENTS.md index f50f1651cdb..8d828aca18c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,10 +34,15 @@ targeting Android, Desktop, and iOS. The app uses MVVM + Repository + Use Case a ``` **Gradle task naming by module type:** -- Legacy Android modules (`form`, `commons`, `tracker`, etc.): `testDebugUnitTest` -- KMP modules (`login`, `commonskmm`, `sync`, `aggregates`), `commonTest` source set: `testAndroidHostTest` -- KMP modules, `androidUnitTest` source set: `testAndroidDebugUnitTest` -- Desktop targets in KMP modules: `desktopTest` +- `app`: `testDhis2DebugUnitTest` (it has flavours; the others do not) +- Other AGP modules (`form`, `commons`, `compose-table`, `stock-usecase`, + `dhis_android_analytics`, `dhis2_android_maps`, `dhis2-mobile-program-rules`): + `testDebugUnitTest` +- KMP modules (`login`, `sync`, `aggregates`, `commonskmm`, `tracker`) — **both** the + `commonTest` and `androidHostTest` source sets: `testAndroidHostTest` + +`testAndroidDebugUnitTest` is **not a task in this repo**, and there is no +`desktopTest` source set in any module despite the desktop targets. --- @@ -50,11 +55,14 @@ root/ ├── login/ # KMP login feature (Android + Desktop) ├── sync/ # KMP sync feature ├── aggregates/ # KMP aggregate data feature -├── tracker/ # Android tracker feature +├── tracker/ # KMP tracker feature ├── form/ # Android form module ├── commons/ # Android shared utilities (legacy) ├── compose-table/ # Compose table component -├── dhis2-mobile-program-rules/ # KMP program rules engine +├── stock-usecase/ # Android stock management feature +├── dhis_android_analytics/ # Android analytics/charts +├── dhis2_android_maps/ # Android maps +├── dhis2-mobile-program-rules/ # Android (com.android.library) program rules engine └── gradle/libs.versions.toml # Central dependency catalog ``` @@ -64,11 +72,15 @@ modulekmm/src/ ├── commonMain/kotlin/ # Shared business logic, interfaces, use cases ├── commonTest/kotlin/ # Shared unit tests (kotlin-test + mockito-kotlin + turbine) ├── androidMain/kotlin/ # Android implementations, SDK access -├── androidUnitTest/kotlin/ # Android-specific unit tests +├── androidHostTest/kotlin/ # Unit tests that need androidMain (e.g. mock D2) ├── desktopMain/kotlin/ # Desktop implementations └── composeResources/ # Shared Compose resources (strings, images) ``` +`androidHostTest` **depends on** `commonTest`, so a test dependency declared in +`commonTest` is already on its compile classpath — no need to declare it twice. +Both source sets are run by the same `testAndroidHostTest` task. + --- ## Code Style (enforced by ktlint 1.7.1) @@ -110,7 +122,7 @@ di/ # Koin module definitions ``` ### UseCase interface (commonskmm) -All use cases must implement `UseCase` from +New use cases implement `UseCase` from `commonskmm/src/commonMain/kotlin/org/dhis2/mobile/commons/domain/UseCase.kt`: ```kotlin @@ -122,6 +134,10 @@ fun interface UseCase { suspend operator fun UseCase.invoke() = this(Unit) ``` +It is the dominant pattern already — 33 implementations across `app` (14), `sync` (9), +`login` (6), `tracker` (3) and `commonskmm` (1). Where it is not used, a plain +`suspend operator fun invoke` returning `Result` is accepted. + Implementation pattern: ```kotlin class SavePinUseCase(private val repo: SessionRepository) : UseCase { @@ -129,24 +145,48 @@ class SavePinUseCase(private val repo: SessionRepository) : UseCase = domainErrorMapper.withDomainErrors { + d2.someModule().someRepository().blockingGet().map(::toDomain) +} +``` + +Why: the SDK's blocking RxJava operators rewrap the checked `D2Error` in a +`RuntimeException`, so an inline `catch (d2Error: D2Error)` misses every blocking +call. The wrappers walk the cause chain (`Throwable.asD2Error()`, depth 5, with a +self-reference guard) before mapping, so both shapes are handled. + +Use `withDomainErrorsAsResult { }` when the call reports failure as a `Result` +instead of throwing. `SyncDataSetRepositoryImpl` is a worked example. ### Dependency Injection (Koin 4.x) ```kotlin @@ -174,12 +214,20 @@ val featureModule = module { ## Testing -- **Unit tests**: `mockito-kotlin` + `kotlin.test` in `commonTest`; `mockito-kotlin` + JUnit in `androidUnitTest` and legacy modules -- **Flow assertions**: Turbine (`app.cash.turbine`) + `kotlinx-coroutines-test` -- **UI tests**: Compose Testing + Espresso, Robot pattern, located in `androidInstrumentedTest/` -- **ViewModel coroutines**: always use `launchUseCase { }` — it wraps `CoroutineTracker` which integrates with Espresso's `IdlingResource`; never use `Thread.sleep()` - -For patterns, examples, and common mistakes load the **android-testing** skill. +**The source of truth is `.claude/skills/android-testing/`** — load it before writing +any test. `SKILL.md` routes you to the right source set and Gradle task, +`references/unit-testing.md` covers host tests, `references/instrumented-testing.md` +covers device tests. + +The rules that hold everywhere: +- **mockito-kotlin only** (`mock()`, `whenever()`, `verify()`). Never MockK +- **JUnit4 / `kotlin.test` annotations only** — `org.junit.jupiter` is excluded from + every test configuration and a Jupiter `@Test` fails the build +- **`D2` must be mocked with `RETURNS_DEEP_STUBS`** — its call chains NPE otherwise +- **Never `Thread.sleep()`** or any hard-coded delay +- **Confirm the test actually ran** — check `build/test-results//TEST-.xml` + for `tests="N"`. A class that was never collected produces no file, and the build + still goes green --- diff --git a/aggregates/build.gradle.kts b/aggregates/build.gradle.kts index 8ccff7dfd36..8fef5b9d2fe 100644 --- a/aggregates/build.gradle.kts +++ b/aggregates/build.gradle.kts @@ -55,7 +55,6 @@ kotlin { implementation(kotlin("test")) // Koin Test features implementation(libs.koin.test) - implementation(libs.koin.test.junit5) implementation(libs.koin.test.junit4) implementation(libs.test.turbine) implementation(libs.test.kotlinCoroutines) @@ -74,10 +73,6 @@ kotlin { compileOnly(libs.androidx.compose.uitooling) } - getByName("androidHostTest") { - dependencies { implementation(libs.junit.jupiter) } - } - val desktopMain by getting { dependencies { implementation(libs.compose.desktop.common) diff --git a/aggregates/src/commonTest/kotlin/org/dhis2/mobile/aggregates/ui/viewModel/DataSetTableViewModelTest.kt b/aggregates/src/commonTest/kotlin/org/dhis2/mobile/aggregates/ui/viewModel/DataSetTableViewModelTest.kt index 009fb277657..47492447f1c 100644 --- a/aggregates/src/commonTest/kotlin/org/dhis2/mobile/aggregates/ui/viewModel/DataSetTableViewModelTest.kt +++ b/aggregates/src/commonTest/kotlin/org/dhis2/mobile/aggregates/ui/viewModel/DataSetTableViewModelTest.kt @@ -77,7 +77,6 @@ import org.junit.After import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import org.junit.jupiter.api.assertThrows import org.koin.core.component.get import org.koin.core.context.startKoin import org.koin.core.context.stopKoin @@ -93,6 +92,7 @@ import org.mockito.kotlin.doReturnConsecutively import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import kotlin.test.assertEquals +import kotlin.test.assertFailsWith @OptIn(ExperimentalCoroutinesApi::class) internal class DataSetTableViewModelTest : KoinTest { @@ -769,7 +769,7 @@ internal class DataSetTableViewModelTest : KoinTest { @Test fun `should throw error if more than one data element is provided`() { val exception = - assertThrows { + assertFailsWith { IdsProvider.getDataElementUid( rowIds = listOf(TableId("dataElementId", TableIdType.DataElement)), columnIds = listOf(TableId("dataElementId", TableIdType.DataElement)), @@ -785,7 +785,7 @@ internal class DataSetTableViewModelTest : KoinTest { fun `should throw error if more than one category option combo is provided`() = runTest { val exception = - assertThrows { + assertFailsWith { IdsProvider.getCategoryOptionCombo( rowIds = listOf( @@ -805,7 +805,7 @@ internal class DataSetTableViewModelTest : KoinTest { fun `should throw error if category options and category option combos are provided`() = runTest { val exception = - assertThrows { + assertFailsWith { IdsProvider.getCategoryOptionCombo( rowIds = listOf( diff --git a/build.gradle.kts b/build.gradle.kts index dc23c15aa12..00884fb435a 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -75,6 +75,14 @@ allprojects { } } + // JUnit Jupiter must never reach a test configuration. useJUnitPlatform() is set + // nowhere in this build, so a Jupiter @Test is not run -- it is silently never + // collected: the class compiles, the build goes green, and no result file is + // written. Failing to resolve is the loud alternative. + configurations.matching { it.name.contains("test", ignoreCase = true) }.configureEach { + exclude(group = "org.junit.jupiter") + } + apply(plugin = "org.jlleitschuh.gradle.ktlint") gradle.projectsEvaluated { @@ -128,6 +136,47 @@ allprojects { } } +// One command for humans and CI, so "what CI runs" has a single definition. +// run_tests.sh calls this; ci.yml deliberately still calls the tasks directly, because +// its unit-tests job already needs the separate lint-check job and would run ktlint twice. +// +// No mustRunAfter is needed here: jacocoReport already orders itself after the unit +// tests. The dependsOn loop in jacoco/jacoco.gradle.kts runs inside that task's +// registration action, which Gradle realises after AGP has created the test tasks, so +// findByName does resolve them. Verified with `./gradlew :app:jacocoReport --dry-run`. +val verificationTaskNames = listOf( + "ktlintCheck", + "testDebugUnitTest", + "testDhis2DebugUnitTest", + "testAndroidHostTest", + "jacocoReport", +) + +tasks.register("verifyAll") { + group = "verification" + description = "Runs ktlint, every unit-test task and the coverage reports. Mirrors CI." + + // A Provider, not a plain list: with org.gradle.configureondemand=true the subprojects + // are not all configured while this script runs, so resolving the tasks eagerly here + // yields an empty set. A Provider is resolved when the task graph is built, by which + // point the allprojects { } block above has forced every project to configure. + dependsOn( + provider { + rootProject.allprojects + .flatMap { project -> + verificationTaskNames.mapNotNull { project.tasks.findByName(it) } + } + .also { resolved -> + // A verification task that resolves to nothing passes without running + // anything -- exactly the silent success this task exists to prevent. + check(resolved.isNotEmpty()) { + "verifyAll resolved no tasks to run." + } + } + }, + ) +} + // Initialize extra properties on the root project for storing totals rootProject.ext.set("totalTestsRun", 0L) rootProject.ext.set("totalTestsPassed", 0L) diff --git a/commonskmm/build.gradle.kts b/commonskmm/build.gradle.kts index 28a49067e15..dff16c6b051 100644 --- a/commonskmm/build.gradle.kts +++ b/commonskmm/build.gradle.kts @@ -61,8 +61,6 @@ kotlin { commonTest.dependencies { implementation(kotlin("test")) implementation(libs.koin.test) - implementation(libs.koin.test.junit5) - implementation(libs.koin.test.junit4) implementation(libs.test.kotlinCoroutines) implementation(libs.test.mockitoKotlin) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b7d83814008..f2741669ee5 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -82,7 +82,6 @@ material3WindowSize = "1.4.0" material3WindowSizeCompose = "1.8.2" material3AdaptiveAndroid = "1.1.0" koin = "4.1.1" -junitJupiter = "6.0.3" ktxml = "1.0.0" atomicfu = "0.29.0" playservicesauth = "21.4.0" @@ -210,9 +209,7 @@ koin-compose = { module = "io.insert-koin:koin-compose", version.ref = "koin" } koin-composeVM = { module = "io.insert-koin:koin-compose-viewmodel", version.ref = "koin" } koin-work = { module = "io.insert-koin:koin-androidx-workmanager", version.ref = "koin" } koin-test = { module = "io.insert-koin:koin-test", version.ref = "koin" } -koin-test-junit5 = { module = "io.insert-koin:koin-test-junit5", version.ref = "koin" } koin-test-junit4 = { module = "io.insert-koin:koin-test-junit4", version.ref = "koin" } -junit-jupiter = { group = "org.junit.jupiter", name = "junit-jupiter", version.ref = "junitJupiter" } atomicfu = { module = "org.jetbrains.kotlinx:atomicfu", version.ref = "atomicfu" } ktxml = { module = "org.kobjects.ktxml:core", version.ref = "ktxml" } commons-text = { module = "org.apache.commons:commons-text", version.ref = "commonsText" } diff --git a/login/build.gradle.kts b/login/build.gradle.kts index 9bcb240a8d3..5d93b477f2f 100644 --- a/login/build.gradle.kts +++ b/login/build.gradle.kts @@ -89,7 +89,6 @@ kotlin { getByName("androidHostTest") { dependencies { implementation(kotlin("test")) - implementation(libs.junit.jupiter) implementation(libs.test.kotlinCoroutines) implementation(libs.test.mockitoKotlin) } diff --git a/run_tests.sh b/run_tests.sh index 1907444367b..ac88da878b0 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -2,10 +2,7 @@ # This will make exit immediately if any command fails set -e -echo "Running Ktlint check..." -./gradlew ktlintCheck - -echo "Running Unit Tests..." -./gradlew testDebugUnitTest testDhis2DebugUnitTest testAndroidHostTest +echo "Running full verification (ktlint + unit tests + coverage)..." +./gradlew verifyAll echo "All tasks completed!" diff --git a/sync/build.gradle.kts b/sync/build.gradle.kts index 57e2ad0145a..92c8c41796c 100644 --- a/sync/build.gradle.kts +++ b/sync/build.gradle.kts @@ -54,7 +54,6 @@ kotlin { commonTest.dependencies { implementation(kotlin("test")) implementation(libs.koin.test) - implementation(libs.koin.test.junit5) implementation(libs.test.kotlinCoroutines) implementation(libs.test.mockitoKotlin) } @@ -71,7 +70,6 @@ kotlin { getByName("androidHostTest") { dependencies { implementation(kotlin("test")) - implementation(libs.junit.jupiter) implementation(libs.test.turbine) implementation(libs.test.kotlinCoroutines) implementation(libs.test.mockitoKotlin) diff --git a/tracker/build.gradle.kts b/tracker/build.gradle.kts index 887e8e78ae3..6e713b64731 100644 --- a/tracker/build.gradle.kts +++ b/tracker/build.gradle.kts @@ -77,7 +77,6 @@ kotlin { getByName("androidHostTest") { dependencies { implementation(kotlin("test")) - implementation(libs.junit.jupiter) implementation(libs.test.turbine) implementation(libs.test.kotlinCoroutines) implementation(libs.test.mockitoKotlin)