appdomain per test (isolation per test), for dealing with static classes

[dbones]

<< Feature Request >>
Run each test in its own app domain.

My subject under test (the actual service) has a dependency on a library which uses static classes.

between each I create a new instance of the Subject, but as it has this dependency it is unable to be an isolated test.

in NUnit 2.6.x I created a new AppDomain in the test setup, and run the subject inside that. This seemed to work (tiny bit flaky), I moved to the latest verion of nunit 3.7.x and my small solution would pass one test and then hung.

I am going to give this ago https://bitbucket.org/zastrowm/nunit.applicationdomain/overview

however I would be epic to have this support out of the box.

[CharliePoole]

I think this would be a cool feature... we could have an attribute that ran either an entire fixture or an individual test case in a separate appdomain, depending where it ws placed. Could be the start of a whole new set of isolation options.

It's a fairly big item but if the referenced app could be a source of ideas that would help. I don't believe we can use it as a source of code, since it's Apache 2 licensed.

[jnm2]

Since AppDomains are not a thing in .NET Core, and there are actually decent reasons why, this would have to be disabled in .NET Core/.NET Standard builds of the framework or else we'd have to provide out-of-proc execution per test. Out-of-proc is where you're supposed to go now if you need that kind of isolation.

[jnm2]

https://tldrlegal.com/license/apache-license-2.0-(apache-2.0)
https://tldrlegal.com/license/mit-license

I don't know anything about them, but only additional restriction of Apache 2 vs MIT seems to be that you have to state changes and if you have a NOTICES file, you have to include their notice.

[CharliePoole]

There are two ways to go.

1. Assume it's OK if you can't see any reason why not.
2. Assume it's not OK unless you get a qualified legal opinion.

As you can probably guess, I'm in camp 2. I've done extensive research trying to find an IP lawyer who published something about this and I failed. I think I even asked Rosen once, without getting an answer.

If the "just take ideas" approach doesn't make you happy, then you can get the author to contribuite it. However, in this case, it's not rocket science so it's probably easiest to just re-invent. We certainly know how to spawn domains and processes in NUnit.

[CharliePoole]

@jnm2 Based on this: http://www.apache.org/foundation/license-faq.html#Distribute-changes I'd say we can use a component licensed with the Apache 2 license provided we jump through a few hoops. Slightly different hoops depending on whether we change the code or use it as is. Not something to worry about till it comes up, I would say.

[jnm2]

@CharliePoole The "just take ideas" approach actually makes me happier either way. I was just interested because I thought of Apache and MIT as almost exactly the same, and we're MIT, and I thought part of the point was so that people could just take code if they wanted.

[CharliePoole]

Every license is different. MIT lets you do just about anything. Apache lets you do a lot of stuff but requires a few things of you, like including the license and any notices. Corporations have somebody review this stuff and make sure they are in compliance. We have always had to make do with research and free legal help where available.

Over the years, I've had to take action myself against a violator of our license a few times. That was our earlier license, which required notices to be kept if they used the code. With MIT, there are only a few things they could do that violate it and none of them are profitable. 😄

[jnm2]

So we need to decide if we want to implement AppDomain isolation and keep it .NET Framework-only (Does Mono have AppDomain support?), or implement process isolation and have it work the same everywhere. We could end up doing both too, though it seems simpler to do just one than both.

Either way, I'm expecting AppDomain-reuse or process-reuse to be important for performance if you have a lot of individual test cases which all use static state.

