Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Groovy 4 support #1382

Merged
merged 15 commits into from
Feb 16, 2022
Merged

Conversation

leonard84
Copy link
Member

No description provided.

@leonard84 leonard84 mentioned this pull request Oct 11, 2021
@leonard84 leonard84 linked an issue Oct 11, 2021 that may be closed by this pull request
@szpak
Copy link
Member

szpak commented Oct 11, 2021

I started to work on the same a few days ago - still having some tests red (I could make a draft then, to do not duplicate our work :-/). However, I'm not sure, if we should release Spock 2.1-groovy-4.0 due to the reasons, I explained in #1374 (comment).

@leonard84
Copy link
Member Author

I don't intend to release a 4.0 variant for 2.1 unless groovy 4.0 final is already release, but I don't want to wait that long.

@szpak
Copy link
Member

szpak commented Oct 11, 2021

I don't intend to release a 4.0 variant for 2.1 unless groovy 4.0 final is already release, but I don't want to wait that long.

Great, my plan was also to be ready with the stuff and eventually release SNAPSHOTs.

@szpak
Copy link
Member

szpak commented Nov 6, 2021

I merged in my initial changes with fixing stack trace related assertions broken by Indy calls (enabled by default in Groovy 4, but still possible with Groovy 2.5 and 3).

What's interesting, there all the tests are executed from Idea, two still fails with:

Condition not satisfied:

methodName == "-" || methodName == traceElem.methodName
|          |      |  |          |  |         |
|          |      |  |          |  |         foo_closure3
|          |      |  |          |  apackage.ASpec.foo_closure3(Script_3fd3a5801e374ef551955ab837465535.groovy:7)
|          |      |  |          false
|          |      |  |          1 difference (91% similarity)
|          |      |  |          foo_closure(1)
|          |      |  |          foo_closure(3)
|          |      |  foo_closure1
|          |      false
|          false
|          12 differences (0% similarity)
|          (foo_closure1)
|          (------------)
foo_closure1


	at org.spockframework.EmbeddedSpecification.stackTraceLooksLike_closure3(EmbeddedSpecification.groovy:58)
	at groovy.lang.Closure.call(Closure.java:419)
	at org.codehaus.groovy.vmplugin.v8.IndyInterface.fromCache(IndyInterface.java:318)
	at org.spockframework.EmbeddedSpecification.stackTraceLooksLike(EmbeddedSpecification.groovy:51)
	at org.spockframework.smoke.StackTraceFiltering.when creation of data provider fails, stack trace points to corresponding parameterization(StackTraceFiltering.groovy:229)

as for some reasons, there seems to be more closures in a generated script file (and own is no. 3). It doesn't happen when StackTraceFiltering tests are executed separately. I leave it for further investigation.

@codecov
Copy link

codecov bot commented Dec 5, 2021

Codecov Report

Merging #1382 (50da321) into master (6e08587) will increase coverage by 0.10%.
The diff coverage is 52.17%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1382      +/-   ##
============================================
+ Coverage     79.36%   79.46%   +0.10%     
- Complexity     3987     3992       +5     
============================================
  Files           403      403              
  Lines         12413    12537     +124     
  Branches       1639     1642       +3     
