tiebin-zhang / powermock

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

Mocked system class is not invoked when it's casted into another (non-final) type #445

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Mock a final system class such as StringBuilder 
2. Cast that instance into a CharSequence
3. Call method and expect the mocked call to be invoked.

What is the expected output? What do you see instead?
The expected output is whatever the StringBuilder was mocked to do. Instead the 
non-mocked instance is used so mocked invocations aren't performed.

What version of the product are you using? On what operating system?
Powermock 1.5, Mac OS X 10.7.5

Please provide any additional information below.
See the attached patch for test case and fix.

Original issue reported on code.google.com by jonatan....@softhouse.se on 9 Jun 2013 at 11:47

GoogleCodeExporter commented 9 years ago
This issue is related to 
https://code.google.com/p/powermock/issues/detail?id=135.

Original comment by jonatan....@softhouse.se on 9 Jun 2013 at 11:57

GoogleCodeExporter commented 9 years ago
Unfortunately I also get test errors :/ All tests in 
"samples.testng.SystemClassUserTest" fails, otherwise everything else seems to 
be working. Could you take a look at that?

Original comment by johan.ha...@gmail.com on 10 Jun 2013 at 5:25

GoogleCodeExporter commented 9 years ago
Damn. If you add 

    @ObjectFactory
    public IObjectFactory objectFactory()
    {
        return new PowerMockObjectFactory();
    }

to it. It works.

Original comment by jonatan....@softhouse.se on 10 Jun 2013 at 6:18

GoogleCodeExporter commented 9 years ago
Hm but why is that? It has already been declared in the suite.xml file so it 
shouldn't be necessary  to add an object factory for this class?

Original comment by johan.ha...@gmail.com on 10 Jun 2013 at 6:44

GoogleCodeExporter commented 9 years ago
I wondered that myself. I'll look at it tonight. I don't think having the 
@ObjectFactory in each class is allowed. Why do you have a suite.xml by the 
way? I usually find those cumbersome to use within an IDE.

Original comment by jonatan....@softhouse.se on 10 Jun 2013 at 7:26

GoogleCodeExporter commented 9 years ago
Thanks! I agree but the reason is that it should work with both suite.xml and 
object factory since they are both supported by TestNG.

Original comment by johan.ha...@gmail.com on 10 Jun 2013 at 7:44

GoogleCodeExporter commented 9 years ago
I saw that SampleServletTest also has a @ObjectFactory method, probably to be 
able to run the tests without having to run the whole suite. Wouldn't it be 
simpler if all the testcases just extended PowerMockTestCase and that the 
suite.xml was removed?

Did you run the tests through suite.xml or by hand? When I run the test by 
itself it doesn't use the suite.xml file and thus the errors occurs. IMO it 
should always be possible to run a unit-test without running the whole suite. 

Btw, I see the same error without my changes.

Original comment by jonatan....@softhouse.se on 10 Jun 2013 at 9:58

GoogleCodeExporter commented 9 years ago
I run it from Maven but even though I try to add the objectFactory I can't get 
it to work with your patch :/

I run it like this: 
JAVA_HOME="/System/Library/Java/JavaVirtualMachines/1.6.0.jdk/Contents/Home" 
mvn clean install -Dmaven.javadoc.skip=true -o

When I run it without your patch it works.

Original comment by johan.ha...@gmail.com on 11 Jun 2013 at 6:35

GoogleCodeExporter commented 9 years ago
Powermock have some differences in testng versions: 
modules/module-test/easymock/testng-test/pom.xml : 6.0.1
modules/module-impl/testng/pom.xml : 6.8

By changing 6.0.1 to 6.8 I'm now down to one test error in 
modules/module-test/easymock/testng-test.

Original comment by jonatan....@softhouse.se on 11 Jun 2013 at 4:20

GoogleCodeExporter commented 9 years ago
By changing @PrepareForTest to @PrepareForTest(SampleServlet.class) in 
SampleServletTest the last error was resolved as well.

Original comment by jonatan....@softhouse.se on 11 Jun 2013 at 4:28

GoogleCodeExporter commented 9 years ago
I've tried this as well but it still doesn't build from Maven. From Intellij I 
can get it working when I run the tests one by one :/

Original comment by johan.ha...@gmail.com on 12 Jun 2013 at 6:07

GoogleCodeExporter commented 9 years ago
What errors are you getting?

Original comment by jonatan....@softhouse.se on 12 Jun 2013 at 6:13

GoogleCodeExporter commented 9 years ago
Here's the Maven details:

[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Building powermock-module-test-easymock-testng 1.5.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]
[INFO] --- maven-clean-plugin:2.4.1:clean (default-clean) @ 
powermock-module-test-easymock-testng ---
[INFO] Deleting 
/Users/johan/devtools/java/projects/powermock/code/svn/trunk/modules/module-test
/easymock/testng-test/target
[INFO]
[INFO] --- maven-enforcer-plugin:1.0-beta-1:enforce (enforce-maven) @ 
powermock-module-test-easymock-testng ---
[INFO]
[INFO] --- maven-resources-plugin:2.5:resources (default-resources) @ 
powermock-module-test-easymock-testng ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory 
/Users/johan/devtools/java/projects/powermock/code/svn/trunk/modules/module-test
/easymock/testng-test/src/main/resources
[INFO]
[INFO] --- maven-compiler-plugin:2.3.2:compile (default-compile) @ 
powermock-module-test-easymock-testng ---
[INFO] No sources to compile
[INFO]
[INFO] --- maven-resources-plugin:2.5:testResources (default-testResources) @ 
powermock-module-test-easymock-testng ---
[debug] execute contextualize
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory 
/Users/johan/devtools/java/projects/powermock/code/svn/trunk/modules/module-test
/easymock/testng-test/src/test/resources
[INFO]
[INFO] --- maven-compiler-plugin:2.3.2:testCompile (default-testCompile) @ 
powermock-module-test-easymock-testng ---
[INFO] Compiling 11 source files to 
/Users/johan/devtools/java/projects/powermock/code/svn/trunk/modules/module-test
/easymock/testng-test/target/test-classes
[INFO]
[INFO] --- maven-surefire-plugin:2.10:test (default-test) @ 
powermock-module-test-easymock-testng ---
[INFO] Surefire report directory: 
/Users/johan/devtools/java/projects/powermock/code/svn/trunk/modules/module-test
/easymock/testng-test/target/surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Picked up JAVA_TOOL_OPTIONS: -Dfile.encoding=UTF-8
Running TestSuite
Tests run: 22, Failures: 12, Errors: 0, Skipped: 0, Time elapsed: 1.263 sec <<< 
FAILURE!

Results :

Failed tests:   doGet(samples.testng.SampleServletTest):
  assertThatMockingOfCollectionsWork(samples.testng.SystemClassUserTest):
  assertThatMockingOfFinalSystemClassesWorks(samples.testng.SystemClassUserTest):
  assertThatMockingOfNonFinalSystemClassesWorks(samples.testng.SystemClassUserTest):
  assertThatMockingOfTheRuntimeSystemClassWorks(samples.testng.SystemClassUserTest):
  assertThatMockingStringWorks(samples.testng.SystemClassUserTest):
  assertThatPartialMockingOfFinalSystemClassesWorks(samples.testng.SystemClassUserTest):
  assertThatPartialMockingOfFinalSystemClassesWorksForNonVoidMethods(samples.testng.SystemClassUserTest):
  mockingInetAddressWorks(samples.testng.SystemClassUserTest):
  mockingInstanceMethodOfFinalSystemClassWorks(samples.testng.SystemClassUserTest):
  mockingStaticVoidMethodWorks(samples.testng.SystemClassUserTest):
  mockingURLWorks(samples.testng.SystemClassUserTest): expected same with:<EasyMock for class java.net.URLConnection> but was:<null>