(Or if all that sounds like it's too complex, we could go with the conventional wisdom that static state is anathema for testing and that dependencies should be injected.)

[CharliePoole]

@jnm2 This issue asks for AppDomain isolation.

Following our conventions, you actually already said we would do it, by adding the labels you added, but then your comment says we have to decide if we will do it. If I were the OP I would be confused. 😄 But then I already said elsewhere that our labels are pretty confusing!

Inability to implement something on every platform has never stopped us from doing something before. We just don't allow it on the platforms where it doesn't work. In fact, there's lots of stuff with AppDomains that we don't do under .NET Standard already. This is just a bit more user-facing than some other things.

My feeling is that implementing AppDomain isolation will eventually lead to Process isolation, but we don't have to figure it all out now. That's YAGNI. Same with reuse. Like you, I'm sure we will get want it at some point but it feels like an optimization that we shouldn't worry about in the first implementation of a new feature.

[jnm2]

@CharliePoole Which label communicates that we have decided to do this, the pri: label?

[CharliePoole]

Yes, together with the is:enhancement label rather than is:idea. See the wiki page about labels.

As already stated, I have come to the conclusion that the system of labels I set up is OK for us to use once we get used to it but doesn't say much to users.

If I had triaged this, I would have marked it as you did minus design and added it to BackLog. Everything needs design, but the label means "don't code till you show us a design." If this is just about what it says - running tests in a separate AppDomain - it's pretty straightforward design-wise even though there are lots of implementation issues to work out. Doing that would mean that somebody is free to do a PR.

[jnm2]

I did read the is: docs and saw An addition or improvement to an existing feature. but nothing suggesting that the team has decided to do the thing being discussed.

design:

Some design decisions need to be made before this can really be worked on. Sometimes this label may be applied before anything happens and other times the work may have started but reached a point where design decisions need to be made involving others in the team.

Seems to fit a scenario where I'm asking the team to decide whether it's worth adding an AppDomain feature versus an out-of-proc feature before anyone writes code =) I am leaning toward the latter. Once you and perhaps another team member respond that it's worth doing the AppDomain feature, then I would expect the label to be removed and an assignment or documentation for up-for-grabs to happen.

I also wouldn't expect it to go in the backlog if we're still discussing whether we want the feature as described vs alternatives.

[jnm2]

@dbones Please feel free to make a case for multiple AppDomains for test cases vs multiple processes for test cases, if you have an opinion either way.

[CharliePoole]

@jnm2 Let's stop discussing meta-stuff here then and keep it to the actual request...

You've asked for discussion, I marked it for discussion. My view: this sounds like a good feature we should add to the Backlog. If you are programming and testing an app that runs on a platform with AppDomains, you should be able to create an appdomain for your tests if you need to. I would see this as used either on a fixture or a test.

There's a chance we may need to run higher-level setup and teardown, including instantiating the fixture, in the appdomain as well. When used on a method, that may mean that OneTimeSetUp is run twice. IOW, this actually requires changing the lifecycle of the fixture.

If we later decide to implement running in a process, we will need everything we do for app domains plus some other stuff, so doing appdomains seems like a good start.

This should be done by somebody with a pretty deep understanding of the innards of NUnit. 😄

[dbones]

@jnm2, I do not mind, either should work. (@both, please do not implement anything you think will effect your upgrade path)

my main goal is to run code in complete isolation, and invoke it via its API (consider a contract/api test on a small service, where the service runs its entire stack, no mocking, in the appdomain/process), when the test is done then destroy the service instance, and that should destroy any dependencies (which have static classes).

[rprouse]

Personally, I am a bit uncomfortable with the idea of creating an AppDomain for every test or even running every test in it's own process. Adding that amount of complexity to the framework to support a very limited use case is over the line IMHO. I might be okay with the idea if it could be limited to one attribute applied to test fixtures or tests, but if the changes started leaking out into the rest of the framework, I would be against the idea.

If someone wanted to map out how we could do this without adding too much complexity, then I could be convinced otherwise.

[CharliePoole]

I assumed this was only about an attribute you apply when you need it.

[rprouse]

I assumed this was only about an attribute you apply when you need it.

If that is the case, then I would be okay with the enhancement. I immediately thought about our various command line options which might have colored my attitude.

[jnm2]

The attribute would have to be able to intercept the method calls to the test and setup and teardown, stand up a new process or AppDomain, and be able to examine the intercepted commands, serialize them, and understand them well enough to manually perform them on the other side. From the AppDomain work I've done, this will be fairly far-reaching into the test command system.

There's no guarantee that parameters will even be serializable.

[CharliePoole]

There's no doubt that it's a hard implementation. Parameterized tests do present an added issue, which might suggest that the implementor take an incremental approach:

1. Simple Tests
2. Parameterized tests with TestCase
3. TestCaseSource using fields
4. TestCaseSource using properties and methods (this is where the serialization issue arises)

I'd be willing to take separate PRs for each stage, but we'd have to decide if we would release with it only working in certain cases.

Ironically, this would be easier for our hypothetical "dynamic" test cases, which we have talked about for a long time... everything would be done at run time in the isolated domain.

[jnm2]

I would rather find a workaround code sample allowing people to manually call IsolationUtils.RunNewProcess(() => { }) or IsolationUtils.RunNewAppDomain(() => { }) (I have such code sitting around) than to prioritize this feature over other hi-pri bug fixes. That puts the burden of setup, teardown, parameterization etc on the end user for now. Only one person has asked for it.

That is, unless it's your thing and you have a particularly burning desire to implement it. 👍

