wheelerlaw commented 2 years ago

Note: The branch for this PR is rebased on top of the branch for the #110 because of the importance of fixing the master build process.

Summary

There are a few changes in this PR:

Jenkins allows shared libraries to be written as a script, in addition to fully qualified class. The superclass of a script is groovy.lang.Script, whereas the superclass of a regular class is whatever the class extends (or java.lang.Object if it doesn't extend a class).

The LocalProjectPipelineExtensionDetector class scans for classes in the project to instrument in the getClassesOfTypeInPackage method by using the Reflections package. It scans for classes that extend Object. Reflections, however, does not supporting scanning for classes that extend a class but are not direct subtypes of the same class. So the getClassesOfTypeInPackage method misses all Jenkins scripts that are a part of the shared library.

An easy solution to this would be to also scan for classes that extend groovy.lang.Script. However, it is also perfectly valid for Jenkins shared libraries classes to extend other Jenkins shared library classes. The solution here is to use a class scanner that supports searching for classes that are descendants of a class.
According to this comment, a ScanResult should be assigned within a try-with-resources, so I have made that change in the Local detector and the WholeClasspath detector.
I found another Java 11+ compatibility issue (not catching NoClassDefFoundError exception) and fixed it.

Checklist

Testing

(Remove this checklist and replace it with "N/A - no code changes" if this PR does not modify source code)

[x] I have manually verified that my code changes do the right thing.
[x] I have run the tests and verified that my changes do not introduce any regressions.
[x] I have written unit tests to verify that my code changes do the right thing and to protect my code against regressions

Documentation

(Remove this checklist and replace it with "N/A - no code changes" if this PR does not modify source code)

[x] I have updated the "Unreleased" section of CHANGELOG.md with a brief description of my changes.
[x] I have updated code comments - both GroovyDoc/JavaDoc-style comments and inline comments - where appropriate.
[x] I have read CONTRIBUTING.md and have followed its guidance.

kriegaex commented 2 years ago

I am not sure if this related, but after adding some dependencies to one of my projects, an old test using jenkins-spock is no longer working, complaining about missing OSGi and JNA classes while using the org.reflections tool. I have no idea why this is happening.

16:40:06.560 [main] INFO  org.reflections.Reflections - Reflections took 996 ms to scan 10 urls, producing 1502 keys and 8401 values 
16:40:06.568 [main] WARN  org.reflections.Reflections - could not get type for name org.osgi.framework.BundleActivator from any class loader
org.reflections.ReflectionsException: could not get type for name org.osgi.framework.BundleActivator
    at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:312)
    at org.reflections.Reflections.expandSuperTypes(Reflections.java:382)
    at org.reflections.Reflections.<init>(Reflections.java:140)
    at com.homeaway.devtools.jenkins.testing.LocalProjectPipelineExtensionDetector.getClassesOfTypeInPackage(LocalProjectPipelineExtensionDetector.java:61)
    at com.homeaway.devtools.jenkins.testing.LocalProjectPipelineExtensionDetector$getClassesOfTypeInPackage.call(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:115)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:135)
    at com.homeaway.devtools.jenkins.testing.JenkinsPipelineSpecification.setupSpec(JenkinsPipelineSpecification.groovy:1122)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:564)
    at org.spockframework.util.ReflectionUtil.invokeMethod(ReflectionUtil.java:200)
    at org.spockframework.runtime.model.MethodInfo.invoke(MethodInfo.java:113)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.doRunSetupSpec(BaseSpecRunner.java:177)
    at org.spockframework.runtime.BaseSpecRunner$3.invoke(BaseSpecRunner.java:161)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:154)
    at org.spockframework.runtime.BaseSpecRunner.doRunSetupSpec(BaseSpecRunner.java:174)
    at org.spockframework.runtime.BaseSpecRunner$3.invoke(BaseSpecRunner.java:161)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:154)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:149)
    at org.spockframework.runtime.BaseSpecRunner.doRunSpec(BaseSpecRunner.java:94)
    at org.spockframework.runtime.BaseSpecRunner$1.invoke(BaseSpecRunner.java:81)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSpec(BaseSpecRunner.java:73)
    at org.spockframework.runtime.BaseSpecRunner.run(BaseSpecRunner.java:64)
    at org.spockframework.runtime.Sputnik.run(Sputnik.java:63)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.lang.ClassNotFoundException: org.osgi.framework.BundleActivator
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:606)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:168)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:310)
    ... 39 common frames omitted
