junit4 icon indicating copy to clipboard operation
junit4 copied to clipboard

Separate Classloaders for parallel methods

Open cowwoc opened this issue 13 years ago • 24 comments

Please add an option for executing each @Test in its own ClassLoader. This is especially important in parallel-methods mode where threads clobber each other's static variables.

See https://jira.codehaus.org/browse/SUREFIRE-749 for a more in-depth discussion.

cowwoc avatar Jun 21 '11 16:06 cowwoc

To clarify, I am asking for an extension point (allow users to pass in a ClassLoader to use) or an outright feature.

cowwoc avatar Jun 21 '11 16:06 cowwoc

I successfully managed to run tests with different ClassLoaders via org.apache.myfaces.test.runners.TestPerClassLoaderRunner

http://myfaces.apache.org/test/myfaces-test12/apidocs/org/apache/myfaces/test/runners/TestPerClassLoaderRunner.html

see also: https://issues.apache.org/jira/browse/MYFACESTEST-50 https://issues.apache.org/jira/browse/MYFACESTEST-19

raf84 avatar Jul 15 '12 10:07 raf84

It'd be neat to be able to have the ClassLoader-per-Test running capability without having to bring in myfaces-test as a dependency.

pholser avatar Nov 05 '12 16:11 pholser

I don't think myfaces-test is a dirty package. Anyway, you can cut and paste bringing the same licence, but then you will have to mantain that package.

raf84 avatar Nov 05 '12 19:11 raf84

@cowwoc Without going inside the Jira report, can you tell us briefly why should I run my concurrent test method in a unique CL ?

Tibor17 avatar Nov 05 '12 19:11 Tibor17

@raf84 I certainly didn't mean to suggest that myfaces-test is "dirty". I meant that if my project has nothing to do with JSF/MyFaces and I wanted new-ClassLoader-per-Test testing, it'd look kind of weird to bring in myfaces-test as a dependency.

pholser avatar Nov 05 '12 19:11 pholser

Tibor,

Most code makes use of static variables somewhere. So long as you run inside the same ClassLoader you run the risk of one test influencing another. I have run into specific warnings and errors in frameworks such as Guice, slf4j and others that have to do with concurrent access of static variables. Yes, these are bugs in the respective frameworks, but no they're not going to get anytime soon. We're better off testing in true isolation anyway.

cowwoc avatar Nov 05 '12 20:11 cowwoc

@cowwoc

Did you write a snapshot code without any framework in order to prove the concept?

Would i be like this ?

  • instanciate test class T in main Thread and loaded by the system class loader;
  • T has one instance method t;
  • T imports class C having a static context in it;
  • use C in t as a test;
  • start other thread T2 with context class loader L loading C again

or it would be different, so that

  • C is not used in T, not imported in T;
  • C is only loaded by context class loader L through T2; , then C can be used in T and t only via Java Reflection.

What's the use case like?

Tibor17 avatar Nov 05 '12 21:11 Tibor17

It would work like this... Given:

Thread 1: T1 Thread 2: T2

T1 creates a new ClassLoader C1 T2 creates a new ClassLoader C2 T1 loads class A using C1 and gets back class A1 T2 loads class A using C2 and gets back class A2 Now whenever T1 accesses A it sees A1 and T2 sees A2

The trick is to avoid referencing any of the shared classes from the system classloader. Even if you do, there are ways to hide them so child class loaders don't see them.

cowwoc avatar Nov 05 '12 21:11 cowwoc

@pholser I agree, but I am a promoter of reuse. May be it would be nice to ask Apache to donate (if someone is able to mantain)?

@Tibor17 if it can be helpful, this was my use case:

  • a test class T has two instance methods t1() e t2()
  • t1 import class C instrumenting it throught javassist
  • t2 import class C instrumenting it in another way throught javassist

if I run the test class throught a single JUnit thread, t1 goes ok, but t2 fails because it try to import an already loaded class.

raf84 avatar Nov 05 '12 21:11 raf84

@raf84 i think we cannot use byte code weaving

Tibor17 avatar Nov 05 '12 21:11 Tibor17