[CharliePoole]

@jnm2 Well, you prioritized it yourself as normal, so it isn't prioritized over high priority bug fixes.

As for IsolationUtils, that's another option for sure. I'd see that as another idea issue myself.

In my view, the purpose of triage is to make it possible for somebody to work on something and know we will accept it if the code is clean and passes the tests. We never actually assign work to anybody, so we are not deciding here what gets done first . Individually, one of us can decide to do it, of course.

We can also decide that we would never accept this change, no matter who did it, no matter how well written. That's actually a courtesy to users so they don't waste time. So if that is what you are saying, then you are on point here, even though I disagree with you.

I do know that this has come up repeatedly over the years and we have always said "not yet."

[jnm2]

All I'm saying is I don't think the framework team should feel obligated to prioritize this because we have the code sample option so no one is being blocked, and the demand is low. I'd be happy to accept a change to add process-per-test-case if someone from the community were to work on it. (Because of the complexity level, I'm not sure it's worth documenting to the point where we could apply up-for-grabs.)

I'm not sure if we should accept a contribution towards AppDomain-per-test-case because the demand for AppDomain versus out-of-proc is zero that I have seen, and eventually people will want out-of-proc if they want either now.

[CharliePoole]

I'm sorry to nag, but I think it's important to have a common ground about what it means to triage a request. The "up-for-grabs" label doesn't matter. Either we think something is an acceptable feature or we don't. There is no "It's really not acceptable but if you do the work we will consider it."

The binary decision of acceptablility is carried out by assigning it as a feature (or enhancement) or closing it as wontfix. Period. Either you hate this idea enough reject it totally or you don't.

If we accept it, then it can get prioritized, have extra labels added, etc.

There is a line between making suggestions about alternative solutions to a problem and telling users what they should want. Lots of our users are at companies that are way behind the curve in technology adoption. Folks will want to use AppDomains for many years, whether we think they should or not.

[jnm2]

👍 Restatingly more clearly:
From what I can see now, I think that process-per-test-case is an acceptable pri:normal feature.
I'm doubtful that AppDomain-per-test-case is a good direction, so I'm included to say that's not an acceptable feature unless there's a reason we can't do the wider and arguably cleaner solution of process-per-test-case.

It's time I step back and let others have their say. 😳

[rprouse]

I think we need to be careful in what we accept as features in the framework. Most users, like @dbones have very valid use cases and this is an interesting idea, but I think this is the first time I've ever seen this request in several years and I doubt it would be widely used.

IMHO, we already have way too many command line options, the majority of which are never used. Similarly, we have dozens of features that sounded cool at the time but are never used. I would like to stop that feature creep and evaluate each enhancement for its value and use. All these extra options make it harder for users to find what they really need.

I wish we could add something like App Analytics to NUnit to track exactly which features are being used and on what frameworks 😄

[CharliePoole]

If you want to avoid creep, this is a good one to start on. 😄

I will say I have seen a lot of users creating AppDomains and sometimes asking if there is a way that NUnit can do it for them.

[psandhu79]

@jnm2 Regarding the code sample you mentioned using IsolationUtils would it be possible to get this.

[jnm2]

@psandhu79 For a separate appdomain, this is the code:

public static class IsolationUtils
{
    public static void RunInNewAppDomain(Action action)
    {
        if (action == null) throw new ArgumentNullException(nameof(action));

        var friendlyName = $"{typeof(IsolationUtils).Name}.{nameof(RunInNewAppDomain)}({action.Method})";
        var domain = AppDomain.CreateDomain(friendlyName, null, AppDomain.CurrentDomain.SetupInformation);
        try
        {
            domain.DoCallBack(action.Invoke);
        }
        finally
        {
            AppDomain.Unload(domain);
        }
    }

    public static T RunInNewAppDomain<T>(Func<T> func)
    {
        if (func == null) throw new ArgumentNullException(nameof(func));

        var friendlyName = $"{typeof(IsolationUtils).Name}.{nameof(RunInNewAppDomain)}({func.Method})";
        var domain = AppDomain.CreateDomain(friendlyName, null, AppDomain.CurrentDomain.SetupInformation);
        try
        {
            var invoker = (CrossDomainInvoker)domain.CreateInstanceAndUnwrap(typeof(CrossDomainInvoker).Assembly.FullName, typeof(CrossDomainInvoker).FullName);
            return invoker.Invoke(func);
        }
        finally
        {
            AppDomain.Unload(domain);
        }
    }

