isaac-udy/Enro

The use of ViewModelExtensions.putNavigationHandleForViewModel requires tests that are aware of application context.

committedteadrinker opened this issue · 2 comments

committedteadrinker commented

Issue

Currently, using putNavigationHandleForViewModel(NavigationKey) on unit tests that are not using AndroidJUnit4 runner would result in failure. This is due to TestNavigationHandle that's created inside this extension function relies on being aware of the application context itself.

Both of these function calls will throw some kind of NPE:

Enro/enro-test/src/main/java/dev/enro/test/TestNavigationHandle.kt

Line 54 in bcde0c3

private val lifecycle = LifecycleRegistry(this).apply { 

LifecycleRegistry(this).apply {
       currentState = Lifecycle.State.RESUMED
}

and

Enro/enro-test/src/main/java/dev/enro/test/TestNavigationHandle.kt

Line 62 in bcde0c3

override val controller: NavigationController = ApplicationProvider 

ApplicationProvider
    .getApplicationContext<Application>()
    .navigationController

Proposal

Enable passing in a null-able NavigationHandle to putNavigationHandleForViewModel in which a user can pass in any kind of mock-able NavigationHandle. If it's null, then use the current way of creating TestNavigationHandle as a default parameter.
Pros: More flexibility for the usages of this extension method, it's backwards compatible to any existing usages.
Cons: Passing around the same thing multiple times down multiple functions. This might be hard to read later on.

Other alternatives

  1. Passing null-able NavigationController and null-able Lifecycle into to putNavigationHandleForViewModel in which a user can pass in any kind of mock. If any of them are null, then the default values would be like what they currently are.
    Pros: TestNavigationHandle is always used in the test context for a specific view model with only specific properties are changeable
    Cons: extension function signature becomes a bit more confusing as it's not self explanatory.
  2. Some other ways I haven't thought of. Feel free for anyone to give more suggestions.
isaac-udy commented

Hi, thanks for writing up this issue - very detailed explanation!

I have had a little look, and I can't see any clear alternative options, so I think we should go with your proposed solution.

These arguments will need to be passed from putNavigationHandleForViewModel down into createTestNavigationHandle. I think that we can mitigate your concern with making the extension function confusing if we named these overrideNavigationController and overrideLifecycle, as well as added some JavaDocs to the comments.

Would you like to open a PR with these changes, or shall I?

isaac-udy commented

This has been resolved as of Enro 1.10.0. Apologies for not updating this issue earlier.

You can see an example of a pure JVM test using EnroTest here: https://github.com/isaac-udy/Enro/blob/main/enro/src/test/java/dev/enro/test/EnroTestJvmTest.kt

  • 🎉1