noushadali / powermock

Automatically exported from code.google.com/p/powermock
Apache License 2.0
0 stars 0 forks source link

whenNew Fails to Match Private Constructor (with args) of a Static Inner Class #422

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

  Sample code under test:

public class Outer {
    public void testMe() {
        System.out.println("Creating Inner");
        Inner inner = new Inner("Outer");
        System.out.println(inner.getMessage());
    }

    static class Inner {
        private String caller;

        private Inner(String caller) {
            this.caller = caller;
        }

        public String getMessage() {
            return "Hello " + this.caller;
        }
    }
}

  JUnit test case to reproduce bug:

@RunWith(PowerMockRunner.class)
@PrepareForTest(Outer.class)
public class OuterTest {

    @Test
    public void testOuter() throws Exception {
        Outer.Inner mockInner = mock(Outer.Inner.class);   whenNew(Outer.Inner.class).withArguments("Outer").thenReturn(mockInner);
        when(mockInner.getMessage()).thenReturn("Hello from a mock");
        Outer underTest = new Outer();
        underTest.testMe(); // should print to stdout "Hello from a mock"
    }
}

What is the expected output? What do you see instead?
  Test should print to stdout "Hello from a mock", but instead a NullPointerException is thrown because no object is created by an expected mock constructor invocation.

What version of the product are you using? On what operating system?
  PowerMock 1.4.12 (and 1.5)
  Mockito 1.9.0 (and 1.9.5)
  Javassist 3.17.1-GA
  Windows 7 Ultimate 64-bit
  Oracle JDK 1.7.0_07 64-bit
  JUnit 4.10

Please provide any additional information below.

I suspect this bug may be introduced by the latest release of Javassist, which 
we're using to resolve many issues testing under Java 7.

When PowerMock searches for a matching constructor invocation, it fails due to 
the args array parameter to MockGateway.newInstanceCall() having an extra 
element equaling to null.  When debugging the source, this method has a comment 
about handling such a scenario with inner classes, but only if the null element 
is first in the array.  In this scenario, the null element appears last.

whenNew() works matching a no-arg constructor of a static inner class, but not 
with one or more args.  One workaround is to not mark the constructor private, 
if default visibility is acceptable, and if the code under test can be modified.

