Skip to content

Improve Component testability + add example tests #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
matejsemancik wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Improve Component testability + add example tests #125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
972dc04
Improve domain layer testability: refactor UseCases to interface
matejsemancik Jun 12, 2025
52d46c8
Refactor BaseComponent and related classes to improve coroutine manag…
matejsemancik Jun 14, 2025
5b2377c
Refactor FirstScreen for better testability
matejsemancik Jun 14, 2025
7ca0b5b
Refactor use case interfaces to use fun interface
matejsemancik Jun 14, 2025
16fea98
Refactor SingleUseCaseExecutionScope to use Job instead of Deferred f…
matejsemancik Jun 14, 2025
a96c990
Add kotlinx.coroutines.test and turbine dependencies for improved tes…
matejsemancik Jun 14, 2025
831379f
Add FirstComponentTest.kt as an example of Component test
matejsemancik Jun 14, 2025
0ed808f
Increase buffer capacity for eventFlow in BaseComponent
matejsemancik Jun 20, 2025
e7edbef
Reformat code
matejsemancik Jun 20, 2025
4e5d6dd
Add ComponentTest interface and ComponentTestPreparation implementati…
matejsemancik Jun 20, 2025
a5a68db
Merge branch 'develop' into feature/component-testing
matejsemancik Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions androidApp/src/main/kotlin/app/futured/kmptemplate/android/ui/screen/FirstScreenUi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import app.futured.kmptemplate.feature.ui.firstScreen.FirstViewState
import app.futured.kmptemplate.resources.MR
import app.futured.kmptemplate.resources.kmpStringResource
import app.futured.kmptemplate.resources.localized
import dev.icerock.moko.resources.desc.desc

