Community
Participate
Working Groups
See https://download.eclipse.org/eclipse/downloads/drops4/I20210413-1400/testresults/html/org.eclipse.e4.ui.tests_ep420I-unit-cen64-gtk3-java16_linux.gtk.x86_64_16.html Most likely side effect of bug 571866 changes. I assume some dependency is not properly resolved / specified now. Underlying exception : java.lang.IllegalStateException: Error invoking java.lang.invoke.MethodHandles$Lookup#defineClass at org.eclipse.e4.ui.tests.workbench.ModelAssemblerTests.setup(ModelAssemblerTests.java:95) Caused by: java.lang.NoClassDefFoundError: org/osgi/service/log/Logger at java.base/java.lang.ClassLoader.defineClass0(Native Method) at java.base/java.lang.System$2.defineClass(System.java:2193) at java.base/java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClass(MethodHandles.java:2446) at java.base/java.lang.invoke.MethodHandles$Lookup$ClassDefiner.defineClass(MethodHandles.java:2423) at java.base/java.lang.invoke.MethodHandles$Lookup.defineClass(MethodHandles.java:1850) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:567) at net.bytebuddy.dynamic.loading.ClassInjector$UsingLookup$Dispatcher$ForJava9CapableVm.defineClass(ClassInjector.java:1641) ... 92 more Caused by: java.lang.ClassNotFoundException: org.osgi.service.log.Logger cannot be found by org.mockito_2.23.0.v20200310-1642 at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:519) at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:171) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:519) ... 102 more
Any idea why org.osgi.service.log.Logger is not visible to mockito with Java 16? Is there any change in the classloading?
(In reply to Dirk Fauth from comment #1) > Any idea why org.osgi.service.log.Logger is not visible to mockito with Java > 16? Is there any change in the classloading? Which Mockito version are we using? IIRC Mockito provides a new version for Java 16 called mockito-inline <dependency> <groupId>org.mockito</groupId> <artifactId>mockito-inline</artifactId> <version>3.7.7</version> </dependency>
I just found this ticket: https://github.com/mockito/mockito/issues/2156
(In reply to Lars Vogel from comment #2) > Which Mockito version are we using? IIRC Mockito provides a new version for > Java 16 called mockito-inline I installed the I-build today with the tooling and can see mockito-core 2.23
(In reply to Dirk Fauth from comment #3) > I just found this ticket: > https://github.com/mockito/mockito/issues/2156 The byte buddy author answered that that bug should have been fixed in JDK, but we are running latest OpenJDK 64-Bit Server VM JVM version : 16+36-2231. See https://adoptopenjdk.net/?variant=openjdk16&jvmVariant=hotspot I've tried the test locally in the IDE, it works with exact that version and also one from Oracle. So that seem to be not JDK issue, but something in the way how tests are setup.
(In reply to Andrey Loskutov from comment #5) > (In reply to Dirk Fauth from comment #3) > > I just found this ticket: > > https://github.com/mockito/mockito/issues/2156 > > The byte buddy author answered that that bug should have been fixed in JDK, > but we are running latest OpenJDK 64-Bit Server VM JVM version : 16+36-2231. > > See https://adoptopenjdk.net/?variant=openjdk16&jvmVariant=hotspot > > I've tried the test locally in the IDE, it works with exact that version and > also one from Oracle. So that seem to be not JDK issue, but something in the > way how tests are setup. It is a plug-in test that mocks the org.osgi.service.log.Logger. And that call throws the ClassNotFoundException. Why is that an error on the test setup? Any idea I could look for?
(In reply to Dirk Fauth from comment #6) > > It is a plug-in test that mocks the org.osgi.service.log.Logger. And that > call throws the ClassNotFoundException. Why is that an error on the test > setup? Any idea I could look for? I haven't looked into the code, one possibility would be that due some JDK change class loading happens earlier than before or from a different class loader and for some reason at that point in time or from that class loader chain the class can't be resolved. Was the dependency to Logger in test always there, or was it added by the recent change?
(In reply to Andrey Loskutov from comment #7) > (In reply to Dirk Fauth from comment #6) > > > > It is a plug-in test that mocks the org.osgi.service.log.Logger. And that > > call throws the ClassNotFoundException. Why is that an error on the test > > setup? Any idea I could look for? > > I haven't looked into the code, one possibility would be that due some JDK > change class loading happens earlier than before or from a different class > loader and for some reason at that point in time or from that class loader > chain the class can't be resolved. Was the dependency to Logger in test > always there, or was it added by the recent change? It was changed from the platform logger to the OSGi logger as the ModelAssembler became a service.
Did anyone managed to test with latest mockito/byte-buddy and Java 16? Using mockito 2.23 when 3.9 is out sounds like badly outdated and very suspicious to be problem here to me.
Using m2e pde I injected latest byte-buddy, objenesis and mockito-core and tests succeed while they were failing prior to that. So mockito version we use is simply too ancient to support Java 16 and has to be updated.
Is anyone interested in dealing with the Orbit updates needed ? I've put it my queue but it's with 3digit number so will take a while until I get to it.
(In reply to Alexander Kurtakov from comment #11) > Is anyone interested in dealing with the Orbit updates needed ? I've put it > my queue but it's with 3digit number so will take a while until I get to it. Would it be possible to use the same approach as with jetty? Especially as this are test-dependecies as far as I understand?
(In reply to Christoph Laeubrich from comment #12) > (In reply to Alexander Kurtakov from comment #11) > > Is anyone interested in dealing with the Orbit updates needed ? I've put it > > my queue but it's with 3digit number so will take a while until I get to it. > > Would it be possible to use the same approach as with jetty? Especially as > this are test-dependecies as far as I understand? With Jetty we have the luxury to be sure that everything is already verified via CQ/Clearlydefined and that there is OSGi metadata in the bundles. As none of these are guaranteed with mockito it's best if it goes into Orbit.
Although the best middle ground would be reusing Jetty repo approach as part of Orbit build process.
I just downloaded the artifacts from Maven Central: - mockito-core 3.9.0 - objenesis 3.2 - byte-buddy 1.10.20 All jar files have a MANIFEST file with proper OSGi metadata. Shouldn't we be able to then directly consume those artifacts using the Maven location type?
(In reply to Dirk Fauth from comment #15) > I just downloaded the artifacts from Maven Central: > > - mockito-core 3.9.0 > - objenesis 3.2 > - byte-buddy 1.10.20 > > All jar files have a MANIFEST file with proper OSGi metadata. > > Shouldn't we be able to then directly consume those artifacts using the > Maven location type? That would have been too good to be true. Unfortunately we have to jarsign these artifacts. The topic has been discussed over and over again at cross-project and architecture council but until there is decision that there is no point in jarsign third party content we can not "just" use maven artifacts. Signing at the end of build (repository level) is a no-go as it will create new artifact (only diff being signature time) and thus flood the ecosystem with same but not exactly jars. If you feel like you want to reopen the discussion please do so at cross-project mailing list. It is never bad thing to discuss obvious pain points over and over again until some resolution is found.
(In reply to Alexander Kurtakov from comment #16) > That would have been too good to be true. Unfortunately we have to jarsign > these artifacts. While this is true for anything that gets published, test-dependecies should never be part of the published artifacts or do I misunderstand something?
(In reply to Christoph Laeubrich from comment #17) > (In reply to Alexander Kurtakov from comment #16) > > That would have been too good to be true. Unfortunately we have to jarsign > > these artifacts. > > While this is true for anything that gets published, test-dependecies should > never be part of the published artifacts or do I misunderstand something? All test bundles are published in the repository as they are used after that to run tests on various ws/os/arch combos skipping the build part and thus different qualifiers.
But if the artifacts in Maven Central already have proper OSGi meta-data. Would the re-publishing in Orbit not also create the same artifacts just with a different signature? Orbit exists to provide third-party artifacts via p2 update site (which was needed before the Maven location type) and to add OSGi meta-data. Wasn't there a point in the discussion that artifacts that do not need to be modified can be used directly? Not sure, just remember that Wayne posted something in that area. But can't remember the details.
(In reply to Dirk Fauth from comment #19) > But if the artifacts in Maven Central already have proper OSGi meta-data. > Would the re-publishing in Orbit not also create the same artifacts just > with a different signature? The artifact in Maven Central has no signature with jarsigner. It's pgp signed which is external for the artifact while jarsigner puts signature insight the artifact. > > Orbit exists to provide third-party artifacts via p2 update site (which was > needed before the Maven location type) and to add OSGi meta-data. > > Wasn't there a point in the discussion that artifacts that do not need to be > modified can be used directly? Not sure, just remember that Wayne posted > something in that area. But can't remember the details. I don't remember anything like this.
(In reply to Alexander Kurtakov from comment #18) > All test bundles are published in the repository as they are used after that > to run tests on various ws/os/arch combos skipping the build part and thus > different qualifiers. Test-bundles --> yes, but the dependencies don't need to be there... BTW: what about pom.dependencies=consider in this case then the artifact will never need to be deployed but is pulled from maven-central directly...
(In reply to Christoph Laeubrich from comment #21) > BTW: what about pom.dependencies=consider in this case then the artifact > will never need to be deployed but is pulled from maven-central directly... But we also need the Mockito dependencies in the target definition to be able to execute the tests locally. Don't we? Just noticed that such a change would be even more intrusive, as we still use Require-Bundle over Import-Package. And the symbolic name in Maven Central is definitely different to what we have in Orbit. So such a change, if in anyway accepted would mean to adjust all test bundles IIUC.
(In reply to Dirk Fauth from comment #22) > (In reply to Christoph Laeubrich from comment #21) > > BTW: what about pom.dependencies=consider in this case then the artifact > > will never need to be deployed but is pulled from maven-central directly... > > But we also need the Mockito dependencies in the target definition to be > able to execute the tests locally. Don't we? Correct we can not execute tests without mockito/butebuddy/junit/etc. being in the repository and thus installed as a dependency when installing the thest bundle. > > Just noticed that such a change would be even more intrusive, as we still > use Require-Bundle over Import-Package. And the symbolic name in Maven > Central is definitely different to what we have in Orbit. So such a change, > if in anyway accepted would mean to adjust all test bundles IIUC. That is probably a good change to do here and now as a first step towards resolving this issue.
(In reply to Dirk Fauth from comment #22) > (In reply to Christoph Laeubrich from comment #21) > > BTW: what about pom.dependencies=consider in this case then the artifact > > will never need to be deployed but is pulled from maven-central directly... > > But we also need the Mockito dependencies in the target definition to be > able to execute the tests locally. Don't we? Maybe, I recently trying to lift that restriction but there was not much support/enthusiasm so I dropped this initiative... > Just noticed that such a change would be even more intrusive, as we still > use Require-Bundle over Import-Package. And the symbolic name in Maven > Central is definitely different to what we have in Orbit. So such a change, > if in anyway accepted would mean to adjust all test bundles IIUC. What about simply drop org.osgi.service.log.Logger and use Platform.getLog() ... not nice but given the impact this might be a low hanging fruit then to solve the issue for now.
Not updating dependencies is a bandaid solution and it always come back later biting ever more when you don't have a choice. I still have bad memories (and years have passed) from the migration from Lucene 3 to 6 which was not updated for years.
(In reply to Christoph Laeubrich from comment #24) > What about simply drop org.osgi.service.log.Logger and use Platform.getLog() > ... not nice but given the impact this might be a low hanging fruit then to > solve the issue for now. Well, it would be kind of the same from the runtime perspective, as this way you get the same Logger IIRC. Just by using a static way to resolve. But it does not solve the tests, as you then can't mock and therefore not verify that no log entries are written. IMHO the tests are kind of strange as they verify if something was logged. Not a good practice. But not sure how the tests could be changed to not do the verification based on the log. @Alexander Not sure how to proceed now. Either - discuss if we are allowed to consume directly from Maven Central, which would imply that we have to update all test bundles that use Mockito. - Add updated Mockito, Objenesis, Bytebuddy to Orbit and consume it from there. I see that all three have already EBR recipes, so adding new versions shouldn't be too difficult. Do we need CQs for the new versions?
(In reply to Alexander Kurtakov from comment #25) > Not updating dependencies is a bandaid solution and it always come back > later biting ever more when you don't have a choice. Obviously this has not biting hard enough if there is still no easy path to upgrade third party dependencies ... We have the tools to do it but not allowed to use them so if no one else cares I would simply go the easy path here, have wasted just too much time in improving here... (In reply to Dirk Fauth from comment #26) > Well, it would be kind of the same from the runtime perspective, as this way > you get the same Logger IIRC. Just by using a static way to resolve. But it > does not solve the tests, as you then can't mock and therefore not verify > that no log entries are written. I recently had the same problem and solved this with a loglistener, even though I can't found my change for example org.eclipse.ui.tests.menus.Bug410426Test.testNoClassCastExceptionForMenuManagerToolbarContribution() uses this approach and if you search for Loglistener there are some more.
(In reply to Dirk Fauth from comment #26) > @Alexander > Not sure how to proceed now. > > Either Do both IMHO. We need these updates to become easier and faster in the long term and we need fixing tests soon. > > - discuss if we are allowed to consume directly from Maven Central, which > would imply that we have to update all test bundles that use Mockito. > > - Add updated Mockito, Objenesis, Bytebuddy to Orbit and consume it from > there. > > I see that all three have already EBR recipes, so adding new versions > shouldn't be too difficult. Do we need CQs for the new versions? (In reply to Christoph Laeubrich from comment #27) > (In reply to Alexander Kurtakov from comment #25) > > Not updating dependencies is a bandaid solution and it always come back > > later biting ever more when you don't have a choice. > > Obviously this has not biting hard enough if there is still no easy path to > upgrade third party dependencies ... We have the tools to do it but not > allowed to use them so if no one else cares I would simply go the easy path > here, have wasted just too much time in improving here... Creating the tools is 1/3 of the work in the most difficult tasks in the rest way less compared to the time needed to change mindsets, legal requirements, processes and etc. I don't want it to be that way but unfortunately such is the world we live in. > > (In reply to Dirk Fauth from comment #26) > > Well, it would be kind of the same from the runtime perspective, as this way > > you get the same Logger IIRC. Just by using a static way to resolve. But it > > does not solve the tests, as you then can't mock and therefore not verify > > that no log entries are written. > > I recently had the same problem and solved this with a loglistener, even > though I can't found my change for example > > org.eclipse.ui.tests.menus.Bug410426Test. > testNoClassCastExceptionForMenuManagerToolbarContribution() > > uses this approach and if you search for Loglistener there are some more.
Fixed via https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/179582 and looks good in https://download.eclipse.org/eclipse/downloads/drops4/I20210421-1800/testresults/html/org.eclipse.e4.ui.tests_ep420I-unit-cen64-gtk3-java16_linux.gtk.x86_64_16.html Thanks all!
