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.

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

Increase code coverage (pt4). #391

Merged
merged 4 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions formula-android-tests/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,19 @@
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>

<activity android:name="com.instacart.formula.NonBoundActivityTest$TestActivity">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>

<activity android:name="com.instacart.formula.NonBoundFragmentActivityTest$TestActivity">
<intent-filter>
<action android:name="android.intent.action.MAIN" />
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,12 @@ class ActivityLifecycleEventTest {
val expected = listOf(INITIALIZED) + lifecycle + lifecycle
assertThat(events).containsExactlyElementsIn(expected).inOrder()
}

@Test fun `calling onPreCreate() twice will throw an exception`() {
val activity = scenario.activity()
val result = runCatching { FormulaAndroid.onPreCreate(activity, null) }
assertThat(result.exceptionOrNull()).hasMessageThat().contains(
"Activity TestActivity was already initialized. Did you call FormulaAndroid.onPreCreate() twice?"
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import com.instacart.formula.android.BackCallback
import com.instacart.formula.android.FormulaFragment
import com.instacart.formula.android.FragmentEnvironment
import com.instacart.formula.android.FragmentStore
import com.instacart.formula.android.events.FragmentLifecycleEvent
import com.instacart.formula.test.TestBackCallbackRenderModel
import com.instacart.formula.test.TestKey
import com.instacart.formula.test.TestKeyWithId
Expand Down Expand Up @@ -43,6 +44,7 @@ class FormulaFragmentTest {
private var onPreCreated: (TestFragmentActivity) -> Unit = {}
private var updateThreads = linkedSetOf<Thread>()
private val errors = mutableListOf<Throwable>()
private val fragmentLifecycleEvents = mutableListOf<FragmentLifecycleEvent>()
private val formulaRule = TestFormulaRule(
initFormula = { app ->
val environment = FragmentEnvironment(
Expand Down Expand Up @@ -74,6 +76,9 @@ class FormulaFragmentTest {
stateChanges(it)
}
))
},
onFragmentLifecycleEvent = {
fragmentLifecycleEvents.add(it)
}
)
}
Expand All @@ -83,6 +88,7 @@ class FormulaFragmentTest {
cleanUp = {
lastState = null
updateThreads = linkedSetOf()
fragmentLifecycleEvents.clear()
}
)

Expand All @@ -104,6 +110,11 @@ class FormulaFragmentTest {
navigateBack()

assertThat(activeContracts()).containsExactly(TestKey()).inOrder()

assertThat(fragmentLifecycleEvents).hasSize(3)
assertThat(fragmentLifecycleEvents[0]).isInstanceOf(FragmentLifecycleEvent.Added::class.java)
assertThat(fragmentLifecycleEvents[1]).isInstanceOf(FragmentLifecycleEvent.Added::class.java)
assertThat(fragmentLifecycleEvents[2]).isInstanceOf(FragmentLifecycleEvent.Removed::class.java)
}

@Test fun `navigating forward should have both keys in backstack`() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class FragmentLifecycleTest {
}
}
bind(featureFactory)
}
},
)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package com.instacart.formula

import android.app.Activity
import androidx.fragment.app.FragmentActivity
import androidx.test.core.app.ActivityScenario
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith

