From 711ad4e889b9b865f3773f4814c19417e7d41e2d Mon Sep 17 00:00:00 2001 From: VysotskiVadim Date: Tue, 28 Nov 2023 13:00:31 +0100 Subject: [PATCH 1/2] added red test --- .../core/CoreRerouteTest.kt | 54 +++++++++++++++++++ .../utils/BlockResponseModifier.kt | 17 ++++++ .../utils/internal/ThreadController.kt | 4 ++ .../utils/internal/ThreadControllerTest.kt | 22 ++++++++ 4 files changed, 97 insertions(+) create mode 100644 instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/BlockResponseModifier.kt diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt index 685b653863b..61bc7ec9a19 100644 --- a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt @@ -27,6 +27,7 @@ import com.mapbox.navigation.core.reroute.NavigationRerouteController import com.mapbox.navigation.core.reroute.RerouteController import com.mapbox.navigation.core.reroute.RerouteState import com.mapbox.navigation.instrumentation_tests.R +import com.mapbox.navigation.instrumentation_tests.utils.BlockResponseModifier import com.mapbox.navigation.instrumentation_tests.utils.DelayedResponseModifier import com.mapbox.navigation.instrumentation_tests.utils.assertions.RerouteStateTransitionAssertion import com.mapbox.navigation.instrumentation_tests.utils.assertions.RouteProgressStateTransitionAssertion @@ -70,6 +71,7 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue +import org.junit.Assert.fail import org.junit.Before import org.junit.Ignore import org.junit.Rule @@ -895,6 +897,58 @@ class CoreRerouteTest : BaseCoreNoCleanUpTest() { } } + @Test + fun reroute_during_destroy_of_navigation() = sdkTest { + val mapboxNavigation = createMapboxNavigation() + val mockRoute = RoutesProvider.dc_very_short(context) + val originLocation = mockRoute.routeWaypoints.first() + val offRouteLocationUpdate = mockLocationUpdatesRule.generateLocationUpdate { + latitude = originLocation.latitude() + 0.002 + longitude = originLocation.longitude() + } + + mockWebServerRule.requestHandlers.addAll(mockRoute.mockRequestHandlers) + val rerouteResponseBlock = BlockResponseModifier() + mockWebServerRule.requestHandlers.add( + MockDirectionsRequestHandler( + profile = DirectionsCriteria.PROFILE_DRIVING_TRAFFIC, + jsonResponse = readRawFileText(context, R.raw.reroute_response_dc_very_short), + expectedCoordinates = listOf( + Point.fromLngLat( + offRouteLocationUpdate.longitude, + offRouteLocationUpdate.latitude + ), + mockRoute.routeWaypoints.last() + ), + relaxedExpectedCoordinates = true + ).apply { + jsonResponseModifier = rerouteResponseBlock + } + ) + + mapboxNavigation.startTripSession() + val routes = mapboxNavigation.requestRoutes( + RouteOptions.builder() + .applyDefaultNavigationOptions() + .applyLanguageAndVoiceUnitOptions(context) + .baseUrl(mockWebServerRule.baseUrl) + .coordinatesList(mockRoute.routeWaypoints).build() + ).getSuccessfulResultOrThrowException().routes + mapboxNavigation.setNavigationRoutes(routes) + + mockLocationReplayerRule.loopUpdateUntil(offRouteLocationUpdate) { + mapboxNavigation.routeProgressUpdates() + .filter { it.currentState == RouteProgressState.OFF_ROUTE } + .first() + } + mapboxNavigation.onDestroy() + mapboxNavigation.registerRoutesObserver { + fail("routes shouldn't be updated after destroy, but received $it") + } + rerouteResponseBlock.release() + delay(3000) + } + private fun createMapboxNavigation(customRefreshInterval: Long? = null): MapboxNavigation { var mapboxNavigation: MapboxNavigation? = null diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/BlockResponseModifier.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/BlockResponseModifier.kt new file mode 100644 index 00000000000..6dbe11c72ec --- /dev/null +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/BlockResponseModifier.kt @@ -0,0 +1,17 @@ +package com.mapbox.navigation.instrumentation_tests.utils + +import java.util.concurrent.CountDownLatch + +class BlockResponseModifier : (String) -> String { + + private val countDownLatch = CountDownLatch(1) + + fun release() { + countDownLatch.countDown() + } + + override fun invoke(p1: String): String { + countDownLatch.await() + return p1 + } +} \ No newline at end of file diff --git a/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt index 7099bd04eae..4338b56f2bb 100644 --- a/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt +++ b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt @@ -101,4 +101,8 @@ class ThreadController { val parentJob = SupervisorJob(mainRootJob) return JobControl(parentJob, CoroutineScope(parentJob + Dispatchers.Main)) } + + fun destroy() { + + } } diff --git a/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt b/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt index aa4c89b8fc1..a4d4014e467 100644 --- a/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt +++ b/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt @@ -1,7 +1,9 @@ package com.mapbox.navigation.utils.internal +import com.mapbox.navigation.testing.MainCoroutineRule import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CompletableJob import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -16,11 +18,16 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertThat import org.junit.Assert.assertTrue +import org.junit.Assert.fail +import org.junit.Rule import org.junit.Test import kotlin.coroutines.suspendCoroutine class ThreadControllerTest { + @get:Rule + private val testCoroutineRule = MainCoroutineRule() + private val threadController = ThreadController() @Test @@ -132,4 +139,19 @@ class ThreadControllerTest { mainJobController.scope.toString() ) } + + @Test + fun destroy_thread_controller() { + val handler = CompletableDeferred() + threadController.getIOScopeAndRootJob().scope.launch { + handler.await() + fail("IO scope should be cancelled") + } + threadController.getMainScopeAndRootJob().scope.launch { + handler.await() + fail("UI scope should be cancelled") + } + threadController.destroy() + handler.complete(Unit) + } } From 5818412845abb559db1f933030f0480a7b160633 Mon Sep 17 00:00:00 2001 From: VysotskiVadim Date: Tue, 28 Nov 2023 13:25:21 +0100 Subject: [PATCH 2/2] fixed reroute during destroy --- .../mapbox/navigation/core/MapboxNavigation.kt | 3 +-- .../utils/internal/ThreadController.kt | 4 +++- .../utils/internal/ThreadControllerTest.kt | 17 ++++++++++++++--- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt index dbc66dd8b7f..63e71a5417f 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt @@ -1289,8 +1289,7 @@ class MapboxNavigation @VisibleForTesting internal constructor( runInTelemetryContext { telemetry -> telemetry.destroy(this@MapboxNavigation) } - threadController.cancelAllNonUICoroutines() - threadController.cancelAllUICoroutines() + threadController.destroy() ifNonNull(reachabilityObserverId) { ReachabilityService.removeReachabilityObserver(it) reachabilityObserverId = null diff --git a/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt index 4338b56f2bb..672afd73987 100644 --- a/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt +++ b/libnavigation-util/src/main/java/com/mapbox/navigation/utils/internal/ThreadController.kt @@ -103,6 +103,8 @@ class ThreadController { } fun destroy() { - + val reason = CancellationException("thread controller is destroyed") + mainRootJob.cancel(reason) + ioRootJob.cancel(reason) } } diff --git a/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt b/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt index a4d4014e467..fc2a452477a 100644 --- a/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt +++ b/libnavigation-util/src/test/java/com/mapbox/navigation/utils/internal/ThreadControllerTest.kt @@ -16,6 +16,7 @@ import kotlinx.coroutines.runBlocking import org.hamcrest.CoreMatchers.`is` import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertThat import org.junit.Assert.assertTrue import org.junit.Assert.fail @@ -26,7 +27,7 @@ import kotlin.coroutines.suspendCoroutine class ThreadControllerTest { @get:Rule - private val testCoroutineRule = MainCoroutineRule() + val testCoroutineRule = MainCoroutineRule() private val threadController = ThreadController() @@ -143,15 +144,25 @@ class ThreadControllerTest { @Test fun destroy_thread_controller() { val handler = CompletableDeferred() + var errorMessage: String? = null threadController.getIOScopeAndRootJob().scope.launch { handler.await() - fail("IO scope should be cancelled") + errorMessage = "IO scope should be cancelled" } threadController.getMainScopeAndRootJob().scope.launch { handler.await() - fail("UI scope should be cancelled") + errorMessage = "UI scope should be cancelled" } threadController.destroy() handler.complete(Unit) + assertNull(errorMessage) + + threadController.getIOScopeAndRootJob().scope.launch { + errorMessage = "IO scope should be cancelled" + } + threadController.getMainScopeAndRootJob().scope.launch { + errorMessage = "UI scope should be cancelled" + } + assertNull(errorMessage) } }