    private sealed class CrossDomainInvoker : MarshalByRefObject
    {
        public T Invoke<T>(Func<T> func) => func.Invoke();
    }
}

One thing to watch out for is to not pass in lambdas or local functions which capture local variables or parameters, because the compiler will create an autogenerated closure class for that which will not be serializable. It's straightforward enough to manually write a serializable closure class. (Note, not MarshalByRefObject but [Serializable]. Otherwise you'll be back to executing in the original assembly.)

It's not possible to create a NewAppDomainAttribute to apply to the test and cause set up and teardown to also run in that other AppDomain. From the test I ran, the only thing holding that back is that every type deriving from TestCommand, including any involved third-party types, would have to be serializable.

Executing in a separate process is a little more involved (emitting an assembly) but I can share that too if you are interested.

[jnm2]

Hey, check out this cool library by @zastrowm! https://bitbucket.org/zastrowm/nunit.applicationdomain

[zastrowm]

The NUnit.ApplicationDomain library works by using TestActionAttribute to execute the test method before the "normal" test in the default appdomain runs. This execution is performed using these steps:

1. Gather up the arguments to the current NUnit method (it may use reflection for this)
2. Create an app domain
3. (in app domain) Construct a new instance of the test class
4. (in app domain) Invoke the SetUp methods (discovered via reflection)
5. (in app domain) Invoke the test with copies of the previously saved arguments
6. (in app domain) Invoke the Teardown methods (discovered via reflection)
7. (in app domain) Capture the test result or the exception that occurred
8. Dispose of the Domain
9. Throw an exception indicating success or failure (thus bypassing the normal test execution)

It is sort of hacky, as it doesn't actually use any of the NUnit infrastructure for executing the test - I'm not sure that when I created the library such infrastructure existed - but it does allow running UTs in an app-domain.

I'm not sure if this actually helps anyone, but I figured I'd come explain it, since I was name dropped :: )

[jnm2]

This is the kind of thing where I don't really have the appetite to implement appdomain-per-test-case, but I'd go to lengths to make sure we provide the right extensibility points so that libraries like NUnit.ApplicationDomain aren't inherently workarounds.

[AlexaCodex]

Do we have any plan to work on this feature?

[PuerNoctis]

Just to throw a real-life scenario into the mix: We have a customer that has a big big code base that heavily depends on static classes with static caches and states. Their unit test execution is abysmal, because they have to manually clear all the relevant data after each test in every test assembly, and sometimes this is forgotten and they have to again look at which data has to be actually cleared and change the tests quite often. Also, tests cannot be executed in parallel, and serial testing takes hours to complete.

The idea now was: Refactor the code to not use statics anymore. This would be a massive undertaking because, as I stated, the code base is very big and the impact is very high (full retesting, rewriting of critical code-paths to adapt to the fact, that the statics "are not there anymore"). The refactor is in itself a good idea, but it would have to be in smaller steps and thoroughly planned.

The tests, however, would still take hours to complete until the whole restructure is ready.

A short-term solution to the problem would be to have per-TEST AppDomains. Just to clarify: The code will probably stay on full .NET Framework and the hardware will be "buffed" to mitigate the increased CPU/Memory demand, so we would be okay with that.

Even if this is not implemented officially, I really tend to implement it the hacky way @zastrowm suggested for our project (or use NUnit.ApplicationDomain directly).

[CharliePoole]

It has to be really confusing for users that this issue...

1. Has been around for so long
2. Is labelled as an accepted feature (i.e. not an idea)
3. Is labelled as requiring design work
4. Has comments by various team members who
   4.1 Favor the feature
   4.2 Are opposed to the feature
   4.3 Want to make it possible to implement the feature by extensibility hooks
5. Has never been assigned to anyone

I'd not that the fact that some platforms don't have AppDomains doesn't mean we can't have the feature. It would obviously need to be limited to platforms that have them, just as we have done with the console option that allows deciding how many AppDomains to use.

[Axemasta]

@jnm2 just popping by to say thanks for the util class, this has helped me fix an issue with AppContext causing subsequent test runs to fail based on the first time the value was set. Thanks for providing the code and a good explanation of how to use it! 🎉

[rprouse]

I'm switching this back to idea as it isn't accepted. Personally, I'm not in favor of adding new AppDomain functionality since it only applies to .NET 4.5 now. If we implement process isolation as in #3378, then we will have an alternative that works on all platforms.

Because of that, I am leaning towards closing. Any objections @nunit/framework-team?

[mikkelbu]

I'm also leaning towards closing