Status Update
Comments
do...@google.com <do...@google.com> #2
In case it's useful, here's the stacktrace when running the following tests:
java.lang.RuntimeException: Method encode in android.net.Uri not mocked. See https://developer.android.com/r/studio-ui/build/not-mocked for details.
at android.net.Uri.encode(Uri.java)
at androidx.navigation.NavType$Companion$StringType$1.serializeAsValue(NavType.kt:834)
at androidx.navigation.NavType$Companion$StringType$1.serializeAsValue(NavType.kt:803)
at androidx.navigation.serialization.RouteBuilder$Filled$addArg$2.invoke(RouteBuilder.kt:114)
at androidx.navigation.serialization.RouteBuilder$Filled$addArg$2.invoke(RouteBuilder.kt:110)
at androidx.navigation.serialization.RouteBuilder$Builder.apply(RouteBuilder.kt:214)
at androidx.navigation.serialization.RouteBuilder$Filled.addArg(RouteBuilder.kt:110)
at androidx.navigation.serialization.RouteEncoder.encodeSerializableValue(RouteEncoder.kt:66)
at kotlinx.serialization.encoding.Encoder$DefaultImpls.encodeNullableSerializableValue(Encoding.kt:299)
at kotlinx.serialization.encoding.AbstractEncoder.encodeNullableSerializableValue(AbstractEncoder.kt:18)
at kotlinx.serialization.encoding.AbstractEncoder.encodeNullableSerializableElement(AbstractEncoder.kt:90)
at com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsDestination.write$Self$interests_demoDebug(InterestsNavigation.kt:25)
at com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsDestination$$serializer.serialize(InterestsNavigation.kt:25)
at com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsDestination$$serializer.serialize(InterestsNavigation.kt:25)
at kotlinx.serialization.encoding.Encoder$DefaultImpls.encodeSerializableValue(Encoding.kt:279)
at kotlinx.serialization.encoding.AbstractEncoder.encodeSerializableValue(AbstractEncoder.kt:18)
at androidx.navigation.serialization.RouteEncoder.encodeRouteWithArgs(RouteEncoder.kt:49)
at androidx.navigation.serialization.RouteSerializerKt.generateRouteWithArgs(RouteSerializer.kt:141)
at androidx.navigation.testing.SavedStateHandleFactoryKt.invoke(SavedStateHandleFactory.kt:48)
at androidx.navigation.testing.SavedStateHandleFactoryKt.invoke$default(SavedStateHandleFactory.kt:38)
at com.google.samples.apps.nowinandroid.interests.InterestsViewModelTest.setup(InterestsViewModelTest.kt:61)
jb...@google.com <jb...@google.com>
ap...@google.com <ap...@google.com> #3
Branch: androidx-main
commit ee37cb1535e3b4d9b27d5288e2a4c98b4773d4eb
Author: Clara Fok <clarafok@google.com>
Date: Thu Jun 06 15:53:07 2024
Refactor generateRoutePattern
Add new internal KSerializer<T>.forEachIndexed() function and applies an operation lambda on each child argument of the serializer. This function takes a Map<KType, NavType<*>>, maps each argument back to a NavType<Any?>, and provides this navType into the operation lambda.
Then extracted common RouteBuilder logic shared between RoutePattern and RouteFilled into a separate RouteBuilder class (old RouteBuilder renamed to OldRouteBuilder temporarily, entire OldRouteBuilder class to be deleted in next CL).
Finally refactored generateRoutePattern to use the new forEachIndexed() and new RouteBuilder.
Test: existing tests
Bug: 340966212
Change-Id: I7dc524d2f49fdcdc6925a5cb8c9649aa8c26bef2
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteBuilder.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteEncoder.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteSerializer.kt
ap...@google.com <ap...@google.com> #4
Branch: androidx-main
commit dd314d59da662fc85fa187f2df9b3a42b93fc586
Author: Clara Fok <clarafok@google.com>
Date: Thu Jun 06 16:08:08 2024
Refactor generateRouteWithArgs
Add new internal KSerializer<T>.forEachIndexed() function and applies an operation lambda on each child argument of the serializer. This function takes a Map<String, NavType<Any?>> and provides this navType into the operation lambda.
Refactored RouteEncoder to output a map of argName to List<String> (the value is the output of NavType.serializeAsValue/serializeAsValues).
Finally refactored generateRouteWithArgs to use the new forEachIndexed(), new RouteBuilder, and new RouteEncoder.
Test: existing tests
Bug: 340966212
Change-Id: Ic2ebe6cc7c396254c10a03c26639432d9c6c6fe3
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteBuilder.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteEncoder.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteSerializer.kt
ap...@google.com <ap...@google.com> #5
Branch: androidx-main
commit b333eceb8ade397c6418f8a6ecc14f24e88c484f
Author: Clara Fok <clarafok@google.com>
Date: Fri Jun 14 12:33:38 2024
Refactor SavedStateHandleFactory
Previous implementation delegates to NavDeepLink operations which has dependencies on Uri, thus limiting this API to androidTests.
Refactor away from NavDeepLink and instead manually replicate the steps that an argument value would go through.
However, since bundle is deeply integrated into the parsing of arguments by NavType, the non-instrumented test will require robolectric in order to properly support bundle.
Test: ./gradlew navigation:navigation-testing:test
Bug: 340966212
Relnote: "SavedStateHandleFactory test api can now be used in non-android tests but will require robolectric to support argument parsing with bundles."
Change-Id: I76cdc4b40474680dbdb58e6b8a6451f0e228f9b1
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteEncoder.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/RouteSerializer.kt
M navigation/navigation-testing/build.gradle
M navigation/navigation-testing/src/main/java/androidx/navigation/testing/SavedStateHandleFactory.kt
M navigation/navigation-testing/src/test/java/androidx/navigation/testing/TestSavedStateHandleFactory.kt
cl...@google.com <cl...@google.com> #6
Fixed internally and available in navigation-2.8.0-rc01
.
Note that Android Bundle is deeply integrated into the parsing of arguments by NavType and cannot be removed, hence non-instrumented tests will require robolectric in order to properly support bundles.
ap...@google.com <ap...@google.com> #7
Branch: androidx-main
commit df6f97f72af9fa61365b44af75905ddd9039bb26
Author: Clara Fok <clarafok@google.com>
Date: Tue Jun 18 10:47:42 2024
Update SavedStateHandleFactory kdoc
Clarify that android Bundles cannot be avoided at this stage and non-instrumented tests need to be run with robolectric.
Test: TH
Bug: 340966212
Relnote: "To use SavedStateHandle test factory in non-instrumented tests, robolectric is required. This is because android Bundle is necessarily integrated into the parsing of nav arguments."
Change-Id: I8047c1a2d2dff4cb2efd0452407287738f2ce157
M navigation/navigation-testing/src/main/java/androidx/navigation/testing/SavedStateHandleFactory.kt
do...@google.com <do...@google.com> #8
Thanks for doing this. Whilst this is technically an improvement, the fact remains that if you want to test a ViewModel which accepts navigation arguments you now need to introduce a dependency on Robolectric which is a fairly major blocker for many developers. Robolectric slows down tests and reduces test fidelity.
Is there no way of removing the Bundle
dependency? Or perhaps moving Bundle
into AndroidX?
pr...@google.com <pr...@google.com> #9
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.navigation:navigation-common:2.8.0-beta04
androidx.navigation:navigation-testing:2.8.0-beta04
do...@google.com <do...@google.com> #10
I created a follow up bug to track the Android dependency forcing unit->instrumented tests for ViewModels.
Description
Component used: Navigation Version used: 2.8.0-beta01 Devices/Android versions reproduced on: all
aosp/3073537 introduced a new API for creating a
SavedStateHandle
using an object for the route parameter e.g.SavedStateHandle(route = MyDestination)
.Internally, this constructor relies on
android.net.Uri
which means that you can only call it from an instrumented test, not a unit test.This is undesirable since one of the use cases for this API is to be able to unit test ViewModels by passing them a
SavedStateHandle
containing a route.Example:https://github.com/android/nowinandroid/pull/1413/files#diff-bbcd9ff3a5be72096922a84b290125441e34d8183492cd27536e343a903579a8R57