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

Deeper integration with mocking frameworks #310

Closed
egil opened this issue Jan 30, 2021 · 14 comments · Fixed by #315
Closed

Deeper integration with mocking frameworks #310

egil opened this issue Jan 30, 2021 · 14 comments · Fixed by #315
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@egil
Copy link
Member

egil commented Jan 30, 2021

To enable automatic mocking of services required by components, e.g. when using AutoFixture in combination with a mocking framework, there should be a way to either replace the TestServicesProvider or to have it redirect to a user provided IServiceProvider when it cannot resolve a service itself.

This is inspired by the post on using AutoFixture and Moq to simplify the "arrange" step of a test, described here: #307.

@egil egil added enhancement New feature or request help wanted Extra attention is needed input needed When an issue requires input or suggestions needs design More design is needed before this issue should result in a feature being implemented. labels Jan 30, 2021
@thopdev
Copy link
Contributor

thopdev commented Jan 31, 2021

Hi @egil, i was looking into it. And i would like to discuss my suggestion before cleaning up etc..
My suggestion/plan:
The TestServiceProvider is available though the TestContext.
For getting services the TestServiceProvider has a function:

public object GetService(Type serviceType)
{
	if (_serviceProvider is null)
		_serviceProvider = _serviceCollection.BuildServiceProvider();

	return _serviceProvider.GetService(serviceType);
}

My plan is to alter this function with a Func like this:

public Func<Type, object> CustomerServiceCreationFunc { get; set; }

/// <inheritdoc/>
public object GetService(Type serviceType)
{

var result = CustomerServiceCreationFunc.Invoke(serviceType);

if (result != null)
return result;

if (_serviceProvider is null)
_serviceProvider = _serviceCollection.BuildServiceProvider();

return _serviceProvider.GetService(serviceType);
}

Was wondering what your opinion on it is?

@egil
Copy link
Member Author

egil commented Jan 31, 2021

Thanks @thopdev. I'm not at home right now, but will respond in detail later.

@egil
Copy link
Member Author

egil commented Jan 31, 2021

We are not too far apart in our thinking of this @thopdev, however, I have a slight change to the order of how things happens.

To TestServiceProvider, I want to add the following (naming tentative):

AddFallbackServiceProvider(IServiceProvider fallbackServiceProvider);

Then, the GetServices should look like this:

public object GetService(Type serviceType)
{
	if (_serviceProvider is null)
		_serviceProvider = _serviceCollection.BuildServiceProvider();

	var result = _serviceProvider.GetService(serviceType);

	if (result is null && fallbackServiceProvider is not null)
		result = fallbackServiceProvider.BuildServiceProvider();

	return result;
}

So why not call the fallback first? The idea is that 3rd party component library venders can have an e.g. ctx.SetupTelerik() method, which might add services, components to the render tree (through the RenderTree property), or configure JSInterop.

If the fallback is called first, then any services already registered that might not need to be mocked, will be, if the fallback is something like Moq or NSubstitute.

What do you think?

@thopdev
Copy link
Contributor

thopdev commented Feb 1, 2021

Now i think about about the order, your right. The user should always be allowed to override the fallback.
And instead of the Func using a other IServiceProvider sounds good.

When i get time i'll start working on a PR. Not sure if i can do it today as i'm streaming tonight. But maybe i get some time before otherwise tomorrow :)

@egil egil removed input needed When an issue requires input or suggestions needs design More design is needed before this issue should result in a feature being implemented. labels Feb 1, 2021
@egil
Copy link
Member Author

egil commented Feb 1, 2021

Sounds good (where do you stream?).

This is probably a PR that will require more work with writing documentation than actual code, e.g. an extra section at the bottom of this page: https://bunit.egilhansen.com/docs/providing-input/inject-services-into-components.html

A paragraph or two that explains how the fallback logic works, and an example that shows how to utilize it with a mocking framework would be great. In other words, as short as possible, but long enough that folks get the point of the feature.

@thopdev
Copy link
Contributor

thopdev commented Feb 1, 2021

Good you say that, i'll also add some documentation.
I stream at twitch (twitch.tv/thopdev) from 7pm CET. I'm creating a Blazor todo app with azure func etc.

@thopdev
Copy link
Contributor

thopdev commented Feb 2, 2021

		/// <inheritdoc/>
		public object GetService(Type serviceType)
		{
			if (_serviceProvider is null)
				_serviceProvider = _serviceCollection.BuildServiceProvider();

			var result = _serviceProvider.GetService(serviceType);
			
			if (result is null && _fallbackServiceProvider is not null)
				result = _fallbackServiceProvider.GetService(serviceType);

#pragma warning disable CS8603 // Possible null reference return.
			return result;
#pragma warning restore CS8603 // Possible null reference return.
		} 
		

@thopdev
Copy link
Contributor

thopdev commented Feb 2, 2021

result can still be null, i can't make the return type object? so i was wondering if you would know a better solution? @egil

@egil
Copy link
Member Author

egil commented Feb 2, 2021

result can still be null, i can't make the return type object? so i was wondering if you would know a better solution? @egil

In NET5 the IServiceProvider actually defines the return obejct as nullable, so we can use #if NET5_0 to create a variant that does that, and one that doesn't, where we do return result!;

@thopdev
Copy link
Contributor

thopdev commented Feb 2, 2021

Thansk for the response, oke thanks, never worked with multiple versions before. But ill do that when i get to my pc.
For the tests, its that a logic to the numbering or should i just just the latest's + 1?

@thopdev
Copy link
Contributor

thopdev commented Feb 4, 2021

@egil I'm currently working on the documentation part. I wanted to add some code samples but I'm not completely sure where to place them, as its not really a component but they are also not testing framework specific. What is your preference?
Contains of example fallbackserviceprovider and test

@egil
Copy link
Member Author

egil commented Feb 4, 2021

@thopdev sounds great. Just a quick response now, more details later if needed:

  • ideally doc samples are runnable from the samples folder, e.g. as xunit tests. It is pretty complex to get docs working locally, so feel free to just include samples inline in the markdown file.

  • no hard or fast rule about the amount of unit tests needed for the new code. All code branches should be covered, e.g.:

    • when no fall back is added, the base service provider is used
    • when a fallback provider is added, the result from service provider is used when it is not null
    • when a fallback provider is added, it is called when the service provider returns null
    • if multipel fallbacks are added, only the latest is used
    • adding null as fallback throws argument null exception

These are just the ones I could come up with without looking at the code.

@thopdev
Copy link
Contributor

thopdev commented Feb 4, 2021

#315
Il also move the tests to xunit and make them work. And add the bad argument, the rest should be done. @egil

@egil egil linked a pull request Feb 5, 2021 that will close this issue
9 tasks
@egil egil closed this as completed Feb 9, 2021
@egil
Copy link
Member Author

egil commented Feb 9, 2021

Implemented in #315.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants