Skip to content
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

Add test params for retained state registry #1655

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −6
Draft

Add test params for retained state registry #1655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions .../commonMain/kotlin/com/slack/circuit/foundation/internal/WithCompositionLocalProviders.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.InternalComposeApi
import androidx.compose.runtime.ProvidedValue
import androidx.compose.runtime.currentComposer
import com.slack.circuit.runtime.InternalCircuitApi

/**
* A slightly more efficient version of [withCompositionLocalProvider] that only accepts a single
* [value].
*/
@Composable
@InternalCircuitApi
@OptIn(InternalComposeApi::class)
internal fun <R> withCompositionLocalProvider(
public fun <R> withCompositionLocalProvider(
value: ProvidedValue<*>,
content: @Composable () -> R,
): R {
Expand All @@ -30,8 +32,9 @@ internal fun <R> withCompositionLocalProvider(
* @param content The content to provide the value to.
*/
@Composable
@InternalCircuitApi
@OptIn(InternalComposeApi::class)
internal fun <R> withCompositionLocalProvider(
public fun <R> withCompositionLocalProvider(
vararg values: ProvidedValue<*>,
content: @Composable () -> R,
): R {
Expand Down
2 changes: 1 addition & 1 deletion circuit-retained/src/commonMain/kotlin/com/slack/circuit/retained/RetainedStateRegistry.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ internal class RetainedStateRegistryImpl(retained: MutableMap<String, List<Any?>

override fun registerValue(key: String, valueProvider: RetainedValueProvider): Entry {
require(key.isNotBlank()) { "Registered key is empty or blank" }
valueProviders.getOrPut(key) { mutableListOf() }.add(valueProvider)
valueProviders.getOrPut(key, ::mutableListOf).add(valueProvider)
return object : Entry {
override fun unregister() {
val list = valueProviders.remove(key)
Expand Down
30 changes: 27 additions & 3 deletions circuit-test/src/commonMain/kotlin/com/slack/circuit/test/PresenterTestExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.ReceiveTurbine
import app.cash.turbine.test
import com.slack.circuit.foundation.internal.withCompositionLocalProvider
import com.slack.circuit.retained.LocalRetainedStateRegistry
import com.slack.circuit.retained.RetainedStateRegistry
import com.slack.circuit.runtime.CircuitUiState
import com.slack.circuit.runtime.InternalCircuitApi
import com.slack.circuit.runtime.presenter.Presenter
import kotlin.time.Duration

Expand All @@ -28,9 +32,10 @@ public suspend fun <UiState : CircuitUiState> Presenter<UiState>.test(
timeout: Duration? = null,
name: String? = null,
policy: SnapshotMutationPolicy<UiState> = structuralEqualityPolicy(),
retainedStateRegistry: RetainedStateRegistry = RetainedStateRegistry(),
Copy link
Contributor

@chrisbanes chrisbanes Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

WDYT to making this nullable (and default to null)?

Providing a default retainedStateRegistry is a bit of a foot gun, as it's so easy for devs to ignore. At least if we force people to provide a value, it's on them to track. If they don't provide a value, nothing is retained anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd be game for that or default to NoopStateRegistry, though I'm still not sure this API is necessary to expose at the top level as the only use case right now was automatic-cleanup.

Copy link
Contributor

@chrisbanes chrisbanes Sep 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, this is basically just recreating NoopStateRegistry. Maybe this PR isn't needed: if people want to provide a real registry in tests then they're free to do so. Might be worth a doc call out though.

block: suspend CircuitReceiveTurbine<UiState>.() -> Unit,
) {
presenterTestOf({ present() }, timeout, name, policy, block)
presenterTestOf({ present() }, timeout, name, policy, retainedStateRegistry, block)
}

/**
Expand All @@ -41,6 +46,7 @@ public suspend fun <UiState : CircuitUiState> Presenter<UiState>.test(
* @param timeout an optional timeout for the test. Defaults to 1 second (in Turbine) if undefined.
* @param policy a policy to controls how state changes are compared in
* [CircuitReceiveTurbine.awaitItem].
* @param retainedStateRegistry a [RetainedStateRegistry] that can operate
* @param block the block to invoke.
* @see moleculeFlow
* @see test
Expand All @@ -50,9 +56,27 @@ public suspend fun <UiState : CircuitUiState> presenterTestOf(
timeout: Duration? = null,
name: String? = null,
policy: SnapshotMutationPolicy<UiState> = structuralEqualityPolicy(),
retainedStateRegistry: RetainedStateRegistry = RetainedStateRegistry(),
block: suspend CircuitReceiveTurbine<UiState>.() -> Unit,
) {
moleculeFlow(RecompositionMode.Immediate, presentFunction).test(timeout, name) {
asCircuitReceiveTurbine(policy).block()
try {
moleculeFlow(RecompositionMode.Immediate, decorate(presentFunction, retainedStateRegistry))
.test(timeout, name) { asCircuitReceiveTurbine(policy).block() }
} finally {
retainedStateRegistry.forgetUnclaimedValues()
}
}

@OptIn(InternalCircuitApi::class)
private fun <UiState : CircuitUiState> decorate(
presentFunction: @Composable () -> UiState,
retainedStateRegistry: RetainedStateRegistry,
): @Composable () -> UiState {
val newFunction =
@Composable {
withCompositionLocalProvider(LocalRetainedStateRegistry provides retainedStateRegistry) {
presentFunction()
}
}
return newFunction
}
Loading