Prism: Question: INavigationServiceExtensions.NavigateAsync not testable

[jBijsterboschNL]

Description

Why is there a INavigationServiceExtensions with a NavigateAsync(name, parameters, useModalNavigation, animated) method? This way I'm unable to verify it's usage in a unit test because it is an extension. Why is this not directly implemented on the PageNavigationService?

Steps to Reproduce

1. Create a unit test.
2. Mock the INavigationService.
3. Try to Verify NavigateAsync(this INavigationService navigationService, string name, NavigationParameters parameters = null, bool? useModalNavigation = null, bool animated = true)

Expected Behavior

Successful unit test

Actual Behavior

Unit test failed with message: System.NotSupportedException : Invalid verify on an extension method: m => m.NavigateAsync(It.IsAny(), null, (Nullable)False, True)

[brianlagunas]

INavigationSerrvice is being used across platforms, which is why there are extensions for platform specific scenarios. You can mock it and you can test it. For now you have to have your mock derive from the prism implementation and then override the methods you are testing. This has been improved in the latest CI build to where this is no longer necessary and you can have your own POCO class implement the required interfaces.

[DavidStrickland0]

Forced implementations of the INavigationService calling into an Internal only Interface Im afraid is code stink. Its basically bypassing the one rule of Interface Extension that being that an Interface Extension can only access the interfaces properties and methods the interface exposes. Expose the internal Interface so it can be Mocked or implement a seperate Contract to use with Forms that inherits from the INavigationService.
Inheriting from the Prism implementation isnt a Mock. You dont "Mock" concretes you "Mock" Interfaces. Now we have to run the constructor of the base class to Overload the method that our implementation needs to call. So we are no longer just testing our code we also have to test the constructor.
Our Method is calling one of the Extended Methods namely NavigateAsync(this INavigationService navigationService, string name, NavigationParameters parameters = null, bool? useModalNavigation = null, bool animated = true); Even if we do Inherit we cant override it because its not in the base class to be overriden its in the Interface.

[chrswgnr]

Same issue here. I can only agree @DavidStrickland0 this smells. In my tests I would like to verifiy that in certain situations GoBackAsync / NavigateAsync is called on that interface, and in other situation it isn't.
This currently works only if the system under tests uses methods without signatures from the extension.

Using a concrete implementation that inherits from Prisms base class is no option, then the it would run that code as well and that "mock" would be a part of the system under test. And as for david, this is a no go for me.

The problem is that INavigationServiceExtensions make assumptions about the implementation. So it is expected that classes that implement INavigationService also implement INavigateInternal. Which is true for the prism code but not for mocks.

If INavigationService would extend INavigateInternal the assumption that every INavigationService also implements INavigateInternal would be correct. But that would lead to a interface change and that was probably the reason why the extension was added.

Mocks could also implement both interfaces, but due to being internal this is not possible. Still this would still be a code smell, but it would already make it testable.

Would definitive suggest to change the INavigationService and their implementations here and the removal of the INavigationServiceExtensions. With that clean mocks of INavigationService could be created.

[brianlagunas]

We cannot remove the INavigationServiceExtensions. This is what gives us the ability to share our INavService across platforms yet still give us the ability to perform platform specific navigation. The one thing I would be willing to do it rename INavigateInternal and make it public.

[chrswgnr]

Thanks for the quick response. I checked the code again, realized your point here.

As mention, renaming an public access modifier would already help here.

But noticed another possible solution: INavigationService is used in Windows10 and Xamarin, and both have custom INavigationServiceExtensions. For Windows10 there is an specific IPlatformNavigationService that inherits from INavigationService, this one is missing on Xamarin. Having a Xamarin interface version for this as well could also help. Missed that because I'm only on Xamarin.

Edit: anyway this would need another Viewmodel setup that only uses the IPlatformNavigstionService, so I would suggest both changes

[brianlagunas]

We are in the process of aligning this implementation, which is why UWP has not been released yet.

[chrswgnr]

Great, will check it again when there is a preview nuget version of those changes. For now I changed the calls in my viewmodels to only use the signature provided by INavigationService, since those are testable.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.