============================================
+ Hits           9852     9963     +111     
- Misses         1968     1978      +10     
- Partials        593      596       +3     
Impacted Files Coverage Δ
...a/org/spockframework/runtime/StackTraceFilter.java 97.56% <ø> (ø)
...framework/specs/jacoco/JacocoAstDumpTrigger.groovy 37.50% <25.00%> (-2.50%) ⬇️
...main/java/org/spockframework/compiler/AstUtil.java 80.28% <37.50%> (-1.95%) ⬇️
...java/org/spockframework/compiler/SpecRewriter.java 93.71% <70.00%> (-0.72%) ⬇️
...rg/spockframework/compiler/WhereBlockRewriter.java 94.01% <100.00%> (ø)
...c/main/groovy/spock/util/EmbeddedSpecRunner.groovy 84.21% <0.00%> (-0.87%) ⬇️
...rc/main/groovy/spock/util/matcher/IsCloseTo.groovy 100.00% <0.00%> (ø)
...ovy/spock/util/concurrent/BlockingVariables.groovy 80.00% <0.00%> (ø)
...ock/util/SourceToAstNodeAndSourceTranspiler.groovy 79.09% <0.00%> (+2.06%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e08587...50da321. Read the comment docs.

@msgilligan
Copy link
Contributor

I don't intend to release a 4.0 variant for 2.1 unless groovy 4.0 final is already release, but I don't want to wait that long.

The Groovy devs are voting on Groovy 4.0 final release as of today.

@szpak szpak force-pushed the groovy-4-support branch 2 times, most recently from 756d3a7 to b204618 Compare January 26, 2022 22:35
@szpak
Copy link
Member

szpak commented Jan 26, 2022

All CI build are green after I disabled @CompileStatic transformation in a few tests and test configuration classes.

However, I don't know why it fails that way and what's more strange, why some of those places fails also with JDK 8 and 11 (not just 17). Could we have some problem with build-cache on CI?
Another interesting thing is that I'm not able to reproduce (some) of those problems locally. Only the one place in the first "workaround" commit fails on my machine (with different JDKs). Those in Spring test configuration work fine (even with gw clean check -Dvariant=4.0 --no-build-cache).

I've just asked on the Groovy Slack about their ideas:
https://groovy-community.slack.com/archives/C2NEFCM55/p1643236824027100

Update. Paul pointed out the issue Leonard created in December :) - https://issues.apache.org/jira/browse/GROOVY-10403. Can we "fix" it on the Spock side?

In addition, I wonder, which Groovy 4 specific features (such as (emulated) sealed classes) you think are worth testing explicitly with Spock?

@szpak
Copy link
Member

szpak commented Jan 26, 2022

Btw, some tests are not stable on CI (just with this branch?).

One parallel test failed twice with JDK 8 and Groovy 3.0 and 4.0. However, it doesn't seem to be related:

ParallelSpec > @ResourceLock with only READ allows parallel execution of data-driven features FAILED
    Condition not satisfied:

    atomicInteger.get() == 3
    |             |     |
    3             1     false
        at apackage.ASpec.writeA(Script_9186d4f70d72d15ebba0478d4c746046.groovy:14)

Another one (in a separate run):

TimeoutExtension > stack trace shows where thread is hung FAILED
    org.gradle.internal.exceptions.DefaultMultiCauseException: Multiple Failures (4 failures)
    	org.spockframework.runtime.SpockComparisonFailure: Condition not satisfied:

    trace.size() == lines.size()
    |     |      |  |     |
    |     1      |  |     2
    |            |  [apackage.ASpec|helper|7,       apackage.ASpec|foo|3]
    |            false
    [apackage.ASpec.foo(Script_4c375ed6a7d6b75b709b5b6f940f6e5c.groovy:3)]

    	org.spockframework.runtime.SpockComparisonFailure: Condition not satisfied:

    trace.size() == lines.size()
    |     |      |  |     |
    |     1      |  |     2
    |            |  [apackage.ASpec|helper|7,       apackage.ASpec|foo|3]
    |            false
    [apackage.ASpec.foo(Script_4c375ed6a7d6b75b709b5b6f940f6e5c.groovy:3)]

    	org.spockframework.runtime.WrongExceptionThrownError: Expected exception of type 'org.spockframework.runtime.SpockTimeoutError', but no exception was thrown
    	org.spockframework.runtime.WrongExceptionThrownError: Expected exception of type 'org.spockframework.runtime.SpockTimeoutError', but no exception was thrown
        at org.spockframework.runtime.ErrorInfoCollector.assertEmpty(ErrorInfoCollector.java:32)
        at org.spockframework.runtime.IterationNode.execute(IterationNode.java:49)

