From dd137da9e61bd35cb86967d8f902c1c15aa2b54b Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Fri, 19 Aug 2022 10:36:02 +0100 Subject: [PATCH 1/2] Spotlight Next and Previous operations crash fix. --- CHANGELOG.md | 1 + .../routingsource/spotlight/operation/Next.kt | 2 +- .../spotlight/operation/Previous.kt | 2 +- .../spotlight/SpotlightTestHelper.kt | 18 +++++++ .../spotlight/operation/NextTest.kt | 50 +++++++++++++++++++ .../spotlight/operation/PreviousTest.kt | 50 +++++++++++++++++++ .../spotlight/operation/Routing.kt | 7 +++ 7 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/com/bumble/appyx/routingsource/spotlight/SpotlightTestHelper.kt create mode 100644 core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/NextTest.kt create mode 100644 core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/PreviousTest.kt create mode 100644 core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/Routing.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index df951ab56..eaf4fd8a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Pending changes +- [#91](https://github.com/bumble-tech/appyx/pull/91) – **Fixed**: Spotlight next and previous operations crash fix ## 1.0-alpha04 diff --git a/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Next.kt b/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Next.kt index 2b4988dd5..a35314244 100644 --- a/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Next.kt +++ b/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Next.kt @@ -12,7 +12,7 @@ import kotlinx.parcelize.Parcelize class Next : SpotlightOperation { override fun isApplicable(elements: RoutingElements) = - elements.any { it.fromState == INACTIVE_AFTER } + elements.any { it.fromState == INACTIVE_AFTER && it.targetState == INACTIVE_AFTER } override fun invoke(elements: RoutingElements): RoutingElements { val nextKey = diff --git a/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Previous.kt b/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Previous.kt index 5f5339d9b..fe2588c9c 100644 --- a/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Previous.kt +++ b/core/src/main/java/com/bumble/appyx/routingsource/spotlight/operation/Previous.kt @@ -12,7 +12,7 @@ import kotlinx.parcelize.Parcelize class Previous : SpotlightOperation { override fun isApplicable(elements: RoutingElements) = - elements.any { it.fromState == INACTIVE_BEFORE } + elements.any { it.fromState == INACTIVE_BEFORE && it.targetState == INACTIVE_BEFORE } override fun invoke( elements: RoutingElements diff --git a/core/src/test/java/com/bumble/appyx/routingsource/spotlight/SpotlightTestHelper.kt b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/SpotlightTestHelper.kt new file mode 100644 index 000000000..9fa386e0c --- /dev/null +++ b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/SpotlightTestHelper.kt @@ -0,0 +1,18 @@ +package com.bumble.appyx.routingsource.spotlight + +import com.bumble.appyx.core.routing.Operation +import com.bumble.appyx.core.routing.RoutingKey +import com.bumble.appyx.routingsource.spotlight.operation.Routing + +internal fun spotlightElement( + element: T, + key: RoutingKey = RoutingKey(routing = element), + fromState: Spotlight.TransitionState, + targetState: Spotlight.TransitionState, + operation: Operation = Operation.Noop() +) = SpotlightElement( + key = key, + fromState = fromState, + targetState = targetState, + operation = operation +) diff --git a/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/NextTest.kt b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/NextTest.kt new file mode 100644 index 000000000..fbfb24362 --- /dev/null +++ b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/NextTest.kt @@ -0,0 +1,50 @@ +package com.bumble.appyx.routingsource.spotlight.operation + +import com.bumble.appyx.routingsource.spotlight.Spotlight +import com.bumble.appyx.routingsource.spotlight.spotlightElement +import org.junit.Assert.assertEquals +import org.junit.Test + +internal class NextTest { + + @Test + fun `Given last element is in transition When next called Then operation is not applicable`() { + val firstElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.INACTIVE_AFTER, + targetState = Spotlight.TransitionState.INACTIVE_BEFORE, + ) + val lastElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.INACTIVE_AFTER, + targetState = Spotlight.TransitionState.ACTIVE, + ) + val elements = listOf(firstElement, lastElement) + val operation = Next() + + val applicable = operation.isApplicable(elements) + + assertEquals(applicable, false) + } + + @Test + fun `Given last element is not in transition When next called Then operation is applicable`() { + val firstElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.ACTIVE, + targetState = Spotlight.TransitionState.ACTIVE, + ) + val lastElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.INACTIVE_AFTER, + targetState = Spotlight.TransitionState.INACTIVE_AFTER, + ) + val elements = listOf(firstElement, lastElement) + val operation = Next() + + val applicable = operation.isApplicable(elements) + + assertEquals(applicable, true) + } + +} diff --git a/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/PreviousTest.kt b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/PreviousTest.kt new file mode 100644 index 000000000..6814e5536 --- /dev/null +++ b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/PreviousTest.kt @@ -0,0 +1,50 @@ +package com.bumble.appyx.routingsource.spotlight.operation + +import com.bumble.appyx.routingsource.spotlight.Spotlight +import com.bumble.appyx.routingsource.spotlight.spotlightElement +import org.junit.Assert.assertEquals +import org.junit.Test + +internal class PreviousTest { + + @Test + fun `Given first element is in transition When previous called Then operation is not applicable`() { + val firstElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.INACTIVE_AFTER, + targetState = Spotlight.TransitionState.INACTIVE_BEFORE, + ) + val lastElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.ACTIVE, + targetState = Spotlight.TransitionState.INACTIVE_BEFORE, + ) + val elements = listOf(firstElement, lastElement) + val operation = Previous() + + val applicable = operation.isApplicable(elements) + + assertEquals(applicable, false) + } + + @Test + fun `Given first element is not in transition When previous called Then operation is applicable`() { + val firstElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.ACTIVE, + targetState = Spotlight.TransitionState.ACTIVE, + ) + val lastElement = spotlightElement( + element = Routing.Routing1, + fromState = Spotlight.TransitionState.INACTIVE_BEFORE, + targetState = Spotlight.TransitionState.INACTIVE_BEFORE, + ) + val elements = listOf(firstElement, lastElement) + val operation = Previous() + + val applicable = operation.isApplicable(elements) + + assertEquals(applicable, true) + } + +} diff --git a/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/Routing.kt b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/Routing.kt new file mode 100644 index 000000000..52d4871c4 --- /dev/null +++ b/core/src/test/java/com/bumble/appyx/routingsource/spotlight/operation/Routing.kt @@ -0,0 +1,7 @@ +package com.bumble.appyx.routingsource.spotlight.operation + +internal sealed class Routing { + object Routing1 : Routing() + object Routing2 : Routing() + object Routing3 : Routing() +} From 3d5f8082061e05581d4fe7c95ff5ac79251e2267 Mon Sep 17 00:00:00 2001 From: andreykovalev Date: Fri, 19 Aug 2022 11:54:23 +0100 Subject: [PATCH 2/2] Use assertFalse assertTrue instead of assertEquals --- .../bumble/appyx/navmodel/spotlight/operation/NextTest.kt | 7 ++++--- .../appyx/navmodel/spotlight/operation/PreviousTest.kt | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/NextTest.kt b/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/NextTest.kt index 255d20c1b..a74c588c6 100644 --- a/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/NextTest.kt +++ b/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/NextTest.kt @@ -5,7 +5,8 @@ import com.bumble.appyx.navmodel.spotlight.Spotlight.TransitionState.INACTIVE_AF import com.bumble.appyx.navmodel.spotlight.Spotlight.TransitionState.INACTIVE_BEFORE import com.bumble.appyx.navmodel.spotlight.operation.Routing.Routing1 import com.bumble.appyx.navmodel.spotlight.spotlightElement -import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Test internal class NextTest { @@ -27,7 +28,7 @@ internal class NextTest { val applicable = operation.isApplicable(elements) - assertEquals(applicable, false) + assertFalse(applicable) } @Test @@ -47,7 +48,7 @@ internal class NextTest { val applicable = operation.isApplicable(elements) - assertEquals(applicable, true) + assertTrue(applicable) } } diff --git a/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/PreviousTest.kt b/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/PreviousTest.kt index 318de28e7..452fca6cb 100644 --- a/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/PreviousTest.kt +++ b/core/src/test/java/com/bumble/appyx/navmodel/spotlight/operation/PreviousTest.kt @@ -5,7 +5,8 @@ import com.bumble.appyx.navmodel.spotlight.Spotlight.TransitionState.INACTIVE_AF import com.bumble.appyx.navmodel.spotlight.Spotlight.TransitionState.INACTIVE_BEFORE import com.bumble.appyx.navmodel.spotlight.operation.Routing.Routing1 import com.bumble.appyx.navmodel.spotlight.spotlightElement -import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue import org.junit.Test internal class PreviousTest { @@ -27,7 +28,7 @@ internal class PreviousTest { val applicable = operation.isApplicable(elements) - assertEquals(applicable, false) + assertFalse(applicable) } @Test @@ -47,7 +48,7 @@ internal class PreviousTest { val applicable = operation.isApplicable(elements) - assertEquals(applicable, true) + assertTrue(applicable) } }