Tests run: 22, Failures: 12, Errors: 0, Skipped: 0

Original comment by johan.ha...@gmail.com on 12 Jun 2013 at 2:03

GoogleCodeExporter commented 9 years ago
Did you add @PrepareForTest(SampleServlet.class) to 
samples.testng.SampleServletTest in modules/module-test/easymock/testng-test? 
Because that is the exact same error I got before I added the 
SampleServlet.class to the annotation. This may break backward-compatibility.

It seems like the first error (doGet(samples.testng.SampleServletTest)) leaves 
some invalid state in the PowerMockObjectFactory as when running the tests 
through surefire the SampleServletTest is executed before SystemClassUserTest. 
In eclipse when I run the suite.xml, SystemClassUserTest is executed before 
SampleServletTest and thus only one error is displayed instead of 12 that 
surefire reports.

Could it be that if a test-class fails (SampleServletTest) it leaves the 
PowerMockObjectFactory in a state where it no longer can mock System classes? 
I'll have to look a bit deeper.

Original comment by jonatan....@softhouse.se on 12 Jun 2013 at 5:31

GoogleCodeExporter commented 9 years ago
The reason for getting 12 errors was that PowerMockTestNGMethodHandler only 
runs:
if(thisMethod.isAnnotationPresent(Test.class))
{
    clearMockFields();
    MockRepository.clear();
}

if the test case doesn't throw anything. Moving this block into a finally block 
now results in one error which is fixed by changing @PrepareForTest to 
@PrepareForTest(SampleServlet.class) in SampleServletTest.

Original comment by jonatan....@softhouse.se on 12 Jun 2013 at 10:38

GoogleCodeExporter commented 9 years ago
While going on the powermock code safari I discovered that EasyMockStateCleaner 
chooses which classloader that has a LastControl loaded, why doesn't it try to 
clear both classloaders? I.e something like this:
private Iterable<ClassLoader> classloadersToClear()
{
    List<ClassLoader> loaders = new ArrayList<ClassLoader>();
    final ClassLoader systemClassLoader = ClassLoader.getSystemClassLoader();
    final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
    if(classFoundInClassloader(LastControl.class, systemClassLoader))
    {
         loaders.add(systemClassLoader);
    }
    if(classFoundInClassloader(LastControl.class, contextClassLoader))
    {
         loaders.add(contextClassLoader);
    }
    return loaders;
}

Is it intentional or a miss?

Original comment by jonatan....@softhouse.se on 12 Jun 2013 at 10:44

GoogleCodeExporter commented 9 years ago
Wow thanks a lot for digging into the code that much! I think not "cleaning" 
all classloaders is a misstake. You've already helped out a great deal but 
would you be so kind to provide an updated patch will all your findings and 
fixes?

Original comment by johan.ha...@gmail.com on 13 Jun 2013 at 5:43

GoogleCodeExporter commented 9 years ago
Coming right up. I'll write a test case for the PowerMockTestNGMethodHandler 
fix as well and attach it to a separate issue.

Original comment by jonatan....@softhouse.se on 13 Jun 2013 at 7:21

GoogleCodeExporter commented 9 years ago
Added SampleServlet.class to the annotation.

Original comment by jonatan....@softhouse.se on 13 Jun 2013 at 7:27

Attachments:

GoogleCodeExporter commented 9 years ago
Attached the state cleaning to 
https://code.google.com/p/powermock/issues/detail?id=54. 

Original comment by jonatan....@softhouse.se on 13 Jun 2013 at 8:07

GoogleCodeExporter commented 9 years ago
Thanks a lot for your help!!! I've applied the patches and committed them to 
trunk.

Original comment by johan.ha...@gmail.com on 14 Jun 2013 at 6:08

GoogleCodeExporter commented 9 years ago
Cool. I'm looking forward to the release!

Original comment by jonatan....@softhouse.se on 14 Jun 2013 at 7:09