Skip to content
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

Use ByteBuddy to instrument classes instead Javassist #727

Open
thekingn0thing opened this issue Dec 1, 2016 · 66 comments · Fixed by #911
Open

Use ByteBuddy to instrument classes instead Javassist #727

thekingn0thing opened this issue Dec 1, 2016 · 66 comments · Fixed by #911

Comments

@thekingn0thing
Copy link
Member

thekingn0thing commented Dec 1, 2016

The main issue with Javassist it's that it read classes from disk before instrumenting. Such behaviour breaks compatibility with other tools which are instrumenting classes (see discussion jacoco/jacoco#51)

ByteBuddy could help us to resolve the issue and supporting online Jacoco Instrumenting.

ByteBudy agent has to be used for implementation of powermock-agent

@thekingn0thing thekingn0thing added this to the PowerMock 2.0.0 milestone Dec 1, 2016
@thekingn0thing thekingn0thing mentioned this issue Dec 1, 2016
6 tasks
thekingn0thing pushed a commit that referenced this issue Mar 18, 2017
Refactoring MockClassLoader  - extract Javassist relative code to a new classes.

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
thekingn0thing pushed a commit that referenced this issue Mar 18, 2017
Refactoring MockClassLoader  - extract Javassist relative code to a new classes.

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
@thekingn0thing thekingn0thing self-assigned this Mar 18, 2017
thekingn0thing pushed a commit that referenced this issue Apr 17, 2017
Refactoring MockClassLoader  - extract Javassist relative code to a new classes.
thekingn0thing pushed a commit that referenced this issue Apr 17, 2017
Refactoring MockTransformer  - split transformer to set of small classes.
thekingn0thing pushed a commit that referenced this issue Apr 17, 2017
Refactoring MockClassLoader  - extract Javassist relative code to a new classes.
thekingn0thing pushed a commit that referenced this issue Apr 17, 2017
Refactoring MockTransformer  - split transformer to set of small classes.
@fabriziomieli
Copy link

Hi, When this is supposed to be ready?

@thekingn0thing
Copy link
Member Author

thekingn0thing commented Apr 26, 2017 via email

@fabriziomieli
Copy link

Thank you very much

thekingn0thing pushed a commit that referenced this issue May 1, 2017
Implement modifiers related transformer

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
thekingn0thing pushed a commit that referenced this issue May 1, 2017
Implement static initialiser transformer

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
thekingn0thing pushed a commit that referenced this issue Jun 18, 2017
Refactoring MockClassLoader  - extract Javassist relative code to a new classes.
thekingn0thing pushed a commit that referenced this issue Jun 18, 2017
Refactoring MockTransformer  - split transformer to set of small classes.
thekingn0thing pushed a commit that referenced this issue Jun 18, 2017
Implement modifiers related transformer

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
thekingn0thing pushed a commit that referenced this issue Jun 18, 2017
Implement static initialiser transformer

Signed-off-by: Arthur Zagretdinov <arthur.zagretdinov@outlook.com>
thekingn0thing pushed a commit that referenced this issue Jun 18, 2017
@peter-janssen
Copy link

Will you release this feature when it is ready or only as part of PowerMock 2.0?

@thekingn0thing
Copy link
Member Author

I'm going to release it as part of PowerMock 2.0, because the change for ButyBuddy is close intertwined with clearing PowerMock from copy-pasted mockito code.

@peter-janssen
Copy link

The 2.0 release looks quite big so I guess the earlier estimate of end of june (you did mean 2017, right?) will be pushed back quite a bit?

@benken-parasoft
Copy link

I am primarily concerned about java version support.

With PowerMock 22 using Javassist again, it looks like PowerMock would only work with Java 9 but not 10 or 11. At least, the pom.xml for PowerMock 2.0.0-RC.2 shows a dependency on Javassist 3.22.0-GA which looks to support Java 9. Perhaps it be possible to use PowerMock with Javassist 3.24.0-GA for Java 11 support instead? Previously, with PowerMock 2.0.0 beta 11, I was able to use ByteBuddy 1.9 for Java 11 support.

@thekingn0thing
Copy link
Member Author

@benken-parasoft

I agree that ByteBuddy is a great well-supported library. However, it does not cover all PowerMock needs, at least for now.

It’s well applied when you need advice your code or generates a code. However, to support such features as constructor stubbing, mocking constructor calls or mocking System class I have to modify bytecode in instruction level where ByteBuddy features end. I have only one option - use ASM in addition to ByteBuddy. As it means that I have to:

  • go deep into JVM implantation.
  • manually track stack, local variables and so on (with a help from the ByteBuddy side, however, there are still a lot of work to do)
  • care about JVM changes and aspect of implementation

