Prism: Testability of NavigationMode needs to be improved

[clawrenceks]

Description

In a Prism application for Xamarin.Forms I was hoping to use the GetNavigationMode extension method to allow logic in OnNavigatedTo to be dependent on the NavigationMode (e.g. new or back). Naturally, I want to be able to unit test this logic, but I am finding that the NavigationMode is not all that easy to unit test.

The nature of the issue seems to come down to the fact that the GetNavigationMode extension method is reading from the internal parameters of the NavigationParameters, shown below.

public static NavigationMode GetNavigationMode(this INavigationParameters parameters)
{
var internalParams = (INavigationParametersInternal)parameters;
if (internalParams.ContainsKey(KnownInternalParameters.NavigationMode))
return internalParams.GetValue<NavigationMode>(KnownInternalParameters.NavigationMode);
throw new System.ArgumentNullException("NavigationMode is not available");
}

Based on the above, the challenge seems to be how to get the known internal parameters into the navigation parameters. Having explored the NavigationParameters I can see that there is actually a public method that should allow this to be achieved, the signature for which is shown below. The issue I have with using this method is two fold:

1) It is hidden from IntelliSense, which doesn't make me comfortable in using it.

2) Given the fact above, and the naming of the method, obviously my unit test would be manipulating internals, again something I am not keen on doing.

[EditorBrowsable(EditorBrowsableState.Never)]
public void AddInternalParameter(string key, object value);

Based on the above, I feel it would be beneficial for classes that wish to use the the NavigationMode to be more easily unit tested.

Steps to Reproduce

See above

Expected Behavior

That unit testing of classes that wish to based logic on the NavigationMode is easier to perform, without using public methods that are hidden from IntelliSense.

Actual Behavior

See the Description.

Basic Information

• Version with issue: Prism.Unity.Forms - 7.0.0.396
• Last known good version: N/A
• Xamarin.Forms version: 3.1.0.583944
• IDE: Visual Studio 15.7.4

Reproduction Link

See all of the information above.

[dansiegel]

Completely understandable that you want to be able to unit test your ViewModels with a given NavigationMode. There are times in when designing an API that certain internal methods have legitimate edge cases for being made public. The issue is that it is an edge case and not wise to make fully public. In those times you will find these sorts of methods hidden from Intellisense as you noted above.

While I realize this might sound weird, Unit Testing often falls into the edge case category. I'm not just saying that because so few people actually test their code. There are a number of times such as the API you referenced in this issue, where the API is designed for Consumption only. Because it is for consumption only, it would be a terrible idea to provide a way to manipulate the given API.

A perfect example of this issue is with an event handler. You can only invoke an event from the class that it's declared in. So what do you do in the case of say Xamarin.Forms where you have a ButtonClicked event in the Button class in the Xamarin.Forms.Core assembly but need to invoke that event from a Renderer in another class in a whole other assembly, but at the same time, it's not mean for you to go around calling some method in your code?

The answer is to have a method and set the EditorBrowsableState to Never. This allows edge cases to still perform the function that they need to. This means you still have a fully functional API for Unit Tests, it just requires a little more work to know that the API does actually exist and when to use it.

Your issue is well taken though, and I would say that rather than changing the API, we do need to ensure that things like this are documented. There has certainly been a call for quite a while about testing.

[clawrenceks]

@dansiegel Thanks for the very detailed reply, the nature of the edge case here is clear now.

One of the reasons for me raising the issue was not just because I thought it could be improved (and documentation seems the right way to do so in this case), by raising the issue I also wanted to make sure I hadn't missed some other obvious way of testing the behaviour I was interested in, and that the way I had identified is currently the appropriate way. Your response seems to confirm this is the case, along with explaining why, so thanks again for that.

I do have another navigation related use case that I want to unit test - that a modal navigation was performed. I haven't found a way to achieve this yet and will be doing some more digging. If I do not find a way to achieve this, what is the best way to raise a query of this nature? Again as an issue on GitHub? or do you have an alternative location for handling queries? This won't just be a general question, it will be the result of perhaps having exhausted all avenues that I can think of to resolve a problem.

[mackayn]

I did run into this today trying to fix unit tests, while it would be handy to override the navigation direction I completely understand why it's not allowed and to me suggests you shouldn't be unit testing navigation events anyway..

[clawrenceks]

@mackayn I think perhaps you are referring to a different issue to the one discussed on this thread. The particular scenario that I was referring to in this thread was about the ability to successfully unit test the navigation mode, so that in the event that I wish to use the NavigationMode to drive navigation logic I can successfully unit test this logic - something that in my opinion should definitely be done.

Looking to override the Navigation Mode that Prism sets when it performs navigation however definitely sounds like something that shouldn't be done.

[mackayn]

Actually, was referring to the same thing, crossed wires

[clawrenceks]

Just following up on one of my previous comments regarding to the ability to test Modal Navigations. I was able to do this just fine.

The approach I ended up taking was to create a Navigation Service Fake that is derived from the Prism PageNavigationService. Within This Fake I add public properties that are of interest to my unit test scenarios and I override NavigateInternal - it is within here that I set the properties I have created, based on the information that is passed to the Fake when navigating. For example, within here I can easily see if the useModalNavigation bool property was passed in as true.

[dansiegel]

Closing this as by design. We will make sure to update docs to better assist developers in understanding how to work with the API for Testing.

[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.