Open chiquitinxx opened 3 years ago
It seems that you are the first module user ;)
We also need to document that due to groovy2-compat
modules work only with groovy-3.0.
I've been attempting to upgrade a project from java 8 to 11 and hit a similar IllegalAccessError in 2.0-M4-groovy-3.0.
In my scenario it was from SpecInfoBuilder's use of setAccessible(true)
on the methods in the specification classes, but I was able to bypass it with some utilities I already wrote that can crack open the Module security system at runtime.
But even after getting past this issue, I then encountered an issue of what seems to be groovy's invocation system failing to function as intended. any functionality in a Specification not marked as @CompileStatic
would fail at the first line of code, either with a missing property or a missing method exception out of Groovy's MetaClass system.
This basically causes most of the usability of Spockframework + Groovy to be lost. 😢
I recently talked with @aalmiray about groovy and modules, and he mentioned that groovy 2 and 3 are not really module system compliant https://groovy-lang.org/releasenotes/groovy-3.0.html#Groovy3.0releasenotes-Splitpackages so I don't think that we can do much here at the moment (thoughts @paulk-asert ?). I couldn't really figure out how to create a module descriptor in gradle for groovy code, so that I could actually recreate this properly. Maybe you can provide an example gradle project for this usecase.
My suggestion would be for the time being to use the classpath mode for testing.
Thanks for the feedback. I did look into the release notes of groovy 3.0.x and did see that it didn't really modularize, but moreso "prepare for modularization" So this likely lands as a requirement on groovy 4.x, which is currently in alpha. I've not seen any mention of a timeline on 4.x, but it likely won't be GA anytime soon.
But even putting groovy to the side, spockframework still has some areas that look to need alteration 1) How it wires methods in as private and then sets them as accessible at runtime. 2) ExtensionClassesLoader manually scans the classpath for SPI registration files and manually loads them, instead of using ServiceLoader. SPI registration files are gone in module land as these are part of the module descriptor/definition instead.
There may be other areas as well, these are just some of the ones I came across in trying to accomplish this upgrade in a couple days' time.
While classpath mode may be a workaround to get the tests to work, such tests lose some relevance.
This may come down to where spockframework will have to be temporarily dropped as a utilized test framework until it and groovy are module compliant. Rather inconvenient, but it doesn't seem avoidable at this time.
Agreed. Work on ClassLoaders can begin at anytime before a fully JPMS compatible version of Groovy becomes available.
Hm, switching over to ServiceLoader SPI makes the newly proposed constructor injection for extensions moot #1271 as you'll have to have a no-args constructor. Is there a recommended way to do DI into services? It also doesn't really support the ConfigObject
approach since we also have to look them up via file and instantiate them, and they are not services.
If you want to have DI on the loaded services, then as noticed already ServiceLoader alone will not suffice.
one option is to have a derivative interface (or interfaces?) of the IGlobalExtension that has setX
method (or methods) on it to accept the desired data after construction. (This would be similar to how Hibernate does things, where it does so to avoid having a "full" DI system)
If injection needs to be done at construction, then this starts to behave a bit more like a "full" DI system (such as some jakarta.inject implementation).
If I'm reading spockframework's current behavior correctly, it loads the ConfigurationObject similar to an SPI, but the registered/loaded classes do not actually implement ConfigurationObject
, but instead are annotated with it.
I believe this is what is referred to with the 'they are not services' phrase, in that it is not compliant with intended ServiceLoader behavior.
Even though ServiceLoader does accept the use of a public static provider
method on the registered class (only within modules though), I'm not sure this is much different in the end compared to normal construction, as the returned value of the provider
method would still have to implement ConfigurationObject
if left as-is
So there might need to be some new SPI interface that provides the class or classes annotated with ConfigurationObject instead if ServiceLoader is still the chosen path.
Btw, we still have the need to scan the classpath to discover tests, I don't think it would make sense to treat tests as services and use the ServiceLoader for them.
Tests are not services, and they can not be treated as services. Doing so would break most of the conventions around being able to select which test or tests to execute.
I've not dived that far into it as of yet, but I suspect that the junit-platform components are doing a lot of the processing for test scanning anyway.
From what I found in SpockEngine
, it seems to additionally refine the scanned test classes to only process on classes that pass the SpecUtil.isRunnableSpec
filter. ClassSelectorResolver
is similar in utilizing SpecUtil.isRunnableSpec
My point was more about the need to still have classpath scanning to discover tests, and also the ServiceLoader only works for global extensions, annotation driven extension will still have to be instantiated via reflection. I'm not against it, just listing potential issues.
Hmm, this is probably due to being able to take away different interpretations.
I do not recall saying something such as the lines of "reflection must not be used". Even in module mode reflection can be used. It is just that reflection must be used more carefully, because new restrictions are enacted on reflection that must be considered when utilizing it. So far, the biggest restriction I've encountered is around use of setAccessible(true)
. this can only be done by modules that have open
rights on the package the class is contained within.
Also, the prior discussion around ServiceLoader
is not particularly around the reflection aspect, but more of the point that SPI registration itself changes in module mode to be from the module descriptor, and not from the META-INF/service
registration files.
Not utilizing ServiceLoader
can lead to bug reports from users who are defining IGlobalExtension
s in their module definitions and Spock does not utilize them as expected.
For ConfigurationObject
, it's not really a service, so the current paradigm could still work in module mode. I've not tested this to verify that it does not work.
At first it may just look a bit inconsistent/confusing for IGlobalExtension
to use ServiceLoader
and ConfigurationObject
to not. But documenting this difference in behavior, and the reasoning behind it, should likely suffice.
Trying to run Spock 2 to test some java module, using Java 15, I get this error. I think the exception in clear, using a class that junit is not allowing to use outside junit. Let me know if you need a full maven project to reproduce.
Using a module-info like this in src/test/groovy:
Additional Environment information
Java/JDK
Build tool version
Apache Maven
Operating System
Build-tool dependencies used
Apache Maven