@lptr
Copy link

lptr commented Feb 1, 2022

Given that Spock 2.1 is not released yet, and that Groovy 4 is released now, is this going to make it into Spock 2.1?

@szpak
Copy link
Member

szpak commented Feb 1, 2022

Given that Spock 2.1 is not released yet, and that Groovy 4 is released now, is this going to make it into Spock 2.1?

Groovy 4 GA is out, so assuming no other obstacles, it will be a part of 2.1.

@eric-milles
Copy link
Contributor

Can the reference to Groovy 4 RC2 be updated? For the "explain why compilation started to fail", do you need some help looking at something?

@szpak
Copy link
Member

szpak commented Feb 4, 2022

Can the reference to Groovy 4 RC2 be updated?

I've just updated it. Thanks for your reminder.

@leonard84 leonard84 marked this pull request as ready for review February 16, 2022 13:24
@leonard84 leonard84 merged commit bec1dae into spockframework:master Feb 16, 2022
@leonard84 leonard84 deleted the groovy-4-support branch February 16, 2022 13:41
gesellix added a commit to gesellix/couchdb-client that referenced this pull request Jul 25, 2022
See http://groovy-lang.org/releasenotes/groovy-4.0.html

Requires a Spock Framework upgrade: spockframework/spock#1382
Also see the Gradle side of things: gradle/gradle#20038
gesellix added a commit to gesellix/couchdb-client that referenced this pull request Jul 25, 2022
See http://groovy-lang.org/releasenotes/groovy-4.0.html

Requires a Spock Framework upgrade: spockframework/spock#1382
Also see the Gradle side of things: gradle/gradle#20038
bot-gradle added a commit to gradle/gradle that referenced this pull request Jan 4, 2023
This is a cleaned up version of #18552.

Building with `-DbundleGroovy4=true` on the CLI will use Groovy 4.0.7 libs.  An additional Gradleception job is already in place to invoke this code path.

If we choose to unconditionally upgrade to Groovy 4, we should revert this PR followed by setting the Groovy version value in `ExternalModulesExtension.kt`.

Alternatively, we could modify this PR to test forward with Groovy 5.  This would likely also need temporary inclusion of apache snapshot repositories; see #20972.

## Preparation PRs

- #18665
- #20037
- #21504
- #21497
- #21986
- #22097
- #22130
- #22133
- #22151
- #22157
- #22158
- #22178
- #22208
- #22213

## TODO

- [x] allow configuration-cache instrumentation to handle `INVOKEDYNAMIC`
  - #20795
  - #20799
  - #20616
- [x] fix capabilities
- [ ] mention relevant breaking changes in release notes/upgrade guide
  - [x] `is*()` getters are not supported for `Boolean` properties anymore; see #22218
  - [ ] `INVOKEDYNAMIC` is the only way forward
- [x] upgrade Spock to Groovy 4-compatible version – spockframework/spock#1382
  - [x] upgrade to Spock 2.2 GA
  - [x] upgrade to Spock 2.2-M1
- [x] upgrade CodeNarc – #20655
  - [x] CodeNarc/CodeNarc#678
    - this prevents running CodeNarc because it depends on the now-removed `groovy.utils.XmlParser`
    - it makes `IsolatedAntBuilderMemoryLeakIntegrationTest.CodeNarc does not fail with PermGen space error` fail
  - [x] CodeNarc/CodeNarc#682