16:40:06.581 [main] WARN  org.reflections.Reflections - could not get type for name org.osgi.framework.SynchronousBundleListener from any class loader
org.reflections.ReflectionsException: could not get type for name org.osgi.framework.SynchronousBundleListener
    at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:312)
    at org.reflections.Reflections.expandSuperTypes(Reflections.java:382)
    at org.reflections.Reflections.<init>(Reflections.java:140)
    at com.homeaway.devtools.jenkins.testing.LocalProjectPipelineExtensionDetector.getClassesOfTypeInPackage(LocalProjectPipelineExtensionDetector.java:61)
    at com.homeaway.devtools.jenkins.testing.LocalProjectPipelineExtensionDetector$getClassesOfTypeInPackage.call(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:47)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:115)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:135)
    at com.homeaway.devtools.jenkins.testing.JenkinsPipelineSpecification.setupSpec(JenkinsPipelineSpecification.groovy:1122)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:564)
    at org.spockframework.util.ReflectionUtil.invokeMethod(ReflectionUtil.java:200)
    at org.spockframework.runtime.model.MethodInfo.invoke(MethodInfo.java:113)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.doRunSetupSpec(BaseSpecRunner.java:177)
    at org.spockframework.runtime.BaseSpecRunner$3.invoke(BaseSpecRunner.java:161)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:154)
    at org.spockframework.runtime.BaseSpecRunner.doRunSetupSpec(BaseSpecRunner.java:174)
    at org.spockframework.runtime.BaseSpecRunner$3.invoke(BaseSpecRunner.java:161)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:154)
    at org.spockframework.runtime.BaseSpecRunner.runSetupSpec(BaseSpecRunner.java:149)
    at org.spockframework.runtime.BaseSpecRunner.doRunSpec(BaseSpecRunner.java:94)
    at org.spockframework.runtime.BaseSpecRunner$1.invoke(BaseSpecRunner.java:81)
    at org.spockframework.runtime.BaseSpecRunner.invokeRaw(BaseSpecRunner.java:484)
    at org.spockframework.runtime.BaseSpecRunner.invoke(BaseSpecRunner.java:467)
    at org.spockframework.runtime.BaseSpecRunner.runSpec(BaseSpecRunner.java:73)
    at org.spockframework.runtime.BaseSpecRunner.run(BaseSpecRunner.java:64)
    at org.spockframework.runtime.Sputnik.run(Sputnik.java:63)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:69)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:235)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:54)
Caused by: java.lang.ClassNotFoundException: org.osgi.framework.SynchronousBundleListener
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:606)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:168)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    at org.reflections.ReflectionUtils.forName(ReflectionUtils.java:310)
    ... 39 common frames omitted
ERROR StatusLogger Log4j2 could not find a logging implementation. Please add log4j-core to the classpath. Using SimpleLogger to log to the console...

Test ignored.

Test ignored.

java.lang.NoClassDefFoundError: com/sun/jna/Library

    at java.base/java.lang.ClassLoader.findBootstrapClassOrNull(ClassLoader.java:1267)
    at java.base/java.lang.System$2.findBootstrapClassOrNull(System.java:2196)
    at java.base/jdk.internal.loader.ClassLoaders$BootClassLoader.loadClassOrNull(ClassLoaders.java:118)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:665)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:641)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:665)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:641)
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:604)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:168)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    at java.base/java.lang.Class.forName(Class.java:377)
    at com.homeaway.devtools.jenkins.testing.LocalProjectPipelineExtensionDetector.getClassesOfTypeInPackage(LocalProjectPipelineExtensionDetector.java:88)
    at com.homeaway.devtools.jenkins.testing.JenkinsPipelineSpecification.setupSpec(JenkinsPipelineSpecification.groovy:1122)

Would this PR fix a problem like this?

kriegaex commented 2 years ago

Would this PR fix a problem like this?

Answering my own question: Yes, this PR fixes my problem. I just cloned the fork and built this PR's branch. I think it would make a lot of sense for the maintainer to actually review and, if it proves to be valid, merge this PR.

kriegaex commented 2 years ago

Why is nobody fixing this problem after such a long time? Locally, I am working with a snapshot version containing this fix, but my CI builds, e.g. on GitHub, cannot use that snapshot, because it is not deployed in any publicly available repository. So I have a choice to either skip my Jenkins Spock tests or let them fail with the error I posted above. It would be nice to get a maintenance release after almost 4 months.

With regard to code review feedback: Why not just be pragmatic, merge locally and fix what you think ought to be fixed, like a normal maintainer would, taking repsonsibility for what is merged into his own repository? You do not need to wait for the PR creator to commit your suggestions.

ExpediaGroup / jenkins-spock

Fix local class scanning #111

Summary

Checklist

Testing

Documentation