I don't know what is ultimately responsible for building the args array passed 
into MockGateway.newInstanceCall(), but since this method has a comment about 
handling a very similar scenario, I patched it with the suggested fix below:

    public static synchronized Object newInstanceCall(Class<?> type, Object[] args, Class<?>[] sig) throws Throwable {
        final NewInvocationControl<?> newInvocationControl = MockRepository.getNewInstanceControl(type);
        if (newInvocationControl != null) {
            /*
             * We need to deal with inner, local and anonymous inner classes
             * specifically. For example when new is invoked on an inner class
             * it seems like null is passed as an argument even though it
             * shouldn't. We correct this here.
             *
             * PATCH: Seems with Javassist 3.17.1-GA, the 'null' is passed
             * as the last argument.
             */
            if (type.isMemberClass() && Modifier.isStatic(type.getModifiers())) {
                    /*** BEGIN PATCH ***/
                if (args.length > 0 && (args[0] == null || args[args.length -1] == null) && sig.length > 0) {
                /*** END PATCH ***/
                    args = copyArgumentsForInnerOrLocalOrAnonymousClass(args);
                }
.
.
.
    private static Object[] copyArgumentsForInnerOrLocalOrAnonymousClass(Object[] args) {
        Object[] newArgs = new Object[args.length - 1];
        /*** BEGIN PATCH ***/
        int start = 0;
        int end = 0;
        int j = 0;

        if (args[0] == null) {
            start = 1;
            end = args.length;
        } else {
            start = 0;
            end = args.length -1 ;
        }
        for (int i = start; i < end; i++) {
            newArgs[j++] = args[i];
                /*** END PATCH ***/
        }
        args = newArgs;
        return args;
    }

Original issue reported on code.google.com by oliverhe...@gmail.com on 14 Jan 2013 at 12:29

GoogleCodeExporter commented 9 years ago
Thanks for the patch! But could you please help out by creating a real patch 
(see 
https://ariejan.net/2007/07/03/how-to-create-and-apply-a-patch-with-subversion) 
from the latest source? That would make it much simpler for me. Also if you 
like please run the test suite in PowerMock to make sure that all tests pass 
(you need Java 6 for this and it's best to run something like: "mvn clean 
install -Dmaven.javadoc.skip=true" since javadoc generation is slow).

Original comment by johan.ha...@gmail.com on 22 Jan 2013 at 7:49

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Ok, no problem.  I'll comment here on my progress.  It may take me a while 
since the project at my job where I discovered this uses Java 7, I'll have to 
setup and work on this  at home.

Original comment by oliverhe...@gmail.com on 23 Jan 2013 at 4:41

GoogleCodeExporter commented 9 years ago
Thanks!

Original comment by johan.ha...@gmail.com on 23 Jan 2013 at 8:35

GoogleCodeExporter commented 9 years ago
I have a patch of the suggested fix ready, but some tests are failing.  I'll 
look into them and see if I can fix the problems, depending on whether or not I 
introduced a bug.

Original comment by oliverhe...@gmail.com on 1 Feb 2013 at 1:38

GoogleCodeExporter commented 9 years ago
Great work! Do you use Java 6 when you build PowerMock? (That is VERY 
important). If you upload the patch I can try it out as well.

Original comment by johan.ha...@gmail.com on 1 Feb 2013 at 7:05

GoogleCodeExporter commented 9 years ago
Here's the patch of the changes.
In trying to troubleshoot the failing tests, for some reason, I'm getting 
errors due to the wildcard imports in Eclipse.  I'm on a Mac using the last 
JDK6 Apple released, so maybe this is a quirk with that compiler.  I'll try 
locally removing the wildcard imports so that I can work in my environment, and 
if I fix the tests, I won't include those changes in a patch.

Original comment by oliverhe...@gmail.com on 2 Feb 2013 at 1:48

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks! I've tried to apply the patch and when I run the entire test suite from 
Maven the following tests fail:

assertLocalClassMockingWorksWithArguments(samples.junit4.classwithinnermembers.C
lassWithInnerMembersTest)   
assertNonStaticMemberClassMockingWorksWithArguments(samples.junit4.classwithinne
rmembers.ClassWithInnerMembersTest)
shouldFindMemberVarArgsCtorWhenPassingArrayToWithArguments(samples.powermockito.
junit4.constructor.InnerConstructorsAreFoundTest): Argument is not an array

Original comment by johan.ha...@gmail.com on 2 Feb 2013 at 11:03

GoogleCodeExporter commented 9 years ago
I only get 2 of the 3 same failures:

Results :

Failed tests: 
  assertLocalClassMockingWorksWithArguments(samples.junit4.classwithinnermembers.ClassWithInnerMembersTest): 
  Unexpected constructor call samples.classwithinnermembers.ClassWithInnerMembers$2MyLocalClass(samples.classwithinnermembers.ClassWithInnerMembers@3e0d1329):
    samples.classwithinnermembers.ClassWithInnerMembers$2MyLocalClass("my value"): expected: 1, actual: 0
  assertNonStaticMemberClassMockingWorksWithArguments(samples.junit4.classwithinnermembers.ClassWithInnerMembersTest): 
  Unexpected constructor call samples.classwithinnermembers.ClassWithInnerMembers$MyInnerClassWithConstructorArgument(samples.classwithinnermembers.ClassWithInnerMembers@4d91f801):
    samples.classwithinnermembers.ClassWithInnerMembers$MyInnerClassWithConstructorArgument("value"): expected: 1, actual: 0

Tests run: 296, Failures: 2, Errors: 0, Skipped: 6

Original comment by oliverhe...@gmail.com on 6 Feb 2013 at 2:19

GoogleCodeExporter commented 9 years ago
Ok, I fixed the failures I was getting.  Turned out there was another scenario 
I wasn't aware of that my first fix broke.  Maven run shows everything passing.

Original comment by oliverhe...@gmail.com on 6 Feb 2013 at 3:38

Attachments:

GoogleCodeExporter commented 9 years ago
All tests seem to pass, good work :) I think it would be really nice if you 
could create a test case that verifies that the issue is solved. Otherwise it 
may easily lead to regression bugs.

Original comment by johan.ha...@gmail.com on 6 Feb 2013 at 7:04

GoogleCodeExporter commented 9 years ago
Definitely, I'm already planning to the next window of free time I get.  :)

Original comment by oliverhe...@gmail.com on 6 Feb 2013 at 8:27

GoogleCodeExporter commented 9 years ago
That would be great! Thanks for your help so far.

Original comment by johan.ha...@gmail.com on 6 Feb 2013 at 8:33

GoogleCodeExporter commented 9 years ago
Ah, I see you committed my patch, thanks!

Here's a unit test for this issue.

If you can point me in the right direction, I can also write a test for this 
issue that attempts to reproduce the problem as I originally described.

Original comment by oliverhe...@gmail.com on 16 Feb 2013 at 11:48

Attachments:

GoogleCodeExporter commented 9 years ago
Forgot to mention, when I get back to my work environment where I originally 
encountered this issue, I'll test this under Java 7 & Javassist 3.17.1-GA.

Original comment by oliverhe...@gmail.com on 16 Feb 2013 at 11:55

GoogleCodeExporter commented 9 years ago
At work with Java 7 & Javassist 3.17.1-GA, tests failed with the following:

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running powermock.classloading.ObjenesisDeepClonerTest
#
# A fatal error has been detected by the Java Runtime Environment:
#
#  EXCEPTION_ACCESS_VIOLATION (0xc0000005) at pc=0x000000000294e6a5, pid=4964, t
id=7784
#
# JRE version: 7.0_11-b21
# Java VM: Java HotSpot(TM) 64-Bit Server VM (23.6-b04 mixed mode windows-amd64
compressed oops)
# Problematic frame:
# j  java.lang.Class.privateGetDeclaredFields(Z)[Ljava/lang/reflect/Field;+51
#
# Failed to write core dump. Minidumps are not enabled by default on client vers
ions of Windows
#
# An error report file with more information is saved as:
# C:\dev\lib\powermock-read-only\classloading\classloading-objenesis\hs_err_pid4
964.log
#
# If you would like to submit a bug report, please visit:
#   http://bugreport.sun.com/bugreport/crash.jsp
#
[INFO] ------------------------------------------------------------------------
[ERROR] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] There are test failures.

Please refer to C:\dev\lib\powermock-read-only\classloading\classloading-objenes
is\target\surefire-reports for the individual test results.
[INFO] ------------------------------------------------------------------------
[INFO] For more information, run Maven with the -e switch
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4 minutes 23 seconds
[INFO] Finished at: Sun Feb 17 14:49:41 EST 2013
[INFO] Final Memory: 274M/516M
[INFO] ------------------------------------------------------------------------

Original comment by oliverhe...@gmail.com on 27 Feb 2013 at 7:08

GoogleCodeExporter commented 9 years ago
Yes that is a known isssue on Java 7. You need to build with Java 6.

Original comment by johan.ha...@gmail.com on 28 Feb 2013 at 7:04

GoogleCodeExporter commented 9 years ago
Is there any fix for above mentioned fatal error for 
powermock.classloading.ObjenesisDeepClonerTest for java 7 or java 8?

Original comment by chirag.o...@gmail.com on 9 Jan 2015 at 8:31

GoogleCodeExporter commented 9 years ago
Good question.  It's been a long time since I've had to deal with this issue, 
and I'm no longer working on the project where we needed this fix.  I'm 
assuming now almost 2 years later there's still a problem under Java 7, and now 
8?  I would think by now PowerMock fully supports Java 7, so hopefully a 
project member can give us a clear answer.

Original comment by mr.olive...@gmail.com on 15 Jan 2015 at 2:19