raamcosta / compose-destinations

Annotation processing library for type-safe Jetpack Compose navigation with no boilerplate.
https://composedestinations.rafaelcosta.xyz
Apache License 2.0
3.22k stars 133 forks source link

ViewModel unit test problems with SavedStateHandle.navArgs() #210

Closed luanbarbosa-sanoma closed 2 years ago

luanbarbosa-sanoma commented 2 years ago

Hi,

I'm using SavedStateHandle.navArgs() to get the navigation arguments on my ViewModel. However, on my Unit tests the call fails with NoClassDefFoundError: Could not initialize class error. I've added the value directly in my SavedStateHandle with no success.

How can I manage to test the ViewModel with the navigation arguments without having to do a UI test?

Could not initialize class com.ramcosta.composedestinations.navargs.primitives.DestinationsStringNavType
java.lang.NoClassDefFoundError: Could not initialize class com.ramcosta.composedestinations.navargs.primitives.DestinationsStringNavType

Code snippets:

ViewModel


class SearchViewModel @Inject constructor(
savedStateHandle: SavedStateHandle,
...
) {
init {
    // Test fails here
    val initialQuery = savedStateHandle.navArgs<SearchScreenNavArgs>().initialQuery
}

> Unit test

@Before fun setup() { viewModel = SearchViewModel( savedStateHandle = SavedStateHandle().apply { set("initialQuery", "") }, ... ) }

raamcosta commented 2 years ago

Hi πŸ‘‹

You can receive the initial query in the constructor of the ViewModel, making use of dependency injection principles. Then in the test you can inject whatever you want.

Let me know if this helped πŸ™‚

raamcosta commented 2 years ago

I closed this but I’ll be paying attention to your replies here πŸ˜ƒ

luanbarbosa-sanoma commented 2 years ago

Do you mean I'm not able to use either (argFrom or navArgs)? When is the case I can use them in the ViewModel and also be able to Unit test it?

SearchScreenDestination.argsFrom(savedStateHandle)
savedStateHandle.navArgs<SearchScreenNavArgs>()
data class SearchScreenNavArgs(
    val initialQuery: String = ""
)

@Destination(
    navArgsDelegate = SearchScreenNavArgs::class,
)
@Composable
fun SearchScreen(...)
luanbarbosa-sanoma commented 2 years ago

One option would be passing by the compose screen... which looks a bit bad.

@Destination
SearchScreen(initialQuery: String) { 
    vm.loadNavArgs(initialQuery) 
}
raamcosta commented 2 years ago

No, you can and should use one of those options, but do it before calling the ViewModel constructor and pass the result to it.

Are you using Hilt or any other DI framework?

luanbarbosa-sanoma commented 2 years ago

Hilt, yes.

raamcosta commented 2 years ago

Ok, so search for assisted injection. This is a good practice either way, it’s not just a workaround to this issue 😁

raamcosta commented 2 years ago

Let me try to find you an example of this

raamcosta commented 2 years ago

https://gist.github.com/manuelvicnt/437668cda3a891d347e134b1de29aee1

raamcosta commented 2 years ago

Coming from Manuel himself ☝️

luanbarbosa-sanoma commented 2 years ago

I'll give it a try. Thank you for the fast responses!

raamcosta commented 2 years ago

Hmm I actually went through this and gave a quick DM to Manuel, and it seems this is not recommended πŸ€”

https://github.com/google/dagger/issues/2287

raamcosta commented 2 years ago

So.. you can either use a setter and call it from the Composable, or you can mock the navArgs method.. Are you using a mocking library?

luanbarbosa-sanoma commented 2 years ago

Yes, Mockito.

raamcosta commented 2 years ago

Hmm not sure how to mock these methods with mockito πŸ€”

Maybe your best bet until they fix the assisted injection in Hilt is to use a setter like you said before. If you were using manual dep injection this would be trivial, so yeah, it’s annoying that this issue is worse because Hilt is missing this support.

epool commented 2 years ago

@raamcosta should this issue be reopened? I'm facing the same issue and seems related with DestinationsStringNavType, when I use the SavedStateHandle directly it works.

Does NOT work

@HiltViewModel
class CountryLocalIdConfirmationViewModel @Inject constructor(
    savedStateHandle: SavedStateHandle,
) : ViewModel() {

    private val _viewState = MutableStateFlow<ViewState>(savedStateHandle.navArgs()) // <======= Unit tests fail here
}

Does work

@HiltViewModel
class CountryLocalIdConfirmationViewModel @Inject constructor(
    savedStateHandle: SavedStateHandle,
) : ViewModel() {

    private val _viewState = MutableStateFlow(
        ViewState(
            countryLocalIdType = savedStateHandle.getOrThrow("countryLocalIdType"),
            countryLocalId = savedStateHandle.getOrThrow("countryLocalId"),
            checkBoxValue = savedStateHandle.getOrThrow("checkBoxValue"),
        )
    )
}

This is the current generated code

    override fun argsFrom(savedStateHandle: SavedStateHandle): ViewState {
        return ViewState(
            countryLocalIdType = countryLocalIdTypeEnumNavType.get(savedStateHandle, "countryLocalIdType") ?: throw RuntimeException("'countryLocalIdType' argument is mandatory, but was not present!"),
            countryLocalId = DestinationsStringNavType.get(savedStateHandle, "countryLocalId") ?: throw RuntimeException("'countryLocalId' argument is mandatory, but was not present!"),  // <======= Unit tests fail here
            checkBoxValue = DestinationsBooleanNavType.get(savedStateHandle, "checkBoxValue") ?: throw RuntimeException("'checkBoxValue' argument is not mandatory and not nullable but was not present!"),
        )
    }

maybe should generate something like this?

    override fun argsFrom(savedStateHandle: SavedStateHandle): ViewState {
        return ViewState(
            countryLocalIdType = savedStateHandle["countryLocalIdType"] ?: throw RuntimeException("'countryLocalIdType' argument is mandatory, but was not present!"),
            countryLocalId = savedStateHandle["countryLocalId"] ?: throw RuntimeException("'countryLocalId' argument is mandatory, but was not present!"),
            checkBoxValue = savedStateHandle["checkBoxValue"] ?: throw RuntimeException("'checkBoxValue' argument is not mandatory and not nullable but was not present!"),
        )
    }

or maybe making DestinationsStringNavType a class instead of an object? the generated

val countryLocalIdTypeEnumNavType = DestinationsEnumNavType(CountryLocalIdType::class.java)

seems to work fine, also DestinationsBooleanNavType.

epool commented 2 years ago

@raamcosta should this issue be reopened? I'm facing the same issue and seems related with DestinationsStringNavType, when I use the SavedStateHandle directly it works.

Does NOT work

@HiltViewModel
class CountryLocalIdConfirmationViewModel @Inject constructor(
    savedStateHandle: SavedStateHandle,
) : ViewModel() {

    private val _viewState = MutableStateFlow<ViewState>(savedStateHandle.navArgs()) // <======= Unit tests fail here
}

Does work

@HiltViewModel
class CountryLocalIdConfirmationViewModel @Inject constructor(
    savedStateHandle: SavedStateHandle,
) : ViewModel() {

    private val _viewState = MutableStateFlow(
        ViewState(
            countryLocalIdType = savedStateHandle.getOrThrow("countryLocalIdType"),
            countryLocalId = savedStateHandle.getOrThrow("countryLocalId"),
            checkBoxValue = savedStateHandle.getOrThrow("checkBoxValue"),
        )
    )
}

This is the current generated code

  override fun argsFrom(savedStateHandle: SavedStateHandle): ViewState {
      return ViewState(
          countryLocalIdType = countryLocalIdTypeEnumNavType.get(savedStateHandle, "countryLocalIdType") ?: throw RuntimeException("'countryLocalIdType' argument is mandatory, but was not present!"),
          countryLocalId = DestinationsStringNavType.get(savedStateHandle, "countryLocalId") ?: throw RuntimeException("'countryLocalId' argument is mandatory, but was not present!"),  // <======= Unit tests fail here
          checkBoxValue = DestinationsBooleanNavType.get(savedStateHandle, "checkBoxValue") ?: throw RuntimeException("'checkBoxValue' argument is not mandatory and not nullable but was not present!"),
      )
  }

maybe should generate something like this?

  override fun argsFrom(savedStateHandle: SavedStateHandle): ViewState {
      return ViewState(
          countryLocalIdType = savedStateHandle["countryLocalIdType"] ?: throw RuntimeException("'countryLocalIdType' argument is mandatory, but was not present!"),
          countryLocalId = savedStateHandle["countryLocalId"] ?: throw RuntimeException("'countryLocalId' argument is mandatory, but was not present!"),
          checkBoxValue = savedStateHandle["checkBoxValue"] ?: throw RuntimeException("'checkBoxValue' argument is not mandatory and not nullable but was not present!"),
      )
  }

or maybe making DestinationsStringNavType a class instead of an object? the generated

val countryLocalIdTypeEnumNavType = DestinationsEnumNavType(CountryLocalIdType::class.java)

seems to work fine, also DestinationsBooleanNavType.

I just realised that DestinationsStringNavType uses

import android.net.Uri
....
internal val DECODED_EMPTY_STRING: String = Uri.decode(ENCODED_EMPTY_STRING)
...
private val DECODED_DEFAULT_VALUE_STRING_PREFIX: String = Uri.decode(ENCODED_DEFAULT_VALUE_STRING_PREFIX)

that might be the reason we are not able to access it in the unit tests.

raamcosta commented 2 years ago

That is the reason, yes.

I can make it a lazy initialized field, and if the tests don't need it (it seems like that is your case) then that would work. But I still need to think more about it since what if the tests do need it? πŸ€”

I'll reopen this to see if I can come up with a better solution. Thanks @epool

raamcosta commented 2 years ago

Until then, use assited injection (if your framework allows it, as said above, Hilt as issues with it atm) and pass the arguments to the ViewModel (this is much better from a clean code perspective in my opinion).

If not, you can set the args from the outside with some setter.

raamcosta commented 2 years ago

This should be fixed guys @epool @luanbarbosa-sanoma please test version 1.x.20 and let me know!