As result, it takes a lot of time to implement the features that already worked with Javassist. I tried to implement it for 6 months, however, with a time that I can give to PowerMock I did not achieve a big progress. The task is too complicated. You need fully focus on it - what hardly achievable when you work for 30-60 minutes per day.

I’m not going to stop working at all on migrating to ByteBuddy. I just changed priorities to be able to release what was already implemented the year ago.

At the end of the day, my decision is aimed to be able to deliver supporting new Java. Thank @ijuma, he provided the pull request to support Java 11 (#952) and @eolivelli who raised the pull request with the fix for Java 12 (#939).

@jhyry-gcpud
Copy link

Is this issue still on the table?

@Tibor17
Copy link

Tibor17 commented Jan 31, 2019

I understand it so that ByteBuddy should be on the table. Without it the JaCoCo report would not cover these tests with PowerMock or Mockito. At least this is my understanding, and some people would see other reasons especially internal ones in PowerMock.

@nEdAy
Copy link

nEdAy commented Mar 6, 2019

Is this issue still on the table?

@vijjukumar
Copy link

Is this issue still on the table ? we do have any alternative approach. Thanks

@Vampire
Copy link
Contributor

Vampire commented Sep 15, 2019

How is the current state of this?
I used JaCoCo online instrumentation with PowerMock for mocking a system class.
It seems there was JaCoCo data recorded, but it doesn't match the class files when the report is generated, so I guess only the order of instrumentation is a problem maybe?

@ninjubohra
Copy link

Hello,

Just another user affected by the 'zero' JaCoCo results with Powermock 😢
The top of this issue thread now says that the issue was fixed by another issue (#911 )

If so can somebody do the following:

  • Add an 'affirmative' message to the thread and close issue
  • Update the Powermock documentation here to reflect the updated state
  • Have a virtual cookie in appreciation of all the hard work that goes into supporting a great tool like Powermock (thanks!) 🍪

@Techtony96
Copy link

The PR #911 that says fixes this issue is unrelated and must be a typo in the commit message.

As a user eagerly awaiting online instrumentation I thank you for all the hard work towards this goal.

@fredwilliamson
Copy link

Hello ByteBuddy implementation will be in your next release ?
BR

@chengyinjie
Copy link

Really looking forward to the ByteBuddy release to fix the Jacoco problem.

@minhnguyenvan95
Copy link

Looking forward to ByteBuddy release

@OluwoleOyetoke
Copy link

OluwoleOyetoke commented Apr 30, 2020

Really looking forward to the ByteBuddy release

1 similar comment
@splf32
Copy link

splf32 commented May 27, 2020

Really looking forward to the ByteBuddy release

@SangHakLee
Copy link

Really looking forward to the ByteBuddy release.

I want to use JaCoCo and Powermock comfortably.

@jhyry-gcpud
Copy link

Been watching this thread for over a year.
Any progress?

@sarajcarbajal
Copy link

any progress with this issue?

@prudhvichitturi
Copy link

Could someone let us know if there is any progress on this issue ?

@kaneg
Copy link

kaneg commented Feb 24, 2021

Is there still any chance to fix the issue?

@chyun
Copy link

chyun commented Mar 31, 2021

According to #911 this problem already has been fixed. I tried PowerMock 2.0.9, I got the coverage.

@Vampire
Copy link
Contributor

Vampire commented Mar 31, 2021

And according to #727 (comment) that is just a typo

@sfc-gh-dschumann
Copy link

I can confirm that 2.0.9 still does not work with producing a test coverage with Jacoco. @chyun, maybe you used a different method of computing test coverage?

@wayqui
Copy link

wayqui commented Sep 15, 2022

It's almost 6 years since this bug was open. Any news on the use of ByteBuddy to instrument classes in Jacoco? Why does it say that is fixed with #911? Is that the case? what about #910 ? is that related to the issue with jacoco and powermock?

@AKON-SU
Copy link

AKON-SU commented Jun 29, 2023

Does anyone have a solution to this problem

@benken-parasoft
Copy link

Does anyone have a solution to this problem

I would recommend migrating your tests to use Mockito exclusively. One of the original motivations for PowerMock was static mocking and constructor mocking but Mockito has since caught up.

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

Successfully merging a pull request may close this issue.