Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
luanbarbosa-sanoma opened this issue Aug 11, 2022 · 20 comments
Closed
Assignees
Labels
question Further information is requested

Comments

@luanbarbosa-sanoma
Copy link

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
Copy link
Owner

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
Copy link
Owner

I closed this but I’ll be paying attention to your replies here 😃

@raamcosta raamcosta added the question Further information is requested label Aug 11, 2022
@luanbarbosa-sanoma
Copy link
Author

luanbarbosa-sanoma commented Aug 11, 2022

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
Copy link
Author

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

@Destination
SearchScreen(initialQuery: String) { 
    vm.loadNavArgs(initialQuery) 
}

@raamcosta
Copy link
Owner

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
Copy link
Author

Hilt, yes.

@raamcosta
Copy link
Owner

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

@raamcosta
Copy link
Owner

Let me try to find you an example of this

@raamcosta
Copy link
Owner

@raamcosta
Copy link
Owner

Coming from Manuel himself ☝️

@luanbarbosa-sanoma
Copy link
Author

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

@raamcosta
Copy link
Owner

Hmm I actually went through this and gave a quick DM to Manuel, and it seems this is not recommended 🤔

google/dagger#2287

@raamcosta
Copy link
Owner

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
Copy link
Author

Yes, Mockito.

@raamcosta
Copy link
Owner

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
Copy link

epool commented Sep 6, 2022

@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
Copy link

epool commented Sep 6, 2022

@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
Copy link
Owner

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 raamcosta reopened this Sep 6, 2022
@raamcosta
Copy link
Owner

raamcosta commented Sep 6, 2022

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 raamcosta self-assigned this Sep 13, 2022
@raamcosta raamcosta moved this to In Progress in Compose Destinations Sep 13, 2022
Repository owner moved this from In Progress to Done in Compose Destinations Sep 13, 2022
@raamcosta
Copy link
Owner

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

@raamcosta raamcosta moved this from Done to Released in Compose Destinations Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
Archived in project
Development

No branches or pull requests

3 participants