search

jmockit / jmockit1

Advanced Java library for integration testing, mocking, faking, and code coverage
Other
465 stars 240 forks source link

NullPointerException in SavePoint.rollback() #288

Closed Vampire closed 8 years ago

Vampire commented 8 years ago

I'm currently upgrading our JMockit 1.8 tests to JMockit 1.24.

One of my colleagues used the MockUps API in some strange way that I will try to rewrite, but nevertheless it triggered an NPE in JMockit that probably shouldn't be there. I fear I cannot really give a code sample. But basically there were some MockUp instance fields, those were assigned with mocks in a @Before method, then in an @After method their tearDown() methods were called (there is no sense in doing so, is there?) and in some of the Tests also tearDown() was called and a new MockUp instance created and assigned to the instance field.

This resulted in the StackTrace

java.lang.NullPointerException
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.runTestClass(JUnitTestClassExecuter.java:86)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecuter.execute(JUnitTestClassExecuter.java:49)
    at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassProcessor.processTestClass(JUnitTestClassProcessor.java:69)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:48)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
    at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.messaging.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:32)
    at org.gradle.messaging.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:93)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:105)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:35)
    at org.gradle.messaging.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.messaging.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:355)
    at org.gradle.internal.concurrent.DefaultExecutorFactory$StoppableExecutorImpl$1.run(DefaultExecutorFactory.java:64)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

which I found out after breaking into the NPE constructor actually comes from mockit.internal.state.MockClasses.SavePoint#rollback:169 where mockUpData.numberOfMockupsForSingleInstance = mockUpClassAndCount.getValue(); is done, but for one loop-iteration mockUpData actually is null and thus the NPE is thrown.

After removing all tearDown() calls (especially the ones in the test methods), the NPE was gone and the tests green.

rliesenfeld commented 8 years ago

It was probably a threading issue. I will consider adding a null check in that rollback method.

Calling tearDown() is something one should do only in the exceptional case where the mock-up needs to get undone at the middle of a test; apart from that it gets called automatically at the appropriate time.

Vampire commented 8 years ago

Ok, thought so, thanks.

It was unlikely a threading issue as it was 100% reproducible.