New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PowerMockRunner processes JUnit Rules incorrectly #427

Closed
johanhaleby opened this Issue Jul 24, 2015 · 9 comments

Comments

Projects
None yet
1 participant
@johanhaleby
Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 09, 2012 09:55:39

What steps will reproduce the problem? Use test class in attachment

  1. Run testWithoutException() without PowerMockRunner

  2. Run testWithoutException() with PowerMockRunner

  3. Run testWithException() without PowerMockRunner

  4. Run testWithException() with PowerMockRunner What is the expected output? What do you see instead? 1. NO EXCEPTION - START
    NO EXCEPTION - STOP
    Rule1
    Rule2 2. Rule1
    NO EXCEPTION - START
    NO EXCEPTION - STOP
    Rule2 3. EXCEPTION - START

Rule1
EXCEPTION - START What version of the product are you using? On what operating system? JUnit 4.8.2
PowerMock 1.4.12 Please provide any additional information below. As you can see, PowerMockRunner handles JUnit Rules differently than BlockJUnit4ClassRunner. Please compare: PowerMockJUnit47RunnerDelegateImpl#executeTest(Method, Object, Runnable) with BlockJUnit4ClassRunner#withRules(FrameworkMethod, Object, Statement).

JUnit PASSES each Rule's returned statement and the last declared rule gets statement that evaluates test method. On contrary PowerMockRunner runs every declared rule one by one.

Please see java doc of MethodRule: "Multiple {@link MethodRule}s can be applied to a test method. The Statement that executes the method is passed to each annotated Rule in turn, and each may return a substitute or modified Statement, which is passed to the next {@link Rule}, if any.".
It means that PowerMockRunner behaviour is wrong.

Attachment: TestClass.java

Original issue: http://code.google.com/p/powermock/issues/detail?id=407

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From johan.ha...@gmail.com on November 29, 2012 10:47:17

Thanks for reporting. It would be really nice if you could help out with this and fix the rule support in PowerMock and provide a patch. Hopefully a new version of PowerMock will be released soon and it would be good if this is fixed until then. But I need help from the community.

Collaborator

johanhaleby commented Jul 24, 2015

From johan.ha...@gmail.com on November 29, 2012 10:47:17

Thanks for reporting. It would be really nice if you could help out with this and fix the rule support in PowerMock and provide a patch. Hopefully a new version of PowerMock will be released soon and it would be good if this is fixed until then. But I need help from the community.

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From Iso.poc...@gmail.com on November 29, 2012 11:43:38

I'm afraid I am not able to help you. I really appreciate your work but we have decided to stop using Power mock. There were some others issue that forces us to do so.
The main one was that we cannot run test using PowerMock as Eclipse's JUnit Plugin Test (OSGi environment). It was caused by dynamic class loading (Class.forName()) which requires us to use Buddy-Policy (which is a lame solution). Even so we get some errors from javassist library (probably because we are using Java 1.7).

Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 29, 2012 11:43:38

I'm afraid I am not able to help you. I really appreciate your work but we have decided to stop using Power mock. There were some others issue that forces us to do so.
The main one was that we cannot run test using PowerMock as Eclipse's JUnit Plugin Test (OSGi environment). It was caused by dynamic class loading (Class.forName()) which requires us to use Buddy-Policy (which is a lame solution). Even so we get some errors from javassist library (probably because we are using Java 1.7).

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From Iso.poc...@gmail.com on November 29, 2012 11:56:42

By the way: is there (sorry that I am asking without even searching) any documentation what kind of cases PowerMock are dealing with and how it is going to achieve that (what kind of byte code manipulations) ?
Moreover some kind of developer documentation (archotecture overview) and list of common problems that you have encountered during your work ?
In other work is it possible to third-party developer can quite easy support your work ?
I don't think that fixing this particular problem without seeing whole picture can really work. It may or it may not.

How do you test the library ? Do you run test in multiple configurations (different Java versions, different versions of JUnit/TestNG) ?

Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 29, 2012 11:56:42

By the way: is there (sorry that I am asking without even searching) any documentation what kind of cases PowerMock are dealing with and how it is going to achieve that (what kind of byte code manipulations) ?
Moreover some kind of developer documentation (archotecture overview) and list of common problems that you have encountered during your work ?
In other work is it possible to third-party developer can quite easy support your work ?
I don't think that fixing this particular problem without seeing whole picture can really work. It may or it may not.

How do you test the library ? Do you run test in multiple configurations (different Java versions, different versions of JUnit/TestNG) ?

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From johan.ha...@gmail.com on November 29, 2012 21:42:34

