Update Groovy and Spock versions #4840

nikita-tkachenko-datadog · 2023-03-03T11:26:49Z

What Does This Do

Updates Groovy version (2.5 -> 3.0) and Spock version (1.3 -> 2.1).
Migrates all tests that used to run on JUnit 4 to JUnit 5.

Motivation

Our current version of Spock is 1.3.
It only support Groovy 2.x, which has some drawbacks:

our Gradle version uses Groovy 3.0.10, as the result gradleTestKit() cannot be used due to dependency conflicts
Groovy 2 has some annoying bugs (e.g. https://issues.apache.org/jira/browse/GROOVY-10137)

Additional Notes

SpockRunner that we use to run instrumentation tests cannot be properly ported to JUnit 5, since the framework does not provide the hooks / extension points that can be used to shadow the tested class (see Introduce extension API for providing a custom ClassLoader (e.g. for Powermock)).

In order to mitigate this, SpockRunner was changed to extend JUnitPlatform, which is a JUnit 4 runner that allows executing JUnit 5 tests in a JUnit 4 environment (i.e. running them as JUnit 4 tests).

So even though Spock 2 tests run on top of JUnit 5, we execute them in "compatibility mode" so that SpockRunner could shadow the test class (see SputnikRunner removed for more details).

dd-java-agent/testing/src/test/groovy/server/HttpServerTest.groovy

dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentApiTest.groovy

PerfectSlayer

Great work 👏

I wonder if it will be an issue for the junit-4 and junit-5 instrumentation to not properly test the minimal version supported. I will try to get some feedback on it.
In the meantime, I left you some comments.

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/PathTraversalModuleTest.groovy

PerfectSlayer · 2023-03-07T09:47:00Z

gradle/dependencies.gradle

@@ -1,5 +1,5 @@
 final class CachedData {
-  static groovyVer = "2.5.21"
+  static groovyVer = "3.0.10"


Why not using groovy 4?

One of the reasons for doing this change was to be able to use gradleTestKit(), and the one that comes with our Gradle version uses Groovy 3.0.10.
If I update it to Groovy 4 I am afraid the issue will be the same as before, only this time Spock's Groovy will be newer than the test kit's one, not older.

Yeah, I tried to bump Groovy version to version 4 but I had to leave it to 3 for Gradle plugins for example.
Is there any specific module(s) that would requires version 3 for your tests or will it impact the whole project?
I wonder if it could be doable (and recommended) to update to 4 while leaving some module to 3 (like we did for JUnit4/5).

It will be just one module that does Gradle instrumentation. Technically it would be possible, of course, to have that specific module use Groovy 3, but it would complicate things too much in my opinion (as that module depends on a couple of common testing modules which themselves depend on Groovy).
Perhaps it would be simpler to migrate to Groovy 4 together with Gradle wrapper update, to avoid the need to have multiple versions.

PerfectSlayer · 2023-03-07T09:47:30Z

gradle/dependencies.gradle

@@ -10,9 +10,8 @@ final class CachedData {
    okhttp_legacy : "[3.0,3.12.12]", // 3.12.x is last version to support Java7)
    okio          : "1.16.0",

-    spock         : "1.3-groovy-$spockGroovyVer",
+    spock         : "2.1-groovy-$spockGroovyVer",


Same question here: why not using latest Spock version 2.3?

Initial intention was to do the minimal upgrade required to use Groovy 3. I will try the newest version to see if it doesn't require further changes

For some strange reason, upgrading Groovy from 3.0.10 to 3.0.11 breaks tests in :dd-java-agent:instrumentation:netty-4.1:test module.
This blocks upgrading Spock.

I assume it's some transitive dependency change in Groovy that is to blame (my prime suspect is asm, which has been bumped from 9.2 to 9.3), but I wasn't able to prove it.

I cannot dedicate much more time to this change at the moment, so my suggestion would be staying on Groovy 3.0.10 / Spock 2.1 combination for now, and then possibly upgrading them later as a separate change

Personally, I wouldn't mind upgrading Groovy and Spock in separate PR, since these will probably require many more fixes.

Sure. About my above comment, this question (global groovy 4 with some groovy 3 modules) can be investigate later in another PR in you have other priorities right now.

gradle/dependencies.gradle

dd-java-agent/instrumentation/build.gradle

nikita-tkachenko-datadog · 2023-03-07T10:36:12Z

I wonder if it will be an issue for the junit-4 and junit-5 instrumentation to not properly test the minimal version supported.

Yeah, I am not too happy about this, but there isn't an alternative that I can think of.
Test framework instrumentations are only used by CI Visibility, so I would be the person responsible for supporting them.
I can live with that, since it doesn't seem to be too much of a burden to do basic manual checks to ensure minimum versions work.
Although I am open to discussion of course.

…Unit 5 assertions

… in projects that utilize SpockRunner for instrumentation tests

…eager loading

…mentation classpath of some instrumentation modules, as they were incompatible with the versions used by Spock

…d to verify mock interactions, and Spock 2.x is strict about this

…h newer Spock version

PerfectSlayer · 2023-03-07T12:40:29Z

Yeah, I am not too happy about this, but there isn't an alternative that I can think of.

We could start thinking of moving away from SpockRunner by creating an agent to load bootstrap classes into bootstrap classloader (like the dd-java-agent.jar but that only alter the classpath) and run tests with the agent on a forked VM.
It should work but it feels way out of the scope of this PR, especially if you (as JUnit integration maintainer) can live with this PR change 😅

…r to use JUnitPlatform

jpbempel

LGTM for debugger

smola

Great job!

gradle/configure_tests.gradle

PerfectSlayer · 2023-03-09T11:35:39Z

Seems all good. Can you squash the commits when merging? 🙏

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/spock-and-groovy-update branch from d1a62af to 41c8c78 Compare March 6, 2023 11:20

This was referenced Mar 6, 2023

Upgrade to Spock 2 #4348

Closed

Upgrade tests to Groovy 2.5.19 #4345

Closed

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/spock-and-groovy-update branch 2 times, most recently from 5bd62a8 to e95d721 Compare March 6, 2023 21:56

smola reviewed Mar 7, 2023

View reviewed changes

dd-java-agent/testing/src/test/groovy/server/HttpServerTest.groovy Show resolved Hide resolved

dd-trace-core/src/test/groovy/datadog/trace/common/writer/DDAgentApiTest.groovy Show resolved Hide resolved

PerfectSlayer reviewed Mar 7, 2023

View reviewed changes

nikita-tkachenko-datadog changed the title ~~WIP: update spock version~~ Update Groovy and Spock versions Mar 7, 2023

nikita-tkachenko-datadog added type: refactoring tag: dependencies Dependencies related changes labels Mar 7, 2023

nikita-tkachenko-datadog added 7 commits March 7, 2023 12:19

Update JUnit 4 tests to run on JUnit 5; update JUnit 5 tests to use J…

9db1916

…Unit 5 assertions

Use JUnit 5 to run tests in all projects by default; use JUnit 4 only…

196d4a6

… in projects that utilize SpockRunner for instrumentation tests

Fix Groovy 3 compilation errors (variable name clashes, etc.)

bdda8c9

Move HelperClass from tests source set to testFixtures, to avoid its …

05d8a9f

…eager loading

Exclude transient JUnit, Assertj and ASM dependencies from test imple…

485bd4d

…mentation classpath of some instrumentation modules, as they were incompatible with the versions used by Spock

Use Mock instead of Stub, since we need both to stub test behavior an…

e7a1afb

…d to verify mock interactions, and Spock 2.x is strict about this

Update JUnit 4 and JUnit 5 instrumentation tests to be compatible wit…

aeb033b

…h newer Spock version

Update Groovy version to 3.x, Spock version to 2.x; update SpockRunne…

f20c2e3

…r to use JUnitPlatform

nikita-tkachenko-datadog force-pushed the nikita-tkachenko/spock-and-groovy-update branch from f8b19af to f20c2e3 Compare March 7, 2023 13:50

nikita-tkachenko-datadog marked this pull request as ready for review March 7, 2023 15:37

nikita-tkachenko-datadog requested a review from a team March 7, 2023 15:37

nikita-tkachenko-datadog requested review from a team as code owners March 7, 2023 15:37

nikita-tkachenko-datadog requested review from ojung and removed request for a team March 7, 2023 15:37

jpbempel approved these changes Mar 7, 2023

View reviewed changes

smola approved these changes Mar 7, 2023

View reviewed changes

PerfectSlayer reviewed Mar 7, 2023

View reviewed changes

gradle/configure_tests.gradle Outdated Show resolved Hide resolved

PerfectSlayer approved these changes Mar 7, 2023

View reviewed changes

Update comments in tests configuration script

4b779ad

nikita-tkachenko-datadog merged commit 2dbc617 into master Mar 9, 2023

nikita-tkachenko-datadog deleted the nikita-tkachenko/spock-and-groovy-update branch March 9, 2023 15:38

github-actions bot added this to the 1.11.0 milestone Mar 9, 2023

bantonsson modified the milestones: 1.11.0, new 1.11.0 Mar 17, 2023

smola added comp: testing Testing tag: no release notes Changes to exclude from release notes labels Mar 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Groovy and Spock versions #4840

Update Groovy and Spock versions #4840

nikita-tkachenko-datadog commented Mar 3, 2023 •

edited

Loading

PerfectSlayer left a comment

PerfectSlayer Mar 7, 2023

nikita-tkachenko-datadog Mar 7, 2023

PerfectSlayer Mar 7, 2023

nikita-tkachenko-datadog Mar 7, 2023

PerfectSlayer Mar 7, 2023

nikita-tkachenko-datadog Mar 7, 2023

nikita-tkachenko-datadog Mar 7, 2023 •

edited

Loading

smola Mar 7, 2023

PerfectSlayer Mar 7, 2023

nikita-tkachenko-datadog commented Mar 7, 2023 •

edited

Loading

PerfectSlayer commented Mar 7, 2023

jpbempel left a comment

smola left a comment

PerfectSlayer commented Mar 9, 2023

Update Groovy and Spock versions #4840

Update Groovy and Spock versions #4840

Conversation

nikita-tkachenko-datadog commented Mar 3, 2023 • edited Loading

What Does This Do

Motivation

Additional Notes

PerfectSlayer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikita-tkachenko-datadog Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikita-tkachenko-datadog commented Mar 7, 2023 • edited Loading

PerfectSlayer commented Mar 7, 2023

jpbempel left a comment

Choose a reason for hiding this comment

smola left a comment

Choose a reason for hiding this comment

PerfectSlayer commented Mar 9, 2023

nikita-tkachenko-datadog commented Mar 3, 2023 •

edited

Loading

nikita-tkachenko-datadog Mar 7, 2023 •

edited

Loading

nikita-tkachenko-datadog commented Mar 7, 2023 •

edited

Loading