Remove powermock

[timja]

Due to not respecting module boundaries powermock does not work out of the box in Java 16+. On Java 11+ it had a poor user experience with annotations, guess work and googling required to make it work at all.

Personally I block new powermock tests and if they break I normally remove them / replace them with something else, and I don't seem to be the only one, e.g. @MarkEWaite removed them from the git plugin, jenkinsci/git-plugin#1139 (comment).

@jglick suggested removing powermock from the plugin pom as an encouragement for maintainers when they upgrade to remove their tests or look for different options

Powermock has not had a merged PR in nearly 1 year at this point.

Here's a sample of some concerning issues:

powermock/powermock#992 (comment)
powermock/powermock#1053
powermock/powermock#969

Mailing list is very in-active:
https://groups.google.com/g/powermock

Options for people coming here:

1. Use a JenkinsRule or a RealJenkinsRule and TestExtension where necessary which will provide more realistic behaviour but slower tests
2. Use dependency injection with the facade pattern, example: https://github.com/jenkinsci/github-checks-plugin/search?q=facade cc @uhafner
3. Use mockito or mockito-inline (https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#39), mockito can now mock static methods and final classes and it is actively maintained. Example PRs: 💣 Remove powermock jenkins#5736, [JENKINS-64803] Fix tags retrieval with folder scoped credentials git-plugin#1139
4. Just remove the tests, often you have better things to do than fight with mocking frameworks that may or may not be adding a lot of value
5. Stay with powermock, you'll need to just add an explicit version yourself, in the future if you wish to test against java 16+ you can configure maven-surefire plugin to add the required add-opens, example PowerMockito 2 + Java 11 causes "An illegal reflective access operation has occurred" powermock/powermock#969 (comment)

Depending on what people think we could bump this to 5.x as well, maybe a time to get something like #133 revived (but with less compatibility and hackery)?

[jglick]

see #441 also

[jglick]

we could bump this to 5.x as well

If you like. I do not personally find the numbering all that helpful. Sometimes there are changes that will break some child POMs and you need to do a bit of work to upgrade; that is what release notes are for.

[jglick]

Ah yes this mess, best left in the rear-view mirror.

Do we care about adding a Mockito IT? To #441 I guess, so prove that the inline static stuff works?

[timja]

I don't think so =/.

Seems a bit weird to add something like that to me. Powermock was more of a special case even so I'm not sure having it here was the best, it wasn't a magic fix to read it from the parent pom and it worked across java versions, you needed the above mess all over the place.

[rsandell]

I am slightly skeptical to this. Yes maintaining PowerMock tests is a pita, but it is still less work for me as a maintainer to tweak the existing tests when core or whatever the test is mocking has changed that trying to rewrite them all into JenkinsRule tests. And if I where to rewrite them all then CI would fail because the unit/integration tests would take much more than an hour to run :)

And don't get me started on the enormous pita trying to upgrade the Mockito tests to the newer version of the framework 😓 If I ever get forced to do that upgrade I don't know what to do :/

[timja]

And don't get me started on the enormous pita trying to upgrade the Mockito tests to the newer version of the framework 😓 If I ever get forced to do that upgrade I don't know what to do :/

This doesn't force you to upgrade 😄, you'll just need to add an explicit dependency in your plugin.