Adjustments and improvements to Umbraco.Cms.Tests.Integration

[lars-erik]

I recently released a package and a blog post about improving setup time for integration tests that make use of the Umbraco.Cms.Tests.Integration package.

• https://github.com/lars-erik/NUnitCompositionWithUmbraco
• https://blog.aabech.no/archive/more-efficient-integration-tests-with-umbraco

I'd like to hear HQs opinion on making a few tiny changes to the very base classes in Umbraco.Cms.Tests.Integration, as well as whether it would be interesting to see about applying a few of the mechanisms I've introduced to the core integration tests.
Applied well, the tricks could drastically improve execution time of the integration tests during CI builds, not to mention locally.

The two main things that prevented me from achieving the setup fixture technique instead of test fixtures are as follows:

1. The TestOptionAttributeBase expects an NUnit test method to be running and use the method name to find the declaring type and the queried attribute.

• This prevents discovery when used on setup fixtures rather than test fixtures since there is no active test method yet.
• The TestExecutionContext has the current fixture object that has the queried attribute
• TestOptionAttributeBase should be rewired to fall back to looking at the type's attributes if a test method is not active. Currently it just blows up without being hacked.
• In my package I've used Lib.Harmony to rewrite the IL of TestOptionAttributeBase, which is totally naughty and a hack. 😇

2. The DefaultUmbracoAssemblyProvider et. al. requires a physical location for the entrypoint assembly.

• This blows up when the fixture is a dynamic proxy. I don't expect that to be a major concern for HQ, but it doesn't need to be hard to enable either.
• It could for instance check whether there is a location for the assembly and otherwise check the direct base type and fall and so on.
• There could for instance be a settings option to replace the entrypoint assembly fullname
• [Insert a bunch of other good suggestions about type discovery...]

I think the first is super easy and a no-brainer to fix, while the second requires a bit more thought and discussion.

Then for the general improvements the techniques would bring and how the core tests could benefit:

• I haven't tried to time or explore setup time and types of seeded data in the core tests
• I am however sure that since the integration tests currently run 6 times, each between 5 and 8 minutes at the least, some CPU, power and tress might be saved by cutting down on setup time. 🙃
• The effort needed to reap the most benefit of moving to setup fixtures is too big to do in one PR while adhering to the contribution guidelines. 🙈

Eager to hear anyone's opinion on this.
Hit me up!

[lars-erik]

Oh, and while at it - why not bump NUnit from 3.14 to 4.x by now?

[Migaroez]

Hey Lars, I have been willingly assigned to have a look at this and am very much interested in trying to get this done.

Send me a DM if you want to have chat about this sometime soon.
In the meantime i will check out how much effort/gains are involved to apply something like this to all the core integration tests

[lars-erik]

Awesome! And well assigned it is. 🥳

[lars-erik]

First PR in. :)
#21896

[lars-erik]

Just a wee status update:

• We've paired on a PR for points 1. and 2. in my OP. The PR is incoming from @Migaroez
• I've made Tests: Extracted fixture agnostic base classes for test base classes. #21896 to "extract superclass" for UmbracoIntegrationTest and UmbracoTestServerTestBase such that only the [SetUp] and [TearDown] methods remain in the current base classes. The new intermediate base classes between them and UmbracoIntegrationTestBase are then lifecycle agnostic and can be used as base classes for [SetUpFixture]s as well.

Hopefully these two PRs will let us experiment with improving the run time of the core integration tests, as well as facilitate any such use in consumer tests. Not quite sure whether the full "reusable database" trick from my package is applicable, but we'll see given time and effort.

[lars-erik]

Some notes after trying to upgrade to 17.

• Hard to re-use or create authenticated HttpClient when using UmbracoTestServerTestBase as setup fixture
• Make sure things like CustomMvcSetup isn't inadvertently made non-virtual. (Should likely add XML-comment and use API validation)
  (Fortunately @AndyButland was quick on the ball with Integration tests: Re-virtualized CustomMvcSetup #21947. 🙏)
• The threaded built-in database cleanup seem to leak again, leaving zombie dotnet processes around. Might be my use of the base classes, but still worth noting and investigating. (Not my reusable database implementation, the built-in testdatabase implementations.)

Just leaving the notes here so we can talk about and optionally address them.