Add Groovy 6 Alpha 2 as a testing variant#2356
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Groovy 6.0 alpha variant support: version catalog and gradle property, build.gradle variant configuration enforcing Java 17, CI matrix infrastructure to include additional variants, workflow matrix updates, invocation script updates, Codecov comment tuning, compiler bytecode expression and DeepBlockRewriter changes, and a test guard for Groovy 6. ChangesGroovy 6.0 Variant Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds Groovy 6.0.0-alpha-1 to the CI matrix via a new
Confidence Score: 3/5The publish pipeline will be blocked on every master push and release tag until the Groovy 6 compile issue is resolved, because the alpha variant now sits in the build-and-verify matrix that release-spock depends on. The release.main.kts script passes Matrix.full — which now includes additionalVariants — to the build-and-verify job. The PR explicitly notes that building with -Dvariant=6.0 currently fails. GitHub Actions skips any job whose needs dependency reports failure, so release-spock will never run while Groovy 6.0 is broken. All other changes (Gradle config, Renovate simplification, library alias, codecov count) are straightforward and correct. release.main.kts and its generated release.yaml — the build-and-verify matrix inclusion of additionalVariants is the root cause; the other files are fine. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
GP["gradle.properties\nvariantsList: 2.5, 3.0, 4.0, 5.0\nadditionalVariantsList: 6.0"]
GP --> |axes.variants| RS_MATRIX["release-spock matrix\n2.5/3.0/4.0 (Java 8)\n5.0 (Java 11)\n NO 6.0"]
GP --> |axes.variants + axes.additionalVariants| BV_MATRIX["build-and-verify matrix\n2.5/3.0/4.0/5.0/6.0\n 6.0 build FAILS"]
BV_MATRIX --> |Matrix.full| BAV["build-and-verify job\n(branches-and-prs + release)"]
RS_MATRIX --> RS["release-spock job"]
BAV --> |needs: must fully succeed| RS
BAV --> |6.0 fails: overall FAILURE| BLOCKED["release-spock SKIPPED\nAll releases blocked"]
Reviews (1): Last reviewed commit: "Add Groovy 6 Alpha 1 as a testing varian..." | Re-trigger Greptile |
| java: '8' | ||
| os: 'ubuntu-latest' | ||
| - variant: '6.0' | ||
| java: '11' |
There was a problem hiding this comment.
Are the java versions from the different os types intentional?
Why Linux 8 and 11 and Windows + Mac 17?
There was a problem hiding this comment.
Groovy 6 requires Java 17, so those Linux builds get excluded. And win+mac are include, i.e., adding executions.
leonard84
force-pushed
the
feat/groovy-6-alpha-testing
branch
3 times, most recently
from
bb3fe0f to
48e4202
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
build-logic/base/src/main/groovy/org/spockframework/gradle/SpockBasePlugin.groovy (1)
42-43: ⚡ Quick winConsider documenting the compiler version choice.
The relationship between
COMPILER_VERSION = 17andCOMPILER_RELEASE_VERSION = 8may not be immediately obvious to future maintainers. Adding a brief comment would clarify that Java 17 is needed to read dependency class files (Groovy 6) while maintaining Java 8 bytecode output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@build-logic/base/src/main/groovy/org/spockframework/gradle/SpockBasePlugin.groovy` around lines 42 - 43, Add a brief inline comment above the COMPILER_VERSION and COMPILER_RELEASE_VERSION constants in SpockBasePlugin.groovy explaining that JavaLanguageVersion.of(17) is required to read/dependency-compile against newer class files (e.g., Groovy 6), while COMPILER_RELEASE_VERSION = 8 is intentionally set to produce Java 8 bytecode for runtime compatibility; place the comment immediately above the two constants so future maintainers see the rationale when editing COMPILER_VERSION or COMPILER_RELEASE_VERSION.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@build-logic/base/src/main/groovy/org/spockframework/gradle/SpockBasePlugin.groovy`:
- Around line 42-43: Add a brief inline comment above the COMPILER_VERSION and
COMPILER_RELEASE_VERSION constants in SpockBasePlugin.groovy explaining that
JavaLanguageVersion.of(17) is required to read/dependency-compile against newer
class files (e.g., Groovy 6), while COMPILER_RELEASE_VERSION = 8 is
intentionally set to produce Java 8 bytecode for runtime compatibility; place
the comment immediately above the two constants so future maintainers see the
rationale when editing COMPILER_VERSION or COMPILER_RELEASE_VERSION.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd961dc7-bc99-4779-87c9-30308ef579ff
📒 Files selected for processing (1)
build-logic/base/src/main/groovy/org/spockframework/gradle/SpockBasePlugin.groovy
Groovy 6 test failures — root cause analysisThe 13 failing Cluster A+C — the
|
| Condition | find() now returns |
Resulting (broken) call |
|---|---|---|
with([1,2,3]){ contains(n) } |
list.find() → 1 |
(1).contains(1) → MissingMethod on Integer |
with(map){ containsKey(k) } |
map.find() → first Map.Entry |
entry.containsKey(...) → MissingMethod |
with(person){ checkCondition() } |
person.find() → person |
spec method on person → "object is not an instance of declaring class" |
verifyEach(1..10){ checks(it) } |
each item Integer.find() → the item |
Integer.checks(...) → all 10 items fail (vs. the snapshot's intended 2) |
The compiled closure's target operand is an invokedynamic "find" (THIS_CALL) on the closure, and the bytecode is byte-identical between Groovy 5 and Groovy 6 — confirming this is purely a runtime resolution change. It is the same area as GROOVY-11858 ("defer non-closure method for in-closure implicit-this call"), whose own test 'a b c'.with { split() } shows Groovy now intends delegate methods to win. So this looks like intended Groovy behavior, not a regression — the hack is fundamentally incompatible with Groovy 6 and Spock needs a delegate-independent way to obtain the current closure.
Cluster B — lost GroovyObject.invokeMethod fallback (1 test)
MockPrimitiveTypeResponsesSpec — "TypeArgumentConstraint with different primitive types #1974".
The mock is a proxy subclass whose metaclass theClass is the mocked supertype (ObjClass). When test(123.0d, false) doesn't match test(int, int), Groovy 5 fell back to GroovyObject.invokeMethod (Spock's interceptor → returns null). Groovy 6's reworked MetaClassImpl.invokeMethod throws MissingMethodException instead, and the invokedynamic recovery guard receiver.getClass() == e.getType() can never match, because the thrown type is the supertype, not the proxy class.
This is not a primitive-coercion change — (Double, Boolean) → int fails in both Groovy 5 and 6 for a plain class; the difference is specifically the lost GroovyObject fallback. It appears to be fallout of the MOP rework (GROOVY-11823 family) and may be a Groovy regression worth reporting upstream. A Spock-side fix would live in the mock factory (e.g. make the mock's metaclass theClass the proxy class, or make proxies GroovyInterceptable).
Cluster D — dropped suppressed exceptions (2 tests)
GroovySpiesThatAreGlobal — non-existing property (getter + setter).
GROOVY-11823 redesigned the inner-class propertyMissing / methodMissing protocol. A missing static property now surfaces as a fresh MissingPropertyExceptionNoStack, discarding the suppressed MissingMethodException that Spock attached. The test's own assertion ex.getSuppressed()[0].cause then indexes an empty array → ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0.
The primary expectation (a MissingPropertyException is thrown) still holds — only the suppressed-chain assertion breaks. This reads as a test over-fitted to Groovy 5 behavior; the likely fix is to version-guard those two assertions (this file already uses GroovyRuntimeUtil.MAJOR_VERSION branches elsewhere).
Summary
| Cluster | Tests | Cause | Likely fix |
|---|---|---|---|
| A+C | 10 | this.find() closure-reference hack breaks; in-closure implicit-this DGM calls now resolve to the delegate |
Delegate-independent way to reference the current closure (design change; possibly an upstream discussion) |
| B | 1 | Lost GroovyObject.invokeMethod fallback for mock proxies whose metaclass theClass is the mocked supertype |
Mock factory metaclass change; possibly report upstream |
| D | 2 | propertyMissing redesign drops suppressed exceptions; test assertion over-fitted |
Version-guard the suppressed-exception assertions |
The most substantial item is A+C — finding a Groovy-6-robust replacement for the current-closure hack. B and D are more contained.
Cluster A resolved:
|
Cluster B confirmed as a Groovy regression: GROOVY-12046
Cluster B ( Refined root cause. The original note framed it as "Groovy 5 falls back inside
Verified: Groovy 5.0.6 ✅ pass · 6.0.0-alpha-1 ❌ · 6.0.0-SNAPSHOT (master) ❌ — master does not fix it. Spock-free reproducer (pure Groovy; isolates the exception-type flip): import groovy.lang.*
class Outer {
static class ObjClass { String test(int a, int b) { "real:${a + b}" } }
static class Proxy extends ObjClass { // runtime subclass = the "proxy"
private final MetaClass mc
Proxy() { mc = new MetaClassImpl(GroovySystem.metaClassRegistry, ObjClass); mc.initialize() }
@Override MetaClass getMetaClass() { mc } // theClass == ObjClass (the SUPERTYPE)
@Override Object invokeMethod(String name, Object args) { "FALLBACK(${name})" }
}
}
Outer.ObjClass client = new Outer.Proxy()
def args = [123d, false] as Object[] // (Double, Boolean): matches no signature
try {
client.getMetaClass().invokeMethod(Outer.ObjClass, client, "test", args, false, false)
} catch (MissingMethodException e) {
println "${GroovySystem.version}: MissingMethodException(type=${e.type.simpleName})"
}A second, end-to-end reproducer (a ByteBuddy mock proxy, no Spock) reproduces the thrown Spock side. Pending the upstream fix, options are unchanged: version-guard the assertion for Groovy 6, or make the mock's metaclass |
|
I've broken out the closure reference fix into a standalone PR, as it is valid on its own. |
|
Maybe there is another issue, which was mentioned by #2363, or it could be the same as the open one GROOVY-12046. 2 GroovySpiesThatAreGlobal specs fail under Groovy 6: accessing a non-existing |
Introduces Groovy 6.0.0-alpha-1 to the CI matrix for early compatibility testing, without including it in published releases. Adds `additionalVariantsList` to gradle.properties (analogous to `additionalJavaTestVersionsList`) as a general mechanism for declaring testing/preview variants. Variants listed there are included in the build-and-verify CI matrix but never passed to release-spock, so they cannot appear in any release without any conditional workflow logic. - gradle.properties: add `additionalVariantsList=6.0` - libs.versions.toml: add groovy6 version pin and library alias for Renovate - build.gradle: add variant==6.0 config block (org.apache.groovy, Java 17+); update ghActionsPublish Java guard to accept Java 17 for variant 6.0 - common.main.kts: expand Matrix.full to include additionalVariants; add Java 17 minimum exclusion for 6.0; rewrite exclude lambda with `when`; update Windows/macOS includes to use Java 17 for 6.0 - release.main.kts: release-spock uses only axes.variants — no changes needed since additional variants are simply absent from the publish matrix
Groovy 6 requires Java 17+. Following the same pattern introduced for Groovy 5, build.gradle silently defaults to Java 17 when none was specified. An explicit but incompatible javaVersion still fails loudly. allVariants and allVariants.bat now include 6.0 in the loop, so e.g. `allVariants -DjavaVersion=21` works correctly for all variants.
Groovy 6 ships Java 17 class files (bytecode version 61.0), which JDK 11 cannot read during compilation. Bumping COMPILER_VERSION to 17 allows all variants to compile against Groovy 6. COMPILER_RELEASE_VERSION stays at 8, so output bytecode remains Java 8 compatible.
For implicit-this method conditions inside with/verifyAll/verifyEach closures, SpockRuntime.verifyMethodCondition must be handed the closure itself as the target so resolution routes through the closure's delegate (with fallback to the owner/spec). Groovy has no source-level keyword for "the closure I am currently in", so Spock fabricated one via this.find() (originally this.each(Closure.IDENTITY)), relying on a no-arg DGM resolving against the closure object. Groovy 6 (GROOVY-11858) resolves in-closure implicit-this calls against the delegate first, so this.find() became delegate.find() and broke with/verifyAll/verifyEach method conditions. A closure's doCall method is always an instance method of the generated Closure subclass, so local variable 0 is the closure instance. The new CurrentClosureExpression emits a single ALOAD 0, referencing the current closure directly without relying on the meta-object protocol at all. The groovyjarjarasm.asm relocation is stable across all supported Groovy versions (2.5-6.0), so this compiles everywhere. "with works with void methods" still fails on Groovy 6, but that is an unrelated upstream regression (GROOVY-12045): a static-nested-class delegate breaks resolution of an owner instance method. It is gated with @IgnoreIf(MAJOR_VERSION >= 6) until Groovy fixes it.
…lback Groovy 6 (GROOVY-12046) repaired the GroovyObject.invokeMethod MOP fallback for receivers whose metaclass theClass is a supertype of the runtime class - exactly the shape of a (ByteBuddy) mock proxy. Calling a method that does not exist on the mocked type is now dispatched through the proxy's GroovyObject.invokeMethod instead of throwing at the call site as it did on Groovy <= 5. JavaMockInterceptor handled getProperty/setProperty/getMetaClass for GroovyObject targets but not invokeMethod, so such calls were silently recorded as 'invokeMethod' interactions instead of failing. Restore the 'dynamic methods are considered to not exist' behavior: when invokeMethod is the MOP fallback for a method the mocked type does not respond to, throw MissingMethodException. Existing methods (e.g. an argument-type mismatch on an overload) are left to normal mock dispatch.
Bump the groovy6 variant from 6.0.0-alpha-1 to 6.0.0-alpha-2 and adapt the
affected GroovySpiesThatAreGlobal specs.
State of the three previously-failing Groovy 6 specs:
- JavaMocksForGroovyClasses "dynamic methods are considered to not exist"
now passes. GROOVY-12046 (included in alpha-2) makes the
GroovyObject.invokeMethod MOP fallback fire for the mock, and Spock's
invokeMethod fallback rethrows MissingMethodException, so @FailsWith is
satisfied again.
- The two GroovySpiesThatAreGlobal "non-existing property [setter] through
global spy" specs assert on the suppressed getter/setter
MissingMethodException that GroovyReal{Get,Set}PropertyInvoker attaches.
On Groovy 6 the property miss is thrown as MissingPropertyExceptionNoStack
and reconstructed at the call site by ScriptBytecodeAdapter.unwrap(),
which rebuilds the exception from name+type only and drops the suppressed
chain (and cause). The suppressed assertions are therefore gated to
Groovy < 6.
Note: the suppressed exception is an artifact of Spock's two-step invoker
(getter/setter method first, then the getProperty/setProperty fallback);
real, non-mocked Groovy throws a plain MissingPropertyException with no
suppressed. The version gate preserves the pre-6 diagnostic assertion
without over-fitting the spec to Spock internals on Groovy 6. The unwrap()
data loss is a latent Groovy issue not fixed in alpha-2.
leonard84
force-pushed
the
feat/groovy-6-alpha-testing
branch
from
d84dea7 to
3267073
Compare
leonard84
changed the title
|
Updated to Groovy Status of the three specs that previously failed under the Groovy 6 variant:
🤖 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2356 +/- ##
============================================
+ Coverage 82.32% 82.34% +0.01%
- Complexity 4877 4879 +2
============================================
Files 474 474
Lines 15205 15210 +5
Branches 1935 1937 +2
============================================
+ Hits 12518 12524 +6
Misses 1993 1993
+ Partials 694 693 -1
🚀 New features to boost your workflow:
|
|
@Vampire, what do you think about the changes to |
GROOVY-12045 is fixed in Groovy 6.0.0-alpha-2, so the @IgnoreIf workaround (and the now-unused GroovyRuntimeUtil import) can be removed. Verified all 10 WithBlocks tests pass on -Dvariant=6.0.
|
It's said that we loose the suppressed ones. |
|
Or maybe add a test with |
✅ All tests passed ✅Test SummaryVerify Branches and PRs / Build and Verify (4.0, 8, windows-latest) > :spock-specs:test
🏷️ Commit: 3d9f7ff Learn more about TestLens at testlens.app. |
|
Using |
|
3 participants
Adds Groovy 6.0.0-alpha-1 to the CI matrix for early compatibility testing, without including it in published releases.
Approach
Introduces
additionalVariantsListingradle.properties(analogous to the existingadditionalJavaTestVersionsList) as a general mechanism for declaring testing/preview variants. Variants listed there are included in thebuild-and-verifyCI matrix but never passed torelease-spock, so they cannot appear in any release — no conditional workflow logic needed.Moving Groovy 6 to
variantsListwhen it is stable requires a one-line change ingradle.properties.Changes
gradle.properties— addadditionalVariantsList=6.0libs.versions.toml— addgroovy6 = '6.0.0-alpha-1'version pin andgroovy-v6library alias (so Renovate can track it)build.gradle— addvariant == 6.0config block (org.apache.groovy, Java 17 minimum, range6.0.0–6.9.99); updateghActionsPublishJava guard to accept Java 17 for variant 6.0common.main.kts— expandMatrix.fullto includeadditionalVariants; add Java 17 minimum exclusion for6.0; rewriteexcludelambda withwhen (variant)for readability; update Windows/macOS includes to use Java 17 for6.0release.main.kts— no changes needed;release-spockalready uses onlyMatrix.axes.variantsNotes
A full build with
-Dvariant=6.0 -DjavaVersion=17currently fails atcompileJavabecause Groovy 6 Alpha 1 ships class files compiled with Java 17 (class file version 61), butSpockBasePluginhard-codes the Java compiler to JDK 11 (COMPILER_VERSION = JavaLanguageVersion.of(11)). Actual source-level changes will be needed before Groovy 6 can be promoted tovariantsList. This PR puts the CI infrastructure in place to track that work.