Migrate to spock 2 #4458
Migrate to spock 2 #4458
Conversation
laurit
requested review from
anuraaga,
iNikem,
jkwatson,
mateuszrzeszutek,
pavolloffay,
trask and
tylerbenson
as code owners
| "io.netty:netty:3.10.6.Final", | ||
| "org.assertj:assertj-core:3.21.0", | ||
| "org.awaitility:awaitility:4.1.0", | ||
| "org.checkerframework:checker-qual:3.14.0", | ||
| "org.codehaus.groovy:groovy-all:${groovyVersion}", | ||
| "org.objenesis:objenesis:3.2", | ||
| "org.spockframework:spock-core:1.3-groovy-2.5", | ||
| "org.spockframework:spock-core:2.0-groovy-2.5", | ||
| "org.spockframework:spock-junit4:2.0-groovy-2.5", |
There was a problem hiding this comment.
Do you know what we need this for? Ideally it runs with jupiter now without needing any junit4 stuff (I noticed we removed some junit4 annotations)
There was a problem hiding this comment.
It is needed for junit @Rule which is used in a couple of tests that mutate environment variables and in jaxrs+dropwizard tests. I initially got rid of these rules by extracting code from rule classes, but reverted that when I found this compatibility module.
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; |
There was a problem hiding this comment.
Quite a bit of groovy -> Java - I'll take what I can get :-)
| @@ -44,7 +42,6 @@ trait HttpServerTestTrait<SERVER> implements RetryOnAddressAlreadyInUseTrait { | |||
| static int port | |||
| static URI address | |||
|
|
|||
| @BeforeClass | |||
There was a problem hiding this comment.
Would jupiter's BeforeAll / AfterAll have worked?
There was a problem hiding this comment.
As far as I understand spock 2 doesn't extend junit 5, it more like replaces it while using the same platform that junit 5 is built on, so none of the junit 5 annotations work.
| @@ -81,7 +81,7 @@ abstract class AbstractGrpcStreamingTest extends InstrumentationSpecification { | |||
| GreeterGrpc.GreeterStub client = GreeterGrpc.newStub(channel).withWaitForReady() | |||
|
|
|||
| when: | |||
| def observer = client.conversation(new StreamObserver<Helloworld.Response>() { | |||
| def observer2 = client.conversation(new StreamObserver<Helloworld.Response>() { | |||
There was a problem hiding this comment.
test failed to compile claiming that there already is a variable named observer in current scope
| @@ -230,11 +230,11 @@ class LettuceAsyncClientTest extends AgentInstrumentationSpecification { | |||
| String successStr = "KEY MISSING" | |||
| BiFunction<String, Throwable, String> firstStage = new BiFunction<String, Throwable, String>() { | |||
| @Override | |||
| String apply(String res, Throwable throwable) { | |||
| String apply(String res, Throwable error) { | |||
There was a problem hiding this comment.
spock 2 has a bug that makes it fail when local variable is named throwable
| @@ -495,15 +495,19 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification { | |||
| connection = driver.connect(jdbcUrl, null) | |||
| } | |||
|
|
|||
| def (Statement statement, ResultSet rs) = runWithSpan("parent") { | |||
| // TODO: def (Statement statement, ResultSet rs) fails to compile, switch back when this is fixed in spock | |||
There was a problem hiding this comment.
I believe this bug only manifests when test has a cleanup block
| @@ -495,15 +495,19 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification { | |||
| connection = driver.connect(jdbcUrl, null) | |||
| } | |||
|
|
|||
| def (Statement statement, ResultSet rs) = runWithSpan("parent") { | |||
| // TODO: def (Statement statement, ResultSet rs) fails to compile, switch back when this is fixed in spock | |||
| @@ -24,7 +24,11 @@ class SnsCamelTest extends AgentInstrumentationSpecification { | |||
| String queueName = "snsCamelTest" | |||
| def camelApp = new CamelSpringApp(awsConnector, SnsConfig, ImmutableMap.of("topicName", topicName, "queueName", queueName)) | |||
|
|
|||
| def (queueUrl, topicArn) = setupTestInfrastructure(queueName, topicName) | |||
| // TODO: def (queueUrl, topicArn) fails to compile, switch back when this is fixed in spock | |||
There was a problem hiding this comment.
It is already fixed, just not released yet: spockframework/spock#1333
Probably makes sense to link that PR/issue here.
|
@laurit looks like i caused merge conflicts, i'll resolve and push |
* Migrate to spock 2 * Fix smoke test suites * address review comments * review comment Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
Resolves #4271