[TEST] Move Jarhell check inside the test.security.manager guard

[s1monw]

This change moves the JarHell check under the test.security.manager guard
such that users that have to opt out of JarHell for testing can do so by disabling
security. It will all users to use our test framework but when doing so the parameter
-Dtests.security.manager=false will signal how serious it is to opt out of this check

Relates to #11932

[synhershko]

👍

[rmuir]

I'm -1 because here is what will happen. you commit this change, then lusers with jar hell will realize that they want to load plugins in their tests too, and will want more leniency.

the excuse will be used, that this change is "precedence" and then everything goes in the wrong direction. better to just stop it right here and now.

[s1monw]

I'm -1 because here is what will happen. you commit this change, then lusers with jar hell will realize that they want to load plugins in their tests too, and will want more leniency.

what does the user prevent from adding plugins here? I mean they can just do that without further checks and leniency nothing prevents them?

[s1monw]

@kimchy @clintongormley I am going to close this again since there seems no room for compromises here.

[synhershko]

I'm confused - didn't you just state the complete opposite here and in #11932 ?

[s1monw]

I'm confused - didn't you just state the complete opposite here and in #11932 ?

rob vetoed that change so we have to go back and re-iterate to see how we can improve the situation or find other compromises. Stay tuned @synhershko

[s1monw]

I had a longer conversation about this with @rmuir today and we have a couple action items for this.

• for 2.2 we can commit the patch as it is
• for 2.3 we will remove JAR hell entirely from the testing bootstrap and add an explicit check to the plugin pom.xml file that runs jar hell as part of the build process with the plugin classpath.
• for 3.0 we remove that check from the testing bootstrap since we already have the check we have to add for 2.3

@rmuir did I miss something?

[rmuir]

@s1monw yes that is fine, the only thing i see missing is that long term we address the issue of "client". In general our test-framework is not setup for that, and it is crucial to separate from the server code for many other reasons. Until that happens, the road will be rocky for someone trying to use this test-framework to test client code. This is just one of the first checks they will hit.

[s1monw]

@s1monw yes that is fine, the only thing i see missing is that long term we address the issue of "client". In general our test-framework is not setup for that, and it is crucial to separate from the server code for many other reasons. Until that happens, the road will be rocky for someone trying to use this test-framework to test client code. This is just one of the first checks they will hit.

agreed @rmuir I think ultimately we have to fix that with test-fixtures and a separate client

[s1monw]

I ported #16174 to 2.x such that folks can disable in 2.3 as well - We have a good build system fix in master so we don't need to waste time calling ant from maven