Skip to content

test: Add Unit Test for :feature:auth module#1871

Merged
niyajali merged 11 commits intoopenMF:developmentfrom
Shreyash16b:MW-221
Jul 16, 2025
Merged

test: Add Unit Test for :feature:auth module#1871
niyajali merged 11 commits intoopenMF:developmentfrom
Shreyash16b:MW-221

Conversation

@Shreyash16b
Copy link
Copy Markdown
Contributor

@Shreyash16b Shreyash16b commented Jun 3, 2025

Issue Fix

Fixes 221
Jira Task: 221

Screenshots

LoginViewModelTest Result
SignUpViewModelTest Result
MobileVerificationViewModelTest Result

Description

  1. Added dependency (mokkery) for unit tests.
  2. Wrote tests for LoginViewModel.kt, SignUpViewModel.kt, MobileVerificationViewModel.kt.
  3. These tests run on the Android and Desktop platforms, but give errors for JS and WASM.

This is the error while running command ./gradlew :feature:auth:jsTest
JS Error

This is the error while running command ./gradlew :feature:auth:wasmJsBrowserTest
WASM Error

  • Apply the AndroidStyle.xml style template to your code in Android Studio.

  • [✅] Run the unit tests with ./gradlew check to make sure you didn't break anything

  • [✅] If you have multiple commits please combine them into one commit by squashing them.

state.savingsProductId == 0 -> {
mutableStateFlow.update {
it.copy(dialogState = SignUpDialog.Error("Please select a savings account."))
it.copy(dialogState = SignUpDialog.Error(SELECT_SAVINGS_ACCOUNT))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert it back to previous as we don't use Constants for these messages we are gonna use stringResource which if we target different audience we will use localization

commonTest.dependencies {
implementation(kotlin("test-common"))
implementation(kotlin("test-annotations-common"))
implementation(libs.kotlinx.coroutines.test)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Add the dependencies if it is needs for example implementation(libs.kotlinx.coroutines.test) this one is already added in KMPLibraryConventionPlugin.kt or implementation(compose.uiTest) as we are not focusing in UI test don't add it and other dependecnies that you added.
  • @niyajali should he use mokerry third party library for testing?

}

@Test
fun `username change updates state`() = runTest(testDispatcher) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use one style for naming the test.
for example loginViewModel_UsernameChanged_UpdatesState

viewModel.trySendAction(LoginAction.PasswordChanged("testpass"))
viewModel.trySendAction(LoginAction.LoginClicked)

println(viewModel.stateFlow.value)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the printlns

const val UNAUTHORIZED_ERROR = "401 Unauthorized"

const val SELECT_SAVINGS_ACCOUNT = "Please select a savings account."
const val ENTER_FIRST_NAME = "Please enter your first name."
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove these added these constants as i mentioned the reason below

@HekmatullahAmin
Copy link
Copy Markdown
Member

@niyajali if you have any suggestions for him regarding to which format or how to write unit testing regarding our format let him know

alias(libs.plugins.mifospay.cmp.feature)
alias(libs.plugins.kotlin.parcelize)
alias(libs.plugins.kotlin.serialization)
id("dev.mokkery") version "2.7.2"
Copy link
Copy Markdown
Contributor

@biplab1 biplab1 Jul 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This direct version dependency should be replaced. Please use alias(libs.mokkery) instead to keep it consistent with the version catalog.

state.mobileNumberInput.isEmpty() -> {
mutableStateFlow.update {
it.copy(dialogState = SignUpDialog.Error("Please enter your mobile number."))
it.copy(dialogState = SignUpDialog.Error("Please enter a your mobile number."))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding a is a grammatical mistake this should not ne added

"\n- At least one lowercase character" +
"\n- At least one numeric digit" +
"\n- At least one special character"
?: """
Copy link
Copy Markdown
Member

@HekmatullahAmin HekmatullahAmin Jul 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't remove the \n otherwise it will show all message attach to each other

revert any changes regarding the text

state.countryInput.isEmpty() -> {
mutableStateFlow.update {
it.copy(dialogState = SignUpDialog.Error("Please enter your country."))
it.copy(dialogState = SignUpDialog.Error("Please enter your country"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one thing keep the text as the way were before

@HekmatullahAmin
Copy link
Copy Markdown
Member

@Shreyash16b fix the things that i mentioned and give me the authority to push changes to this PR. once i get the change i will pull it

@HekmatullahAmin
Copy link
Copy Markdown
Member

@Shreyash16b don't push any new code for now. just give me the authority so i do it

@Shreyash16b
Copy link
Copy Markdown
Contributor Author

@HekmatullahAmin Sorry, I just pushed the code a minute before your message.

@HekmatullahAmin
Copy link
Copy Markdown
Member

HekmatullahAmin commented Jul 11, 2025

  • @Shreyash16b i can't change the PR title. could u please change it to this: Add feature/auth unit tests and add KDoc for clarity
  • And also add these photos in the description of PR remove those error images that u received and posted.

Description

This PR improves test coverage and developer experience in the authentication feature by:

  • ✅ Added unit tests for:

    • SignUpViewModelTest

    • LoginViewModelTest

    • MobileVerificationViewModelTest

  • 🔧 Mocking approach:

    • Used Mokkery to mock repositories in SignUpViewModelTest and MobileVerificationViewModelTest.

    • Mocked LoginUseCase in LoginViewModelTest for isolated and deterministic behavior.

  • 🛠️ Test support enhancements:

    • Introduced the @OpenForMokkery annotation and applied the kotlin-allopen plugin scoped to this module, allowing open classes for mocking without polluting production codebase.

    • Created and integrated a StringProvider interface to decouple direct usage of getString() in ViewModels.

      • This enables test-safe injection of fake/mock StringProvider implementations.

      • In production, DefaultStringProvider uses the real Compose getString() call.

  • 📚 Documentation:

    • Added comprehensive KDoc comments to improve test and utility class readability and maintainability.

Android

SignUpViewModelTest

image

LoginViewModelTest

image

MobileVerificationViewModelTest

image

Ios

ios

Desktop

desktop

WasmJsBrowser

wasmJsBrowser

JsNode

JsNode

@Shreyash16b Shreyash16b changed the title Initial commit feature/auth unit test Add feature/auth unit tests and add KDoc for clarity Jul 12, 2025

import org.mifospay.core.model.search.SearchResult

val fakeSearchResult = SearchResult(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this directly on that file where it has been used no need to create separate file for this

Enhancement: Move the following code in to a Use Case
*/
private fun initiateSignUp() {
DialogManager.showLoading()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we introduced DialogManager, @HekmatullahAmin are you sure that these changes are necessary

Copy link
Copy Markdown
Member

@HekmatullahAmin HekmatullahAmin Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niyajali Previously i created so to have one object handle the dialog logic in all our screens and cause of Parcelization we couldn't persist those of StringResource type. Although we can achieve the unit testing with DialogManager, but it will bound to lifecycle of viewmodel and also in unit testing then we also need to call it to check it is state.


import org.mifospay.core.model.user.UserInfo

val fakeUserInfo = UserInfo(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this too and make it internal

Dispatchers.resetMain()
}

// --------------------------------------------------------------------------
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this comments

* Verifies initial state values are set correctly on ViewModel init.
*/
@Test
fun loginViewModel_InitialState_ValidInitialConditions() = runTest(testDispatcher) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the prefix from all test methods, and try to followT, est method should follow the given_when_then or when_then format.

assertFalse(viewModel.stateFlow.value.isPasswordVisible)

viewModel.trySendAction(LoginAction.TogglePasswordVisibility)
advanceUntilIdle()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we won't need this always, remove the testDispatcher from this lamda runTest(testDispatcher) as you've defined in setup method, use turbine libray to test the flow

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator

@niyajali niyajali Jul 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i removed it, but we don't have any flow in the view models to test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

viewModel.stateFlow, viewModel.eventFlow are flowable property

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand your point, but testing can be inconsistent and really sucks sometimes in my experience, sometimes the UI displays correct output while tests fail, or tests pass locally in your IDE but fail in CI.

This is why I'd suggest combining both approaches. When collecting values with .value, you might need to use waitUntilIdle() or collect the flow properly, since flows only emit values when there's an active collector. However, this might not be necessary in some cases as this project designed UDF (Unidirectional Data Flow) pattern.

Let's implement what works best for our use case, and for reference, check the Now in Android project or other similar examples given above.

https://github.com/android/nowinandroid/blob/main/feature/foryou/src/test/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niyajali

Our BaseViewModel handles every user action asynchronously via viewModelScope.launch, using advanceUntilIdle() is necessary to ensure state changes are fully applied before we assert on stateFlow.value.

viewModelScope.launch {
    internalActionChannel.consumeAsFlow().collect { action -> handleAction(action) }
}

This means:

Even if handleAction updates the state immediately, it's still queued on a coroutine and will run in a future dispatch cycle.

So when your test does:

viewModel.trySendAction(...)
assertEquals(expected, viewModel.stateFlow.value)

...it might assert too early, before handleAction actually runs.

That's why you use advanceUntilIdle() — it gives the coroutine scheduler time to process any enqueued tasks before you assert.

  • .value makes perfect sense when we only care about the final, conflated state.

  • That said, Turbine is a great fit when we need to capture multiple state emissions over time.

So going forward, I’ll continue using .value + advanceUntilIdle() for most simple stateFlow assertions, but I’ll switch to using Turbine for eventFlow testing to ensure we properly collect and verify emitted events.

below is an example for both approach which i think using turbine here is not necessary our logic is more simple in first one:

@Test
    fun givenFirstName_whenInputChanged_thenFirstNameIsUpdated() = runTest {
        val firstName = "John"
        viewModel.trySendAction(SignUpAction.FirstNameInputChange(firstName))
        advanceUntilIdle() 
        assertEquals(firstName, viewModel.stateFlow.value.firstNameInput)
    }

    @Test
    fun givenFirstNames_whenInputChanged_thenFirstNameIsUpdated() = runTest {
        viewModel.stateFlow.test {
            // Skip initial state
            skipItems(1)

            // Act: send username change
            val firstName = "John"
            viewModel.trySendAction(SignUpAction.FirstNameInputChange(firstName))

            // Assert: username should now be "alice"
            val state = awaitItem()
            assertEquals(firstName, state.firstNameInput)
        }
    }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I meant the same, refactor all the test cases accordingly and use proper naming conventions like given_when_then or when_then and let me know.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niyajali yeah already used given_when_then naming convention.
and also added unit test for eventFlow

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve directly tested the stateFlow using .value along with advanceUntilIdle(), as done in the Now in Android project. Since our logic is relatively simple and we only care about the latest state, using Turbine would’ve added unnecessary checks like skipItems() or awaitItem() that don’t add value in this case.

import kotlin.test.assertTrue

@OptIn(ExperimentalCoroutinesApi::class)
class MobileVerificationViewModelTest {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly apply those changes on these test classes

@niyajali niyajali changed the title Add feature/auth unit tests and add KDoc for clarity test: Add Unit Test for :feature:auth module Jul 12, 2025
@niyajali
Copy link
Copy Markdown
Collaborator

niyajali commented Jul 15, 2025

@HekmatullahAmin Upload Test results for all platforms

@HekmatullahAmin
Copy link
Copy Markdown
Member

@HekmatullahAmin Upload Test results for all platforms

@niyajali done. i run one viewmodel test in one platform if i did all it would take a lot of space. and also could u just cut it and paste it in PR description i don't have authority to do it.

@niyajali niyajali merged commit 72c988b into openMF:development Jul 16, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants