PowerMock sometimes causes other TestNG tests to fail #319
Comments
|
From johan.ha...@gmail.com on December 26, 2010 00:39:44 Thanks a lot for the good example and explanation. Would it be possible for you to share your ObjectFactory with us by e.g. sending us a patch? Status: Accepted |
|
From flush...@gmail.com on December 28, 2010 09:57:28 Thank you for the response. Please see the attachment with custom ObjectFactory and annotation that controls usage of PowerMockObjectFactory. I have to note that there is a significant problem with my implementation of ObjectFactory: no cleanup call is made for normal non-PowerMock tests (MockRepository.clear()). Attachment: ConditionalPowerMockObjectFactory.java EnablePowerMockObjectFactory.java |
|
From johan.ha...@gmail.com on December 28, 2010 11:02:00 Thanks! Do you think it would be possible to somehow integrate this in the standard PowerMockObjectFactory instead of having an additional one? |
|
From flush...@gmail.com on December 28, 2010 11:09:39 Yes, it is possible BUT we have to resolve the problem of proper test cleanup first. Ordinary tests that do not declare PowerMock annotations may still use PowerMock.createMock, etc. thus updating the MockRepository. If MockRepository will not be cleared after such test, other tests may be affects if verifyAll is executed. Do you understand what I mean? |
|
From johan.ha...@gmail.com on January 06, 2011 04:08:59 Yes I understand what you mean. But I tend regard this as a user error. If you're using any PowerMock functionality (excluding Whitebox) you must use the PowerMockRunner or PowerMockRule (in JUnit) or PowerMockObjectFactory (in TestNG). The JUnit runner cleans up previous state on startup, not sure if PowerMockObjectFactory does the same, probably not. But it should do so. |
|
From flush...@gmail.com on January 06, 2011 04:19:22 With PowerMockObjectFactory all test classes within the suite will go through 'mock classloader' provided by PowerMock. This will cause issues in existing unit tests and have nothing to do with PowerMock (e.g. ClassCastExceptions). So, it breaks isolation. Original description of the issue stated that "the overall idea is that tests that do not use PowerMock must not be affected by it". |
|
From caoi...@gmail.com on April 05, 2011 04:37:09 Related TestNG JIRA http://jira.opensymphony.com/browse/TESTNG-422 |
|
From johan.ha...@gmail.com on April 05, 2011 05:28:13 Great, thanks. |
|
From cbe...@gmail.com on April 05, 2011 07:21:26 The TestNG issue is closed so it looks there is no action item on my part for this issue? (just making sure) |
|
From johan.ha...@gmail.com on April 05, 2011 07:51:31 cbe: I suppose not. Flush: Could you please verify if things works as you expect? |
|
From caoi...@gmail.com on April 05, 2011 07:59:55 My understanding was that the TestNG guys don't think that a fix is needed in their code, but it is still very difficult to use Powermock in TestNG so I think either some sort of accommodation will have to be reached between the two projects or some sort of work around will be needed in Powermock. Possibly the PowerMockObjectFactory could be adapted to only be used when tests use @PrepareForTest? |
|
From johan.ha...@gmail.com on April 05, 2011 11:27:51 Ah sorry I didn't read the TestNG issue throughly enough, I thought TestNG had implemented a fix already. Have I understood it correctly that the problem is when you have a PowerMock ObjectFactory declared anywhere in your suite then all tests will be using PowerMock (i.e. the PowerMock classloader)? If this is the case I suppose we could patch the PowerMockObjectFactory along the lines of the ConditionalPowerMockObjectFactory attached above but triggering on the @PrepareForTest annotation. Is this a nice way to solve it? |
|
From caoi...@gmail.com on April 06, 2011 01:16:44 Possibly, but I still haven't got my head around the cleanup issues described above (although they don't seem to be affecting me yet). |
|
From johan.ha...@gmail.com on April 06, 2011 23:33:07 I think the problem that flush is talking about is that PowerMock keeps some state in something called the MockRepository which is cleared after each test method to avoid taking up unnecessary memory. It's the ObjectFactory that takes care of cleaning out the state so if the ObjectFactory is not used but you still use PowerMock functionality (such as PowerMock.createMock(..)) there will be unused state in the MockRepository which is not cleared. For large suites it may lead to OOM issues in the end. How ever you should NOT use PowerMock.createMock(..) unless you use the ObjectFactory (or JUnit Runner). I think we could go ahead and implement this. Perhaps it should be made clearer in the Javadoc that you're not suppose to use PowerMock without using the ObjectFactory/Runner. (Whitebox is OK though). |
|
From flush...@gmail.com on April 07, 2011 01:55:24 Hi there, sorry for not responding for a while. @johan It is fine that tests which use PowerMock functionality should leverage ObjectFactory. HOWEVER the problem is that tests which DO NOT USE PowerMock are still affected by ObjectFactory. It causes issues. Have a look at the project I've posted in the bug. |
|
From johan.ha...@gmail.com on April 07, 2011 02:10:15 But wouldn't it work if we use your approach and makes PowerMockObjectFactory "conditional"? I.e. we only use the "PowerMockObjectFactory" (as it is today) when we find a @PrepareForTest annotation in the test class and the default ObjectFactory for test classes not having this annotation. So the new PowerMockObjectFactory should be pretty much like your ConditionalPowerMockObjectFactory but checking @PrepareForTest instead of @EnablePowerMockObjectFactory and delegating to the current "PowerMockObjectFactory" (i.e. we need to copy the current implementation to a new name). |
|
From flush...@gmail.com on April 07, 2011 02:23:28 @johan Yes, it makes sense. |
|
From johan.ha...@gmail.com on April 07, 2011 02:33:58 Ok good. Then I'll implement this asap. Thanks everyone for your help so far. |
|
From johan.ha...@gmail.com on April 07, 2011 13:08:57 I've committed the changes to trunk. Please help me verify whether it helps or not. An issue that I encountered with the new implementation is that if you've annotated fields to make them eligible for mock injection (e.g. using @mock) you now must use the @PrepareForTest annotation on the test class (or a method in the test class) for this to work. I think this is a reasonable trade off but it's a non-backward compatible change. |
|
From flush...@gmail.com on April 09, 2011 13:00:48 I've tested changes in trunk against sample project (attached to original bug report) and found ClassLoader issues. The problem is that even if you use TestNG's ObjectFactoryImpl for test instance creation PowerMockClassloaderObjectFactory may override current thread's ClassLoader (mockLoader). It may lead to ClassCastException(s) and similar errors in test which doesn't use PowerMock but reside in the same test suite. BTW I've tried to change to source of PowerMockObjectFactory and PowerMockClassloaderObjectFactory so that current thread's ClassLoader is set to a normal ClassLoader before test instance creation is delegated to ObjectFactoryImpl. This fix resolved the issue. |
|
From johan.ha...@gmail.com on April 09, 2011 13:20:04 You're right, I overlooked the thread's classloader. By "normal" classloader do you mean the system classloader or the classloader that loads the PowerMockObjectFactory? I suspect we may need to use the latter since otherwise I have a feel we can run into issues when tests are run from e.g. OSGi. Do you think you could you create a patch? Thanks a lot for your help! |
|
From flush...@gmail.com on April 09, 2011 14:42:47 You are right about OSGi and usage of classloader that loads the PowerMockObjectFactory. I think I can create a patch, but not earlier than Monday (have limited access to my dev env at the moment). |
|
From johan.ha...@gmail.com on April 10, 2011 10:17:34 It would be nice if you could create a patch. I would also be really glad if someone could try out the new Javaagent based bootstrapper I've been playing around with during the weekend. If you build PowerMock and add powermock-module-testng-agent instead powermock-module-testng to your pom PowerMock should bootstrap from the javaagent instead of using the custom classloader. This means that you shouldn't have any problems with ClassCastException's and other classloading related issues (I don't think it'll work in OSGi though). |
|
From johan.ha...@gmail.com on April 18, 2011 02:41:02 I think I've fixed it in trunk. Please try it out. |
|
From flush...@gmail.com on May 20, 2011 09:48:00 Tried out the fix, works fine. Thank you! |
|
From johan.ha...@gmail.com on May 21, 2011 08:44:52 Great. Is it alright to close the issue? |
|
From flush...@gmail.com on May 21, 2011 15:02:56 Yes! |
|
From johan.ha...@gmail.com on May 22, 2011 00:52:10 Status: Fixed |
From flush...@gmail.com on December 16, 2010 19:55:01
Please see the attached small demo project with two test classes that demonstrate the problem.
My environment is the following:
Windows 7 Enterprise 64-bit
JDK 1.6.0_22
Maven 2.2.1
PowerMock 1.4.6
TestNG 5.14.1
ServiceTest uses PowerMock to mock class with final modifier (@PrepareForTest and @objectfactory are there). XmlReaderTest just creates new instance of SAXParser. Each of the tests succeed when they run independently. However XmlReaderTest fails when running the whole suite with the following exception:
java.lang.ClassCastException: com.sun.org.apache.xerces.internal.parsers.SAXParser cannot be cast to org.xml.sax.XMLReader
FAQ describes this issue and advises to solve it by adding @PowerMockIgnore({"javax.xml.", "org.xml.sax."}) to XmlReaderTest.
And this fix works. BUT why does PowerMock alter behavior of other tests? It is definetely not OK.
There are other classloading issues that are not demoed in this sample. The overall idea is that tests that do not use PowerMock must not be affected by it.
Partially the problem is that there is only one ObjectFactory per suite and this ObjectFactory is responsible for creating all test class instances.
I have solved the problem by writing and applying custom ObjectFactory that inspects test class and delegates creation of test class instance to PowerMockObjectFactory ONLY if test class has PowerMock annotations. Otherwise ObjectFactory delegates creation to default ObjectFactoryImpl.
It seems to me that similar ObjectFactory logic should be the part of the library. Classloader that PowerMock uses causes issues for other tests frequently and sometimes it is VERY HARD to understand what is going wrong.
Attachment: affecting-other-tests.zip
Original issue: http://code.google.com/p/powermock/issues/detail?id=299
The text was updated successfully, but these errors were encountered: