Parallel execution of Tests in Umbraco v8

[Adolfi]

Using the unit testing approach in the documentation: https://our.umbraco.com/documentation/Implementation/Unit-Testing/, parallel test execution is not possible, which expessially is a problem since it is enabled by default in xUnit.

See discussion (me discussing with myself :) ) on our:
https://our.umbraco.com/forum/using-umbraco-and-getting-started/99361-parallel-execution-of-tests-in-umbraco-v8

I have prepeard a pull request, making the Umbraco.Core.Composing.Current.HasFactory property public instead of internal, so that it could be used in testing purposes. I dont really see an issue with exposing this property, but I would like to run it by a Core member to verify that this is an excepted solution. If so, i will submit the pull request, and also update the Unit testing Documentation and add a few words on parallel execution and how it affects your tests (using the new exposed HasFactory property).

Cheers!

[Shazwazza]

Hi @Adolfi,

Can I ask what you are testing?

I think we should update these docs on how to unit test with Umbraco. In hopefully most cases people shouldn't need to initialize any singletons like it is currently telling you to and instead classes should be using constructor injection with their dependencies. For example, if you are testing a controller, the controller should have a declared overridden constructor declaring all dependencies, then you unit test can just inject these services, whether mocked, or otherwise. Then there's generally no need for initializing singletons which is also why parallel execution is not working.

The only time singletons might need to be initialized is if testing needs to take place over top of some of the internal umbraco surface area that relies on them. These days there isn't much of that.

I'm unsure how exposing HasFactory will assist with parallel execution since it will still be the same singleton object shared between all tests and if you are resetting this or something you are just going to run into concurrency issues.

[Adolfi]

Hi @Shazwazza.
Thank you for the in-depth answer. This makes a lot of sense, and I agree that there would be necessary to update the documentation with more explicit examples. I'm mostly testing controllers and content-models when i get the "Factory has not been set" errors, and there where not much information on the subject available. By exposing the HasFactory property i was able to bypass the Current.Factory get/set throwing the errors, but i guess that I eventually would run into concurrency issues anyway?

I would love to see some examples of the overridden constructors you mentioned where I would get around the parallel problem.

Cheers.

[rasmusjp]

I ran into this problem with api controllers a while ago, but forgot about it again.

I think the problem is in this attribute

Umbraco-CMS/src/Umbraco.Web/WebApi/Filters/FeatureAuthorizeAttribute.cs

Line 23 in e7196d5

_features = Current.Factory?.TryGetInstance<UmbracoFeatures>();

which is added to the UmbracoApiControllerBase. I would assume that instead of doing a null check it should check Current.HasFactory and only try to call TryGetInstance if the the factory is actually set.

[Shazwazza]

There are various ways of testing controllers - one is an end to end (integration) test which you can essentially 'host' the process and route the entire fake request. This will trigger filters, authorization, etc... In this case, there will probably be some odd singletons to initialize because unfortunately attributes in .net framework cannot be injected and rely on singletons.

The other way, is just like the examples in the docs, where you are simply just testing a method/action on a controller. In these examples, no filters, etc... will be executing. Every Umbraco controller has 2x constructors: Empty (for folks that just don't want to learn dependency injection), All parameters (the normal constructor). If you look at this link and scroll down a bit until you see the "Tip", it has a GIF showing how you can auto-generate the full overridden constructor of any class: https://our.umbraco.com/documentation/Implementation/Services/#using-the-custom-siteservice-inside-a-controller

In fact, this whole article is quite helpful at explaning how constructor injection works https://our.umbraco.com/documentation/Implementation/Services/#custom-services-and-helpers-1 and this is really how you should be configuring your controllers - with full constructors and injecting any additional dependencies you need. This means for unit testing you are simply passing in your dependencies for your controller which could be mocks, etc... and you don't have to worry about initializing singletons.

Exposing the HasFactory will innevitably just cause you other concurrency issues so I don't think that will solve your parallel test execution issue - what will solve it though is using constructor injection. Keep in mind though, that if you are doing full integration testing, you will still run into issues because some of the Umbraco filters like rasmus mentioned, do use singletons.

I will close this issue for now but we can keep chatting here.

[Adolfi]

Thanks Shannon.
I appreciate that your take the time and explain and provide related links, I’m learning a lot here.
I would be happy to update the documentation, (I’ve already started), but I have one last question:

So what would be the bare minimum amount of code needed to mock all dependencies for a RenderMvcController/SurfaceController and UmbracoApiController? Could you provide an example?

Since not all of the dependencies are interfaces (UmbracoHelper & ServiceContext especially, which also in turn has explicit non-interface dependencies of their own), every time I start mocking them up, it quickly grows into more and more dependencies needed to be mocked in order to get a simple little test working, which makes me think I am doing something wrong or more complicated than it has to be. The documentation should probably be short, simple and to the point and not a mocking bible. :)

