Skip to content

feat(onboarding): Add OnboardingViewModel tests and improve effect handling#670

Merged
Mostafa-alsaygh merged 24 commits intodevelopfrom
add/onboarding-viewmodel-testing
Aug 21, 2025
Merged

feat(onboarding): Add OnboardingViewModel tests and improve effect handling#670
Mostafa-alsaygh merged 24 commits intodevelopfrom
add/onboarding-viewmodel-testing

Conversation

@Mostafa-alsaygh
Copy link
Contributor

What does this PR do?

this PR introduces comprehensive unit tests for OnboardingViewModel, covering various scenarios like page changes, scrolling, and navigation.

Key changes:

  • Added OnboardingViewModelTest.kt with tests for all public methods and state updates.
  • Refactored onboardingScreen.kt to use Listen for handling OnboardingEffect, simplifying the effect handling logic and removing LaunchedEffect.
  • Removed unnecessary logging in OnboardingViewModel for onboardingFinished.
  • Updated welcomeScreen.kt to use onNavigateAsGuest instead of onNavigateContinue for better clarity.
  • Included kotlin-test dependency in presentation/build.gradle.kts for unit testing.
  • Updated onboardingNavGraph.kt to reflect the name change from onNavigateContinue to onNavigateAsGuest.

…ndling

This commit introduces comprehensive unit tests for `OnboardingViewModel`, covering various scenarios like page changes, scrolling, and navigation.

Key changes:
- Added `OnboardingViewModelTest.kt` with tests for all public methods and state updates.
- Refactored `onboardingScreen.kt` to use `Listen` for handling `OnboardingEffect`, simplifying the effect handling logic and removing `LaunchedEffect`.
- Removed unnecessary logging in `OnboardingViewModel` for `onboardingFinished`.
- Updated `welcomeScreen.kt` to use `onNavigateAsGuest` instead of `onNavigateContinue` for better clarity.
- Included `kotlin-test` dependency in `presentation/build.gradle.kts` for unit testing.
- Updated `onboardingNavGraph.kt` to reflect the name change from `onNavigateContinue` to `onNavigateAsGuest`.
@london-bot
Copy link
Contributor

london-bot bot commented Aug 18, 2025

[London Bot] Metrics Update Status

📊 PR metrics successfully updated for event: closed.

@london-bot
Copy link
Contributor

london-bot bot commented Aug 18, 2025

[London Bot] Auto-update Status

✅ Branch successfully updated from develop.

Copy link
Collaborator

@yusufnasserdev yusufnasserdev left a comment

Choose a reason for hiding this comment

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

Kindly check failed test below:

OnboardingViewModelTest > when onboardingFinished is called, preferences service should be invoked FAILED
java.lang.AssertionError at OnboardingViewModelTest.kt:255

@Mostafa-alsaygh Mostafa-alsaygh self-assigned this Aug 19, 2025
Mostafa-alsaygh and others added 3 commits August 19, 2025 15:24
…ation

This commit renames the `OnboardingEffect.NavigateToWelcome` to `OnboardingEffect.OnWelcomeNavigation` for better clarity.

Additionally, it removes an unused `testImplementation(kotlin("test"))` dependency from the `presentation/build.gradle.kts` file.
# Conflicts:
#	presentation/src/main/java/com/london/presentation/feature/welcome/onboarding/welcomeScreen.kt
@github-actions
Copy link

github-actions bot commented Aug 19, 2025

Refactors the `onboardingFinished` and `setOnBoardingShown` functions in `OnboardingViewModel` to use the `tryToExecute` extension function for setting the onboarding shown preference. This improves error handling and logging.
Mostafa-alsaygh and others added 14 commits August 19, 2025 20:06
# Conflicts:
#	presentation/src/main/java/com/london/presentation/feature/welcome/onboarding/OnboardingViewModel.kt
This commit addresses potential flakiness in `OnboardingViewModelTest` by:

- Introducing `advanceTimeBy(1000)` and `advanceUntilIdle()` in tests involving `onboardingFinished()` and `navigateToWelcome()`. This ensures that IO operations (specifically, `appPreferencesService.setOnBoardingShown()`) have sufficient time to complete before assertions are made.
- Adding an alternative test case (`onboardingFinished is called, preferences service should be invoked - with timeout`) that utilizes `coVerify(timeout = 2000)` as another way to handle asynchronous operations, providing more robust verification.

These changes enhance the reliability of the tests by better managing asynchronous operations within the `runTest` environment.
# Conflicts:
#	app/src/main/java/com/london/app/navigation/graph/onboardingNavGraph.kt
#	presentation/src/main/java/com/london/presentation/feature/welcome/onboarding/OnboardingScreen.kt
The `scrollNext` function in `OnboardingViewModel` involves a coroutine scope for scrolling the Pager. This commit adds `advanceTimeBy(1000)` to the corresponding test case to ensure the coroutine has enough time to complete, preventing potential flakiness.
… and `coVerify` with `timeout`

This commit refactors tests in `OnboardingViewModelTest` to improve reliability and readability.

- `advanceTimeBy(1000)` has been replaced with `withTimeout(2000)` when asserting navigation effects. This provides a more explicit timeout mechanism for operations that might take longer.
- For `coVerify` calls that interact with `appPreferencesService`, a `timeout = 2000` parameter has been added directly to the `coVerify` function. This ensures the verification waits for the asynchronous preference setting operation to complete.

These changes make the tests more robust by explicitly handling potential timeouts for asynchronous operations, particularly those involving I/O like preference updates and navigation.
# Conflicts:
#	presentation/src/main/java/com/london/presentation/feature/category/tvshow/TvShowCategoryScreen.kt
#	presentation/src/main/java/com/london/presentation/feature/details/actor/info/gallery/ActorsGalleryScreen.kt
#	presentation/src/main/java/com/london/presentation/feature/home/trending/actor/TrendingActorsScreen.kt
@Mostafa-alsaygh Mostafa-alsaygh merged commit 2898f79 into develop Aug 21, 2025
17 of 20 checks passed
@Mostafa-alsaygh Mostafa-alsaygh deleted the add/onboarding-viewmodel-testing branch August 21, 2025 17:42
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.

3 participants