You've probably made the right decision not to use PowerMock with OSGi :) Even though I'm the founder of PowerMock I usually don't advise people to use it unless there's no other way out. PowerMock should be for experienced developers since most often than not it'll probably give you more pain (in terms of maintainability issues) than what you actually gain. There are corner cases where I believe it's legitimate to use PowerMock they are quite few.

Back to your questions (which are excellent), there's no good developer implementation and that could of course be a major factor to why it's hard to contribute to PowerMock. I've written some blogs about it (such as this one that I usually refer to as a very high level introduction: http://www.jayway.com/2009/05/17/mocking-static-methods-in-java-system-classes/ ) People have contributed but there has not been any major help in the project for a long time.

I'm trying my best to make PowerMock go around but it's hard to find all the time needed. PowerMock is really something I maintain on my spare time and I have other open source projects as well which I'm more interested in right now. And if that's not enough I have two jobs and (quite amazingly :)) a girlfriend so time is quite precious :)

Collaborator

johanhaleby commented Jul 24, 2015

From johan.ha...@gmail.com on November 29, 2012 21:42:34

You've probably made the right decision not to use PowerMock with OSGi :) Even though I'm the founder of PowerMock I usually don't advise people to use it unless there's no other way out. PowerMock should be for experienced developers since most often than not it'll probably give you more pain (in terms of maintainability issues) than what you actually gain. There are corner cases where I believe it's legitimate to use PowerMock they are quite few.

Back to your questions (which are excellent), there's no good developer implementation and that could of course be a major factor to why it's hard to contribute to PowerMock. I've written some blogs about it (such as this one that I usually refer to as a very high level introduction: http://www.jayway.com/2009/05/17/mocking-static-methods-in-java-system-classes/ ) People have contributed but there has not been any major help in the project for a long time.

I'm trying my best to make PowerMock go around but it's hard to find all the time needed. PowerMock is really something I maintain on my spare time and I have other open source projects as well which I'm more interested in right now. And if that's not enough I have two jobs and (quite amazingly :)) a girlfriend so time is quite precious :)

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From Iso.poc...@gmail.com on November 30, 2012 01:31:37

I can agree that in most cases developers should not use PowerMock but just refactor code to create classes that are testable (program to interfaces, use Dependency Injection, prevent creating highly coupled classes, avoid sharing complex business logic through static utility classes, etc), but ...
I see potential of PowerMock when your code relay on third-party-libraries or frameworks. For example: Eclipse 3.x Platform lacks of IoC container, which encourages to use static utilicty classes/methods a lot. Moreover there are plenty of places when you need to obtain Eclipse's Workbench - through some static methods.
The other example is when you are using third party library's class which is final or has some final methods. To do some unit tests you have to (want to ?) mock this class.

But lets back to the original issue. I'll try to provide some patch but I'm not an expert of JUnit. Moreover as far as I know the newest version of JUnit deprecates usage of MethodRule in favour of TestRule. So it may (or may not) be handled differently in the newest version (currently I'm using JUnit 4.8.2).

Generally I think that fact that PowerMock needs it own runner: PowerMockRunner is a source of maintenance hell. If JUnit implements new feature you also have to do it (or maybe I'm wrong - I'm not very familiar how exactly PowerMockRunner works. I only imagine that the "only" think that it should do is to somehow cheat class loader ;. Here comes idea whether we can use Aspect Programming to achieve that)).

For me personally I want to use PowerMock to:

  • mock final classes
  • mock final methods
  • mock static methods in utility classes

I'm thinking about investigate how to deal with it basing on yours experience. I'm not familiar with PowerMock requirements but form me it should work under JUnit and EasyMock and to be as more transparent as possible.

Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 30, 2012 01:31:37

I can agree that in most cases developers should not use PowerMock but just refactor code to create classes that are testable (program to interfaces, use Dependency Injection, prevent creating highly coupled classes, avoid sharing complex business logic through static utility classes, etc), but ...
I see potential of PowerMock when your code relay on third-party-libraries or frameworks. For example: Eclipse 3.x Platform lacks of IoC container, which encourages to use static utilicty classes/methods a lot. Moreover there are plenty of places when you need to obtain Eclipse's Workbench - through some static methods.
The other example is when you are using third party library's class which is final or has some final methods. To do some unit tests you have to (want to ?) mock this class.

But lets back to the original issue. I'll try to provide some patch but I'm not an expert of JUnit. Moreover as far as I know the newest version of JUnit deprecates usage of MethodRule in favour of TestRule. So it may (or may not) be handled differently in the newest version (currently I'm using JUnit 4.8.2).

Generally I think that fact that PowerMock needs it own runner: PowerMockRunner is a source of maintenance hell. If JUnit implements new feature you also have to do it (or maybe I'm wrong - I'm not very familiar how exactly PowerMockRunner works. I only imagine that the "only" think that it should do is to somehow cheat class loader ;. Here comes idea whether we can use Aspect Programming to achieve that)).

