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

Remove powermock #6439

Closed
rnveach opened this Issue Feb 19, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@rnveach
Copy link
Member

rnveach commented Feb 19, 2019

Drop powermock completely and use a suppression list similar to pitest to manage uncovered code coverage or rewrite production code so just reflection can be used without mocking.
If that isn't acceptable, the next best thing is to completely isolate powermock tests to their own class(es) but this probably won't solve the main problem.

Reasons supporting removal of powermock:
#6438 (comment)
#6435 (comment)
#6424 (comment)
hcoles/pitest#230 (comment)
#5678 (comment)

@romani

This comment has been minimized.

Copy link
Member

romani commented Feb 20, 2019

I am not sure it is possible as powermock was used mock static methods that are out of our code .

But I am agree to separate them in special Test classes or ..... To avoid conflicts between powermock and pitest.

@romani romani added the approved label Feb 20, 2019

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Feb 20, 2019

I am not sure it is possible

Like I stated above, I am thinking we need to forgo the 100% code coverage and just remove them completely.

powermock was used mock static methods

I looked at some for another issue, and the ones I was seeing was to clear hard to hit catch exceptions, code in the middle of a method, and tests that would require us to have classes in weird packages. You can still call static/private methods with reflection.

avoid conflicts between powermock and pitest

Two of my examples was conflict between normal tests and powermock too. So it is not just pitest. No one wants to work with powermock.
I will split powermock tests to their own section and we can see how that goes for now.

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Feb 20, 2019

ImportControlLoader is package private which prevents moving power tests to a separate package.
PropertyCacheFile is too and also has a package private constructor.

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Feb 23, 2019

Fix was merged. I will continue to change powermock tests to normal tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.