Would be very happy if you cleared this up for me. And again, thank you for your time.
Cheers! 🍻

[Shazwazza]

Hi, yes unfortunately there's not enough interfaces ;/

You're best bet is to browse the Umbraco source code, there's tons of tests in there. Lots are integration tests but lots do a lot of the mocking you describe.

• UmbracoHelper - is constructed based on several interfaces so you can mock each of those, as for the MembershipHelper - that one is annoying. You can mock most of it's ctor params but not others, though you can fake the others, else if you aren't using that in your tests/code, you can just pass in null for those params
• UmbracoContext can be created by the UmbracoContextFactory which you can mock/fake all dependencies for that
• ServiceContext.CreatePartial allows you to mock whatever services you want and create a service context

[Adolfi]

Got it working using these mocks:

[SetUp]
        public void SetUp()
        {
            var serviceContext = ServiceContext.CreatePartial(
                Mock.Of<IContentService>(),
                Mock.Of<IMediaService>(),
                Mock.Of<IContentTypeService>(),
                Mock.Of<IMediaTypeService>(),
                Mock.Of<IDataTypeService>(),
                Mock.Of<IFileService>(),
                Mock.Of<ILocalizationService>(),
                Mock.Of<IPackagingService>(),
                Mock.Of<IEntityService>(),
                Mock.Of<IRelationService>(),
                Mock.Of<IMemberGroupService>(),
                Mock.Of<IMemberTypeService>(),
                Mock.Of<IMemberService>(),
                Mock.Of<IUserService>(),
                Mock.Of<ITagService>(),
                Mock.Of<INotificationService>(),
                Mock.Of<ILocalizedTextService>(),
                Mock.Of<IAuditService>(),
                Mock.Of<IDomainService>(),
                Mock.Of<IMacroService>(),
                Mock.Of<IPublicAccessService>(),
                Mock.Of<IExternalLoginService>(),
                Mock.Of<IServerRegistrationService>(),
                Mock.Of<IRedirectUrlService>(),
                Mock.Of<IConsentService>(),
                Mock.Of<IContentTypeBaseServiceProvider>());
            
            var membershipHelper = new MembershipHelper(
                Mock.Of<HttpContextBase>(),
                Mock.Of<IPublishedMemberCache>(),
                Mock.Of<MembershipProvider>(), 
                Mock.Of<RoleProvider>(),
                Mock.Of<IMemberService>(),
                Mock.Of<IMemberTypeService>(),
                Mock.Of<IUserService>(),
                Mock.Of<IPublicAccessService>(),
                AppCaches.NoCache,
                Mock.Of<ILogger>());

            var umbracoHelper = new UmbracoHelper(
                Mock.Of<IPublishedContent>(),
                Mock.Of<ITagQuery>(),
                Mock.Of<ICultureDictionaryFactory>(),
                Mock.Of<IUmbracoComponentRenderer>(),
                Mock.Of<IPublishedContentQuery>(),
                membershipHelper);

            this.controller = new ProductsController(Mock.Of<IGlobalSettings>(), Mock.Of<IUmbracoContextAccessor>(), Mock.Of<ISqlContext>(), serviceContext, AppCaches.NoCache, Mock.Of<IProfilingLogger>(), Mock.Of<IRuntimeState>(), umbracoHelper);
        }

Could you just confirm that this is the best practice for mocking a controller, and then i can update the documentation.

[Adolfi]

This is the controller:

public class ProductsController : UmbracoApiController {
        public ProductsController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext serviceContext, AppCaches appCaches, IProfilingLogger profilingLogger, IRuntimeState runtimeState, UmbracoHelper umbracoHelper) : base(globalSettings, umbracoContextAccessor, sqlContext, serviceContext, appCaches, profilingLogger, runtimeState, umbracoHelper)
        {
            
        }

       public IEnumerable<string> GetAllProducts()
        {
            return new[] { "Table", "Chair", "Desk", "Computer", "Beer fridge" };
        }
    }

[Adolfi]

Actually, I've got all the tests working using these new mocks, so I'm starting to think that it is the right approach. Also worked great with parallel execution. I feel confident enough to update the docs and send in a PR, would you be up for reading that PR yourself, if I reference it in this thread?

Thank you SO MUCH Shannon for all your help, I learned a lot!
Cheers!

[Shazwazza]

For the service context, you don't need to declare all mocks, all of those parameters are optional. So if you are only using (i.e. IContentService) you can do:

var serviceContext = ServiceContext.CreatePartial(
                contentService: Mock.Of<IContentService>());

By naming the optional parameters you only need to mock what you need.

[Shazwazza]

But yep, you are on the right track, if we update the docs, just like they are now, it will be good to start with testing controllers and then perhaps have some headings on how to mock these main umbraco components. I totally understand it's annoying and UmbracoContext and UmbracoHelper, etc... could/should be interfaces, but that means large breaking changes so we can't do that. The other reason for them not being interfaces is because they tend to change and if that were the case with interfaces, we couldn't without breaking changes. ... but anyways, in v9 maybe we can have nicer interfaces ;)

[Adolfi]

Yep, I will add a section in the beginning of the docs with a heading about Mocking, which will be about mocking the umbraco components (ServiceContext, UmbracoContext, UmbracoHelper, MembershipHelper). This way I wont need to copy/paste the same chunks of mocking code in every single test-class, but instead I can keep referencing the inital mocks in every tests.

I will also re-write the dictionary+contentquery tests, so that they are using the umbracoHelper together with mocks of the IContentQuery and ICultureDictionaryFactory.

I totally understand, I can imagine the difficulties of maintaining a huge codebase where you "want" to refactor a bunch of thing but still need to think about not having too many breaking changes.

Thanks, I'll ping you in this thread when the PR is ready, looking forward to your feedback.

[Adolfi]

So I’m almost done with my PR, I’m on my last test to rewrite, the UmbracoApiControllerTest, but I’m stuck. Seems like the UmbracoApiController constructor fails in the UmbracoApiControllerBase if I have not mocked the UmbracoContext, unlike the other Controller tests which does not seem to care, so I can just pass in a mock of UmbracoContextAccessor. I tried mocking the UmbracoContext using the UmbracoContextFactory as you mentioned, but again there is a few non interface dependencies there (Urlproviders, MediaUrlProviders etc) which makes my mocking grow and grow and grow.

What would be the minimum amount of code needed to mock a UmbracoContext to get a UmbracoApiController test running?

[Shazwazza]

I can see that (unfortunately) the UmbracoApiControllerBase uses a service locator pattern in it's main ctor (which we should fix!). There's currently a TODO in that class: https://github.com/umbraco/Umbraco-CMS/blob/v8/dev/src/Umbraco.Web/WebApi/UmbracoApiControllerBase.cs#L62

This base class should obsolete this ctor and add a new ctor with all dependencies.

Could this be the problem you are seeing? You haven't provided a stack trace or the errors you are receiving so I'm unsure where the problem lies.

[Adolfi]

Yes! Thanks, this is exactly what I am experiencing. (Sorry for the missing stack-trace/error, I was writing from by phone, on the bus. :) )

Is there an issue on GitHub for this issue up for grabs or is it something that you'd like to do yourself? Until it is done I will go ahead and submit my PR for the ducumentation today, but leave out the UmbracoApiController Tests, since they would be considered "untestable" using the full constructor injection. I'll add them back in once the constructor fix has been done.

Cheers!

[Adolfi]

umbraco/UmbracoDocs#1987

[Shazwazza]

@Adolfi there's no issue or anything logged for fixing that but if you could, it would be superb if you knocked up a PR to fix the ctor for UmbracoApiControllerBase as mentioned above, then we can get that merged so testing can start working normally again.

[Adolfi]

Sure, I fix. :)