- [x] upgrade to Groovy 4
  - [x] upgrade to Groovy 4.0.7
  - [x] [GROOVY-10765](https://issues.apache.org/jira/browse/GROOVY-10765) _STC: Closure implementation of Java @FunctionalInterface loses type information_
    - Can workaround using a cast in UnitOfWorkBuilder
    - Fix coming in 4.0.6
  - [x] [GROOVY-10731](https://issues.apache.org/jira/browse/GROOVY-10731) _Exceptions thrown from MarkupTemplateEngine when map accessors and GString interpolation are used_
    - fixed in 4.0.5
  - [ ] [GROOVY-10709](https://issues.apache.org/jira/browse/GROOVY-10709) _Performance regression in Gradle with Groovy 4_
    - no clear cause; mitigation/investigation is ongoing
  - [x] [GROOVY-10708](https://issues.apache.org/jira/browse/GROOVY-10708) _Property access for wrapped vs. primitive boolean_
    - Groovy team will not fix
    - @big-guy has suggested a workaround in Gradle to prevent breaking legacy plugins/tasks, to be implemented as #22218
  - [x] [GROOVY-10772](https://issues.apache.org/jira/browse/GROOVY-10772) _Possible memory leak, CacheableCallSite retains objects across invocations_
    - Causes OOME-related flakiness in `o.g.w.i.WorkerExecutorIntegrationTest#does not leak project state across multiple builds`, and OOME mostly happens with Java 8
  - [x] [GROOVY-10707](https://issues.apache.org/jira/browse/GROOVY-10707) _Regression in property access for conflicting isXxx and getXxx accessors_
    - Groovy team will not fix
    - Our tests are refactored to accommodate the breaking change
  - [x] [GROOVY-10591](https://issues.apache.org/jira/browse/GROOVY-10591) _Static method not found on Java interface_
    - no fix planned yet
  - [x] [GROOVY-10543](https://issues.apache.org/jira/browse/GROOVY-10543) _Problem with groovy-all Gradle module metadata_
    - fixed in 4.0.2
  - [x] [GROOVY-10514](https://issues.apache.org/jira/browse/GROOVY-10514) _Class method is not called anymore from inside closure_
    - apparently works as designed, added workarounds
  - [x] [GROOVY-10555](https://issues.apache.org/jira/browse/GROOVY-10555) _MetaClass.getProperites() returns package private fields_
    - no plan to fix yet, but added a workaround
  - [x] [GROOVY-10521](https://issues.apache.org/jira/browse/GROOVY-10521) _Compiler complains about abstract method not implemented when implementing trait_
    - not fixed yet, but we added a workaround
  - [x] ~~[GROOVY-10466](https://issues.apache.org/jira/browse/GROOVY-10466) _Compilation error on Spock expectation_~~
    - To be fixed in apache/groovy#1698
    - Fixed in 4.0.2-SNAPSHOT
  - [x] ~~[GROOVY-10467](https://issues.apache.org/jira/browse/GROOVY-10467) _Compilation fails with method detected as transient_~~
    - doesn't seem to be a problem with `4.0.1`
  - [x] ~~[GROOVY-10299](https://issues.apache.org/jira/browse/GROOVY-10299) _Groovy compiler generates invalid Java stubs_~~
    - already fixed in `4.0.0-beta-2`
  - [x] ~~[GROOVY-10290](https://issues.apache.org/jira/browse/GROOVY-10290) _Dynamic Groovy code in Gradle doesn't compile because of $getLookup() method is not static_~~
    - fixed in `4.0.1`
- [x] fix build scripts being compiled with binary compatibility (=class format version) of the runtime Java version
  - this makes the following tests fail:
    - `WorkerDaemonIntegrationTest`
    - `ConfigureRuntimeClasspathNormalizationIntegrationTest`
    - `CheckstylePluginToolchainsIntegrationTest`
    - `BuildEnvironmentIntegrationTest`
- [x] use non-deprecated Groovy `AntBuilder` for the super-class of our own `AntBuilder`
- [x] upgrade Groovy version in `AbstractSourcesAndJavadocJarsIntegrationTest`
- [x] use `org.apache.groovy` group ID for Groovy versions > 4 in `GroovyParametersMetadataIntegrationTest` (and probably in many other places)

Co-authored-by: Octavia Togami <otogami@gradle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Groovy 4
5 participants