@Composable
fun FirstScreenUi(
Expand All @@ -51,7 +50,7 @@ fun FirstScreenUi(
Content(viewState = viewState, actions = actions, modifier = modifier)

EventsEffect(eventsFlow = screen.events) {
onEvent<FirstUiEvent.ShowToast> { event ->
onEvent<FirstUiEvent.Notify> { event ->
Toast.makeText(context, event.text.toString(context), Toast.LENGTH_SHORT).show()
}
}
Expand Down Expand Up @@ -80,12 +79,12 @@ private fun Content(
verticalArrangement = Arrangement.Center,
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(text = viewState.counter.localized())
Text(text = viewState.counterText.localized())
Spacer(modifier = Modifier.height(4.dp))
Text(text = viewState.createdAt.localized())
Text(text = viewState.createdAtText.localized())
Spacer(modifier = Modifier.height(4.dp))
AnimatedVisibility(viewState.randomPerson != null) {
viewState.randomPerson?.let { person ->
AnimatedVisibility(viewState.randomPersonText != null) {
viewState.randomPersonText?.let { person ->
Column {
Spacer(modifier = Modifier.height(4.dp))
Text(
Expand Down Expand Up @@ -115,7 +114,7 @@ private fun FirstScreenPreview() {
MyApplicationTheme {
Surface {
Content(
viewState = FirstViewState(counter = "Hey there".desc()),
viewState = FirstViewState.mock(),
actions = actions,
modifier = Modifier.fillMaxSize(),
)
Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ dokkaVersion = "1.9.20"
google-servicesPlugin = "4.4.2"
google-firebaseAppDistributionPlugin = "5.0.0"
poet = "2.0.0"
turbine = "1.2.1"

# Android Namespaces
project-android-namespace = "app.futured.kmptemplate.android"
Expand Down Expand Up @@ -104,6 +105,7 @@ androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-co
androidx-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version.ref = "uiautomator" }
androidx-benchmark-macro-junit4 = { group = "androidx.benchmark", name = "benchmark-macro-junit4", version.ref = "benchmarkMacroJunit4" }
androidx-profileinstaller = { group = "androidx.profileinstaller", name = "profileinstaller", version.ref = "profileinstaller" }
turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" }

# Desugar
androidTools-desugarLibs = { group = "com.android.tools", name = "desugar_jdk_libs", version.ref = "desugarLibs" }
Expand Down
4 changes: 2 additions & 2 deletions iosApp/iosApp/Views/Screen/First/FirstView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ struct FirstView<ViewModel: FirstViewModelProtocol>: View {
.navigationTitle(Localizable.first_screen_title.localized)
.eventsEffect(for: viewModel.events) { event in
switch onEnum(of: event) {
case .showToast(let event):
viewModel.showToast(event: event)
case .notify(let event):
viewModel.notify(event: event)
}
}
.alert(viewModel.alertText, isPresented: viewModel.isAlertVisible) {
Expand Down
11 changes: 6 additions & 5 deletions iosApp/iosApp/Views/Screen/First/FirstViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ protocol FirstViewModelProtocol: DynamicProperty {
var alertText: String { get }

func onNext()
func showToast(event: FirstUiEvent.ShowToast)
func notify(event: FirstUiEventNotify)
func hideToast()
}

Expand All @@ -30,16 +30,17 @@ struct FirstViewModel {
}

extension FirstViewModel: FirstViewModelProtocol {

var counter: String {
viewState.counter.localized()
viewState.counterText.localized()
}

var createdAt: String {
viewState.createdAt.localized()
viewState.createdAtText.localized()
}

var randomPerson: String? {
viewState.randomPerson?.localized()
viewState.randomPersonText?.localized()
}

var isAlertVisible: Binding<Bool> {
Expand All @@ -53,7 +54,7 @@ extension FirstViewModel: FirstViewModelProtocol {
actions.onNext()
}

func showToast(event: FirstUiEvent.ShowToast) {
func notify(event: FirstUiEventNotify) {
alertText = event.text.localized()
alertVisible = true
}
Expand Down
10 changes: 2 additions & 8 deletions ...arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/FlowUseCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,16 @@
package app.futured.arkitekt.crusecases

import app.futured.arkitekt.crusecases.scope.FlowUseCaseExecutionScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.Flow

/**
* Base [Flow] use case meant to use in [FlowUseCaseExecutionScope] implementations
*/
abstract class FlowUseCase<ARGS, T> {

/**
* [Job] used to hold and cancel existing run of this use case
*/
var job: Job? = null
interface FlowUseCase<ARGS, T> {

/**
* Function which builds Flow instance based on given arguments
* @param args initial use case arguments
*/
abstract fun build(args: ARGS): Flow<T>
fun build(args: ARGS): Flow<T>
}
10 changes: 2 additions & 8 deletions shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/UseCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
package app.futured.arkitekt.crusecases

import app.futured.arkitekt.crusecases.scope.SingleUseCaseExecutionScope
import kotlinx.coroutines.Deferred

/**
* Base Coroutine use case meant to use in [SingleUseCaseExecutionScope] implementations
*/
abstract class UseCase<ARGS, T> {

/**
* [Deferred] used to hold and cancel existing run of this use case
*/
var deferred: Deferred<T>? = null
interface UseCase<ARGS, T> {

/**
* Suspend function which should contain business logic
*/
abstract suspend fun build(args: ARGS): T
suspend fun build(args: ARGS): T
}
2 changes: 1 addition & 1 deletion ...ecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/CoroutineScopeOwner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface CoroutineScopeOwner {
* It is your responsibility to cancel it when all running
* tasks should be stopped
*/
val viewModelScope: CoroutineScope
val useCaseScope: CoroutineScope

/**
* Provides Dispatcher for background tasks. This may be overridden for testing purposes.
Expand Down
18 changes: 12 additions & 6 deletions .../src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/FlowUseCaseExecutionScope.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app.futured.arkitekt.crusecases.scope

import app.futured.arkitekt.crusecases.FlowUseCase
import app.futured.arkitekt.crusecases.error.UseCaseErrorHandler
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.launchIn
Expand All @@ -13,6 +14,11 @@ import kotlin.coroutines.cancellation.CancellationException

interface FlowUseCaseExecutionScope : CoroutineScopeOwner {

/**
* Map of [Job] objects used to hold and cancel existing run of any [FlowUseCase] instance.
*/
val useCaseJobPool: MutableMap<Any, Job>

Comment on lines +17 to +21
Copy link

@coderabbitai coderabbitai bot Jun 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Potential memory leak & thread-safety in useCaseJobPool

Jobs are stored but never removed; finished or cancelled jobs will accumulate in the map for the lifetime of the scope.
Additionally, MutableMap isn’t thread-safe – concurrent executions on different dispatchers may race.

@@
-    val useCaseJobPool: MutableMap<Any, Job>
+    val useCaseJobPool: MutableMap<Any, Job>  // consider ConcurrentHashMap on JVM / atomic wrapper on native
🤖 Prompt for AI Agents 
In
shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/FlowUseCaseExecutionScope.kt
between lines 17 and 21, the useCaseJobPool map stores Job objects but never
removes finished or cancelled jobs, causing a potential memory leak. Also,
MutableMap is not thread-safe, risking race conditions during concurrent access.
To fix this, replace the MutableMap with a thread-safe concurrent map
implementation and ensure that jobs are removed from the map once they complete
or are cancelled to prevent accumulation.

Copy link
Member Author

@matejsemancik matejsemancik Jun 20, 2025

Choose a reason for hiding this comment

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

Can you elaborate with concrete examples?

Copy link

@coderabbitai coderabbitai bot Jun 20, 2025

Choose a reason for hiding this comment

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

@matejsemancik, let me provide concrete examples of both issues:

Memory Leak Example

Consider a screen component that executes a data refresh use case multiple times:

class DataScreenComponent : BaseComponent<DataViewState, DataEvent>() {
    private val refreshDataUseCase = RefreshDataUseCase()
    
    fun onRefreshClicked() {
        // This gets called every time user pulls to refresh
        refreshDataUseCase.execute {
            onNext { data -> updateState { copy(data = data) } }
        }
    }
}

What happens:

  1. First refresh: useCaseJobPool[refreshDataUseCase] = Job1
  2. Second refresh: useCaseJobPool[refreshDataUseCase] = Job2 (Job1 gets cancelled but stays in memory)
  3. Third refresh: useCaseJobPool[refreshDataUseCase] = Job3 (Job1, Job2 still in memory)
  4. After 100 refreshes: Map contains 1 active job + 99 cancelled jobs consuming memory

The cancelled jobs can't be garbage collected because the map holds references to them.

Thread Safety Example

Consider concurrent execution from different coroutine contexts:

class ConcurrentComponent : BaseComponent<ViewState, Event>() {
    private val useCase1 = NetworkUseCase()
    private val useCase2 = DatabaseUseCase()
    
    fun loadData() {
        // Both launched simultaneously on different dispatchers
        launch(Dispatchers.IO) {
            useCase1.execute { /* ... */ }  // Thread A
        }
        
        launch(Dispatchers.Main) {
            useCase2.execute { /* ... */ }  // Thread B
        }
    }
}

Race condition scenario:

// Thread A executing useCase1.execute():
if (flowUseCaseConfig.disposePrevious) {
    useCaseJobPool[this]?.cancel()  // Read operation
    // Context switch happens here!
}
useCaseJobPool[this] = newJob      // Write operation

// Thread B executing useCase2.execute() at the same time:
if (flowUseCaseConfig.disposePrevious) {
    useCaseJobPool[this]?.cancel()  // Read operation
}
useCaseJobPool[this] = anotherJob  // Write operation

This can lead to:

  • Lost updates: One thread's job overwrites another's
  • Inconsistent state: Reading while another thread is writing
  • Map corruption: On some platforms, concurrent writes can corrupt the internal map structure

Real-world Impact

Memory leak in long-running app:

// A chat screen that refreshes messages every 30 seconds
class ChatComponent {
    fun startPeriodicRefresh() {
        repeat(Int.MAX_VALUE) {
            delay(30_000)
            messageUseCase.execute { /* update messages */ }
            // After 24 hours: ~2,880 cancelled jobs in memory
        }
    }
}

Solutions:

  1. For memory leaks - Clean up completed jobs:
useCaseJobPool[this] = build(args)
    .onCompletion { 
        useCaseJobPool.remove(this@execute) // Clean up when done
    }
    .launchIn(useCaseScope)
  1. For thread safety - Use concurrent collections:
// JVM
val useCaseJobPool: ConcurrentHashMap<Any, Job> = ConcurrentHashMap()

// Multiplatform alternative
val useCaseJobPool: MutableMap<Any, Job> = Collections.synchronizedMap(mutableMapOf())

Copy link
Member Author

@matejsemancik matejsemancik Jun 20, 2025

Choose a reason for hiding this comment

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

@coderabbitai Bro, it's a Map. How does map keep reference to old canceled jobs, when each execution of UseCase cancels and overwrites the job in the map?

Copy link

@coderabbitai coderabbitai bot Jun 20, 2025

Choose a reason for hiding this comment

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

@matejsemancik You're absolutely right, and I apologize for the confusion! 🤦‍♂️

You've correctly identified the flaw in my memory leak analysis. Let me trace through what actually happens:

  1. First execution: useCaseJobPool[useCase] = Job1
  2. Second execution:
    • useCaseJobPool[useCase]?.cancel() → cancels Job1
    • useCaseJobPool[useCase] = Job2 → overwrites the reference to Job1

Once Job1's reference is overwritten, it becomes eligible for garbage collection. The map doesn't accumulate old jobs - it replaces them. Each UseCase instance can only hold one Job reference at a time.

My memory leak concern was incorrect.

The thread safety concern could still be relevant, but only in the specific scenario where the same UseCase instance is executed concurrently from different threads:

// Thread A and Thread B both calling useCase.execute() simultaneously
// Thread A: useCaseJobPool[useCase]?.cancel() 
// Thread B: useCaseJobPool[useCase]?.cancel() (might cancel Thread A's new job)
// Thread A: useCaseJobPool[useCase] = jobA
// Thread B: useCaseJobPool[useCase] = jobB (overwrites jobA)

But in practice, this scenario is probably rare since components typically execute use cases in a coordinated manner.

Thanks for catching that - the map behavior is much cleaner than I initially analyzed!

🐰

   (\   /)
  ( ._. )
 o_(")(")  oops, learned something!
✏️ Learnings added 
Learnt from: matejsemancik
PR: futuredapp/kmp-futured-template#125
File: shared/arkitekt-cr-usecases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/FlowUseCaseExecutionScope.kt:17-21
Timestamp: 2025-06-20T08:58:15.336Z
Learning: The useCaseJobPool map in FlowUseCaseExecutionScope overwrites job references for each UseCase instance rather than accumulating them, so cancelled jobs don't cause memory leaks as they become eligible for garbage collection when their references are replaced.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

fun <T : Any?> FlowUseCase<Unit, T>.execute(config: FlowUseCaseConfig.Builder<T, T>.() -> Unit) =
execute(Unit, config)

Expand Down Expand Up @@ -42,10 +48,10 @@ interface FlowUseCaseExecutionScope : CoroutineScopeOwner {
}

if (flowUseCaseConfig.disposePrevious) {
job?.cancel()
useCaseJobPool[this]?.cancel()
}

job = build(args)
useCaseJobPool[this] = build(args)
.flowOn(getWorkerDispatcher())
.onStart { flowUseCaseConfig.onStart() }
.mapNotNull { flowUseCaseConfig.onMap?.invoke(it) }
Expand All @@ -65,7 +71,7 @@ interface FlowUseCaseExecutionScope : CoroutineScopeOwner {
}
}
.catch { /* handled in onCompletion */ }
.launchIn(viewModelScope)
.launchIn(useCaseScope)
}

/**
Expand All @@ -91,10 +97,10 @@ interface FlowUseCaseExecutionScope : CoroutineScopeOwner {
}

if (flowUseCaseConfig.disposePrevious) {
job?.cancel()
useCaseJobPool[this]?.cancel()
}

job = build(args)
useCaseJobPool[this] = build(args)
.flowOn(getWorkerDispatcher())
.onStart { flowUseCaseConfig.onStart() }
.onEach { flowUseCaseConfig.onNext(it) }
Expand All @@ -113,7 +119,7 @@ interface FlowUseCaseExecutionScope : CoroutineScopeOwner {
}
}
.catch { /* handled in onCompletion */ }
.launchIn(viewModelScope)
.launchIn(useCaseScope)
}

/**
Expand Down
29 changes: 18 additions & 11 deletions ...rc/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/SingleUseCaseExecutionScope.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
package app.futured.arkitekt.crusecases.scope

import app.futured.arkitekt.crusecases.FlowUseCase
import app.futured.arkitekt.crusecases.UseCase
import app.futured.arkitekt.crusecases.error.UseCaseErrorHandler
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineStart
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Job
import kotlinx.coroutines.async
import kotlinx.coroutines.launch

interface SingleUseCaseExecutionScope : CoroutineScopeOwner {

/**
* Map of [Job] objects used to hold and cancel existing run of any [FlowUseCase] instance.
*/
val useCaseJobPool: MutableMap<Any, Job>

/**
* Asynchronously executes use case and saves it's Deferred. By default, all previous
* pending executions are canceled, this can be changed by the [config].
Expand Down Expand Up @@ -42,19 +49,19 @@ interface SingleUseCaseExecutionScope : CoroutineScopeOwner {
return@run build()
}
if (useCaseConfig.disposePrevious) {
deferred?.cancel()
useCaseJobPool[this]?.cancel()
}

useCaseConfig.onStart()
deferred = viewModelScope
useCaseJobPool[this] = useCaseScope
.async(context = getWorkerDispatcher(), start = CoroutineStart.LAZY) {
build(args)
runCatching { build(args) }
}
.also {
viewModelScope.launch(Dispatchers.Main) {
useCaseScope.launch(Dispatchers.Main) {
try {
useCaseConfig.onSuccess(it.await())
} catch (cancellation: CancellationException) {
useCaseConfig.onSuccess(it.await().getOrThrow())
} catch (_: CancellationException) {
// do nothing - this is normal way of suspend function interruption
} catch (error: Throwable) {
UseCaseErrorHandler.globalOnErrorLogger(error)
Expand Down Expand Up @@ -83,15 +90,15 @@ interface SingleUseCaseExecutionScope : CoroutineScopeOwner {
cancelPrevious: Boolean = true,
): Result<T> {
if (cancelPrevious) {
deferred?.cancel()
useCaseJobPool[this]?.cancel()
}

return try {
val newDeferred = viewModelScope.async(getWorkerDispatcher(), CoroutineStart.LAZY) {
build(args)
}.also { deferred = it }
val newDeferred = useCaseScope.async(getWorkerDispatcher(), CoroutineStart.LAZY) {
runCatching { build(args) }
}.also { useCaseJobPool[this] = it }

Result.success(newDeferred.await())
Result.success(newDeferred.await().getOrThrow())
} catch (exception: CancellationException) {
throw exception
} catch (exception: Throwable) {
Expand Down
4 changes: 2 additions & 2 deletions ...ases/src/commonMain/kotlin/app/futured/arkitekt/crusecases/scope/UseCaseExecutionScope.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface UseCaseExecutionScope :
FlowUseCaseExecutionScope {

/**
* Launch suspend [block] in [viewModelScope].
* Launch suspend [block] in [useCaseScope].
*
* Encapsulates this call with try catch block and when an exception is thrown
* then it is logged in [UseCaseErrorHandler.globalOnErrorLogger] and handled by [defaultErrorHandler].
Expand All @@ -25,7 +25,7 @@ interface UseCaseExecutionScope :
*/
@Suppress("TooGenericExceptionCaught")
fun launchWithHandler(block: suspend CoroutineScope.() -> Unit) {
viewModelScope.launch {
useCaseScope.launch {
try {
block()
} catch (exception: CancellationException) {
Expand Down
Loading
Loading