/**
* Tests that formula-android module handles non-bound activities gracefully.
*/
@RunWith(AndroidJUnit4::class)
class NonBoundActivityTest {
class TestActivity : Activity()

private val formulaRule = TestFormulaRule(
initFormula = { app ->
FormulaAndroid.init(app) {}
}
)

private val activityRule = ActivityScenarioRule(TestActivity::class.java)

@get:Rule
val rule = RuleChain.outerRule(formulaRule).around(activityRule)
lateinit var scenario: ActivityScenario<TestActivity>

@Before
fun setup() {
scenario = activityRule.scenario
}

@Test
fun `full lifecycle`() {
scenario.recreate()
scenario.close()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.instacart.formula

import androidx.fragment.app.FragmentActivity
import androidx.test.core.app.ActivityScenario
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith

/**
* Tests that formula-android module handles non-bound activities gracefully.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: fragment activities?

*/
@RunWith(AndroidJUnit4::class)
class NonBoundFragmentActivityTest {
class TestActivity : FragmentActivity()

private val formulaRule = TestFormulaRule(
initFormula = { app ->
FormulaAndroid.init(app) {}
}
)

private val activityRule = ActivityScenarioRule(TestActivity::class.java)

@get:Rule
val rule = RuleChain.outerRule(formulaRule).around(activityRule)
lateinit var scenario: ActivityScenario<TestActivity>

@Before
fun setup() {
scenario = activityRule.scenario
}

@Test
fun `full lifecycle`() {
scenario.recreate()
scenario.close()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import kotlin.reflect.KClass
*/
class ActivityConfigurator {
internal class Binding<A : FragmentActivity>(
val init: ActivityStoreContext<A>.() -> ActivityStore<A>?
val init: ActivityStoreContext<A>.() -> ActivityStore<A>
)

internal val bindings = mutableMapOf<KClass<*>, Binding<*>>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import com.instacart.formula.android.FragmentId
* Models when a fragment key is attached and detached. Provides a way to indicate
* when to initialize state stream and when to destroy it.
*/
sealed class FragmentLifecycleEvent() {
sealed class FragmentLifecycleEvent {

abstract val fragmentId: FragmentId

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ internal class ActivityStoreFactory internal constructor(
?: return null

val activityDelegate = ActivityStoreContextImpl<A>()
return initializer.init.invoke(activityDelegate)?.let { store ->
return initializer.init.invoke(activityDelegate).let { store ->
ActivityManager(
environment = environment,
delegate = activityDelegate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ internal class AppManager(
private fun <A : FragmentActivity> findOrInitActivityStore(
activity: FragmentActivity, savedState: Bundle?
): ActivityManager<A> {
val key = findOrGenerateActivityKey(activity, savedState) // generate new key
if (activityToKeyMap.containsKey(activity)) {
throw IllegalStateException("Activity ${activity::class.java.simpleName} was already initialized. Did you call FormulaAndroid.onPreCreate() twice?")
}

val key = savedState?.getString(BUNDLE_KEY) // Activity recreated, let's use saved key
?: UUID.randomUUID().toString() // New activity, create new key
activityToKeyMap[activity] = key

val cached = componentMap[key] as? ActivityManager<A>?
Expand All @@ -121,13 +126,4 @@ internal class AppManager(
val component = componentMap.remove(key)
component?.dispose()
}

/**
* Key is persisted across configuration changes.
*/
private fun findOrGenerateActivityKey(activity: Activity, savedState: Bundle?): String {
return (activityToKeyMap[activity] // Try the map
?: savedState?.getString(BUNDLE_KEY) // Try the bundle
?: UUID.randomUUID().toString())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.instacart.formula.android.internal

import android.view.LayoutInflater
import android.view.ViewGroup
import androidx.annotation.VisibleForTesting
import com.instacart.formula.android.FeatureView
import com.instacart.formula.android.ViewFactory
import com.instacart.formula.android.FeatureEvent
Expand All @@ -18,9 +19,22 @@ internal class FormulaFragmentViewFactory(
private var factory: ViewFactory<Any>? = null

override fun create(inflater: LayoutInflater, container: ViewGroup?): FeatureView<Any> {
val viewFactory = viewFactory()
val delegate = environment.fragmentDelegate
return delegate.createView(fragmentId, viewFactory, inflater, container)
}

@VisibleForTesting
internal fun viewFactory(): ViewFactory<Any> {
return factory ?: findViewFactory().apply {
factory = this
}
}

private fun findViewFactory(): ViewFactory<Any> {
val key = fragmentId.key
val featureEvent = featureProvider.getFeature(fragmentId) ?: throw IllegalStateException("Could not find feature for $key.")
val viewFactory = factory ?: when (featureEvent) {
return when (featureEvent) {
is FeatureEvent.MissingBinding -> {
throw IllegalStateException("Missing feature factory or integration for $key. Please check your FragmentStore configuration.")
}
Expand All @@ -31,8 +45,5 @@ internal class FormulaFragmentViewFactory(
featureEvent.feature.viewFactory
}
}
this.factory = viewFactory
val delegate = environment.fragmentDelegate
return delegate.createView(fragmentId, viewFactory, inflater, container)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ internal class FragmentFlowRenderView(
) {

private var fragmentState: FragmentState? = null
private var features: Map<FragmentId, FeatureEvent> = emptyMap()
private val visibleFragments: LinkedList<Fragment> = LinkedList()

private val featureProvider = object : FeatureProvider {
override fun getFeature(id: FragmentId): FeatureEvent? {
return fragmentState?.features?.get(id)
return features[id]
}
}

Expand All @@ -53,10 +54,7 @@ internal class FragmentFlowRenderView(
super.onFragmentViewCreated(fm, f, v, savedInstanceState)

visibleFragments.add(f)

fragmentState?.let {
updateVisibleFragments(it)
}
updateVisibleFragments()

onFragmentViewStateChanged(f.getFormulaFragmentId(), true)
notifyLifecycleStateChanged(f, Lifecycle.State.CREATED)
Expand Down Expand Up @@ -93,7 +91,10 @@ internal class FragmentFlowRenderView(
override fun onFragmentAttached(fm: FragmentManager, f: Fragment, context: Context) {
super.onFragmentAttached(fm, f, context)
if (FragmentLifecycle.shouldTrack(f)) {
onLifecycleEvent(FragmentLifecycle.createAddedEvent(f))
val event = FragmentLifecycleEvent.Added(
fragmentId = f.getFormulaFragmentId(),
)
onLifecycleEvent(event)
} else {
fragmentEnvironment.logger("Ignoring attach event for fragment: $f")
}
Expand All @@ -103,8 +104,12 @@ internal class FragmentFlowRenderView(
super.onFragmentDetached(fm, f)

// Only trigger detach, when fragment is actually being removed from the backstack
if (FragmentLifecycle.shouldTrack(f) && !FragmentLifecycle.isKept(fm, f)) {
val event = FragmentLifecycle.createRemovedEvent(f)
if (FragmentLifecycle.shouldTrack(f) && f.isRemoving) {
val formulaFragment = f as? BaseFormulaFragment<*>
val event = FragmentLifecycleEvent.Removed(
fragmentId = f.getFormulaFragmentId(),
lastState = formulaFragment?.currentState(),
)
onLifecycleEvent(event)
}
}
Expand All @@ -118,7 +123,8 @@ internal class FragmentFlowRenderView(
Utils.assertMainThread()

fragmentState = state
updateVisibleFragments(state)
features = state.features
updateVisibleFragments()
}

fun onBackPressed(): Boolean {
Expand Down Expand Up @@ -149,7 +155,8 @@ internal class FragmentFlowRenderView(
onLifecycleState.invoke(fragment.getFormulaFragmentId(), newState)
}

private fun updateVisibleFragments(state: FragmentState) {
private fun updateVisibleFragments() {
val state = fragmentState ?: return
visibleFragments.forEachIndices { fragment ->
if (fragment is BaseFormulaFragment<*>) {
state.outputs[fragment.getFormulaFragmentId()]?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ package com.instacart.formula.android.internal
import android.os.Bundle
import androidx.fragment.app.Fragment
import androidx.fragment.app.FragmentInspector
import androidx.fragment.app.FragmentManager
import com.instacart.formula.android.FragmentId
import com.instacart.formula.android.FragmentKey
import com.instacart.formula.android.BaseFormulaFragment
import com.instacart.formula.android.FormulaFragment
import com.instacart.formula.android.events.FragmentLifecycleEvent
import java.util.UUID

/**
Expand All @@ -19,19 +17,6 @@ internal object FragmentLifecycle {
internal fun shouldTrack(fragment: Fragment): Boolean {
return !fragment.retainInstance && !FragmentInspector.isHeadless(fragment)
}

internal fun isKept(fragmentManager: FragmentManager, fragment: Fragment): Boolean {
return !fragment.isRemoving
}

internal fun createAddedEvent(f: Fragment): FragmentLifecycleEvent.Added {
return FragmentLifecycleEvent.Added(f.getFormulaFragmentId())
}

internal fun createRemovedEvent(f: Fragment): FragmentLifecycleEvent.Removed {
val fragment = f as? BaseFormulaFragment<*>
return FragmentLifecycleEvent.Removed(f.getFormulaFragmentId(), fragment?.currentState())
}
}

private fun Fragment.getFragmentKey(): FragmentKey {
Expand All @@ -47,12 +32,10 @@ private fun Fragment.getFragmentInstanceId(): String {
return if (this is BaseFormulaFragment<*>) {
val arguments = getOrSetArguments()
val id = arguments.getString(FormulaFragment.ARG_FORMULA_ID, "")
if (id.isNullOrBlank()) {
id.ifBlank {
val initializedId = UUID.randomUUID().toString()
arguments.putString(FormulaFragment.ARG_FORMULA_ID, initializedId)
initializedId
} else {
id
}
} else {
""
Expand Down
Loading
Loading