Bytecode intrumentation is evil and results in many annoying problems (especially when it comes to debugging and dealing with PermGen leaks). Using separate Classloaders solves all that.

cowwoc avatar Nov 05 '12 21:11 cowwoc

@cowwoc suppose A has t1 and t2 methods.

Do you want T1 to run only t1, and T2 run only t2 ?

if yes, then junit does not have the solution right now.

Tibor17 avatar Nov 05 '12 21:11 Tibor17

In an ideal world, yes. Every @Test should run in its own ClassLoader. More realistically, applying concurrency on a per-class level should be a good first step. So all tests inside one class run sequentially in their own ClassLoader but multiple classes may run at a time. We still run the test of one @Test method impacting the behavior of subsequent @Test methods inside the same class, but at least the multi-threading risk is gone.

cowwoc avatar Nov 05 '12 22:11 cowwoc

@cowwoc the junit does not support this concept of multiple class objects and multiple class loaders. We can run classes/methods in parallel, but not like this. I am afraid you cannot use the current junit runners, and features like Parametrized tests, and annotations like Before/After Class will be called with every method.

Tibor17 avatar Nov 05 '12 22:11 Tibor17

I understand, which is why I filed this issue in the first place. It would be useful to crunch some numbers to check whether this is even reasonable to implement. If it is, we'd have to consider a new API to pull it off.

cowwoc avatar Nov 05 '12 22:11 cowwoc

@cowwoc Have a look at the JUnit API first. Honestly you have to start there, and continue with our tests and features.

Next important question, would it still make sense to spend an effort on it even if trying to work around other framework using this feature candidate in to JUnit?

Tibor17 avatar Nov 05 '12 22:11 Tibor17

I can't speak to the JUnit API. I've used it for years but I'm sure you know its inner workings more than me.

The incentive behind this RFE is to facilitate concurrent tests. When I started converting my tests to run concurrently I very quickly ran into serious problems that made it impossible to go down that road. I don't foresee this problem getting solved without more help from JUnit, but maybe I'm wrong. I'd simply like to point out that if concurrent tests were so easy, JUnit and TestNG would run tests concurrently by default (or very nearly so). Currently, it's a lot more difficult than simply flipping a switch for the majority of code out there.

cowwoc avatar Nov 05 '12 23:11 cowwoc

@cowwoc On the other side reloading classes slows the entire parallelism. If other frameworks are using static context, and you are convinced it's wrong idea, why then fixing it here and not in those frameworks? For me the static context in (lightweight) frameworks is same evil as bytecode intrumentation.

Tibor17 avatar Nov 05 '12 23:11 Tibor17

Even Guice has a problem with static context. You're simply never going to get them all.

I understand reloading classes will slow things down, but it doesn't really matter. AMD has just announced a 16-core CPU and you can be sure there is more to come. It doesn't matter if each test runs slower in each core, so long as you're running the overall suite faster by running more tests in parallel.

cowwoc avatar Nov 05 '12 23:11 cowwoc

@cowwoc I've worked with having custom classloaders that reload classes, and it can be a maintenance headache, because some classes don't work well if the classes they depend on don't keep static state. So I'm not sure we would want to support that directly in JUnit.

If you are still interested in pursuing this, let's talk about extension points. A custom runner is probably the best option, because the test class is loaded by the runner. Could you look at the JUnit code (perhaps start at ParrentRunner) and make a proposal for where this extension point would have to be?

kcooney avatar Jan 29 '14 06:01 kcooney

I created a project on github based on myfaces-test and extracted the minimum required classes. Also fixed a couple of issues including making @Rule annotation work.

https://github.com/bitstrings/junit-clptr

<dependency>
    <groupId>org.bitstrings.test</groupId>
    <artifactId>junit-clptr</artifactId>
    <version>1.0</version>
</dependency>

bitstrings avatar Jun 16 '15 04:06 bitstrings

@bitstrings Looks great! Maybe this can be added to junit.org's home page as a third-party extension?

pholser avatar Jun 16 '15 13:06 pholser

I'm for it but I would like to improve it further.

bitstrings avatar Jun 20 '15 02:06 bitstrings