For me personally I want to use PowerMock to:

  • mock final classes
  • mock final methods
  • mock static methods in utility classes

I'm thinking about investigate how to deal with it basing on yours experience. I'm not familiar with PowerMock requirements but form me it should work under JUnit and EasyMock and to be as more transparent as possible.

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From Iso.poc...@gmail.com on November 30, 2012 01:33:40

I forget to write that I know you have implement some PowerMockRule to not to be force to use PowerMock Runner. I think that is the good path, but how does it fits into TestNG (I'm also not familiar with this testing library) ?

Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 30, 2012 01:33:40

I forget to write that I know you have implement some PowerMockRule to not to be force to use PowerMock Runner. I think that is the good path, but how does it fits into TestNG (I'm also not familiar with this testing library) ?

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From johan.ha...@gmail.com on November 30, 2012 01:51:19

Yes when using certain containers you're sometimes "forced" to use something like PowerMock but in several cases there are other ways to go about (but not always, and this is when PowerMock can be useful). And even if you need to use PowerMock you should consider if you really need mocking. Many times it's better to use the stubbing API in PowerMock (see https://code.google.com/p/powermock/wiki/SuppressUnwantedBehavior ) to simply stub, suppress or replace a method/field etc.

The PowerMockRunner IS a maintenance hell. It was initially created back in 2007 and if I only had enough time it would be rewritten from scratch a long time ago. Especially now that JUnit has change a lot of internal stuff and the runner could be made MUCH simpler. There are lots of technical accidental complexity in there.

I'm not that well-rounded with TestNG either so I haven't fully explored something similar to the rule in TestNG. But you can use a java agent based bootstrap approach for TestNG ( https://code.google.com/p/powermock/wiki/PowerMockAgent ).

Collaborator

johanhaleby commented Jul 24, 2015

From johan.ha...@gmail.com on November 30, 2012 01:51:19

Yes when using certain containers you're sometimes "forced" to use something like PowerMock but in several cases there are other ways to go about (but not always, and this is when PowerMock can be useful). And even if you need to use PowerMock you should consider if you really need mocking. Many times it's better to use the stubbing API in PowerMock (see https://code.google.com/p/powermock/wiki/SuppressUnwantedBehavior ) to simply stub, suppress or replace a method/field etc.

The PowerMockRunner IS a maintenance hell. It was initially created back in 2007 and if I only had enough time it would be rewritten from scratch a long time ago. Especially now that JUnit has change a lot of internal stuff and the runner could be made MUCH simpler. There are lots of technical accidental complexity in there.

I'm not that well-rounded with TestNG either so I haven't fully explored something similar to the rule in TestNG. But you can use a java agent based bootstrap approach for TestNG ( https://code.google.com/p/powermock/wiki/PowerMockAgent ).

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From Iso.poc...@gmail.com on November 30, 2012 02:16:33

I have some kind of proposal to you. I would like to do "review" of your work. I'll explore PowerMock wiki, and the link that you have provided to gather some requirements.
Then I'll mark those requirements that are really useful (especially for me) and check how the current code is fitting there. I'll try to create some prototype how to do it better/easier/more transparent/easier to maintenance (of course If I only can and it is possible) in contrast to some requirements subset.

From you I "only" require expert knowledge and some feedback. Of course this kind of cooperation can take forever and I am aware of that (I have also a lot of my own work).

Collaborator

johanhaleby commented Jul 24, 2015

From Iso.poc...@gmail.com on November 30, 2012 02:16:33

I have some kind of proposal to you. I would like to do "review" of your work. I'll explore PowerMock wiki, and the link that you have provided to gather some requirements.
Then I'll mark those requirements that are really useful (especially for me) and check how the current code is fitting there. I'll try to create some prototype how to do it better/easier/more transparent/easier to maintenance (of course If I only can and it is possible) in contrast to some requirements subset.

From you I "only" require expert knowledge and some feedback. Of course this kind of cooperation can take forever and I am aware of that (I have also a lot of my own work).

@johanhaleby

This comment has been minimized.

Show comment
Hide comment
@johanhaleby

johanhaleby Jul 24, 2015

Collaborator

From johan.ha...@gmail.com on November 30, 2012 02:28:59

That sounds good to me. Just let me know and I'll try to answer. Feel free to e-mail me directly if you like.

Collaborator

johanhaleby commented Jul 24, 2015

From johan.ha...@gmail.com on November 30, 2012 02:28:59

That sounds good to me. Just let me know and I'll try to answer. Feel free to e-mail me directly if you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment