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

Migrate to spock 2 #4458

Merged
merged 5 commits into from
Oct 22, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions conventions/src/main/kotlin/otel.java-conventions.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,24 @@ dependencies {
testRuntimeOnly("org.junit.vintage:junit-vintage-engine")

testImplementation("org.objenesis:objenesis")
testImplementation("org.spockframework:spock-core")
testImplementation("org.spockframework:spock-core") {
// exclude optional dependencies
exclude(group = "cglib", module = "cglib-nodep")
exclude(group = "net.bytebuddy", module = "byte-buddy")
exclude(group = "org.junit.platform", module = "junit-platform-testkit")
exclude(group = "org.jetbrains", module = "annotations")
exclude(group = "org.objenesis", module = "objenesis")
exclude(group = "org.ow2.asm", module = "asm")
}
testImplementation("org.spockframework:spock-junit4") {
// spock-core is already added as dependency
// exclude it here to avoid pulling in optional dependencies
iNikem marked this conversation as resolved.
Show resolved Hide resolved
exclude(group = "org.spockframework", module = "spock-core")
}
testImplementation("ch.qos.logback:logback-classic")
testImplementation("org.slf4j:log4j-over-slf4j")
testImplementation("org.slf4j:jcl-over-slf4j")
testImplementation("org.slf4j:jul-to-slf4j")
testImplementation("info.solidsoft.spock:spock-global-unroll")
testImplementation("com.github.stefanbirkner:system-rules")
}

Expand Down
6 changes: 3 additions & 3 deletions dependencyManagement/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ val grpcVersion = "1.41.0"
rootProject.extra["otelVersion"] = otelVersion

// Need both BOM and -all
val groovyVersion = "2.5.11"
val groovyVersion = "2.5.14"

rootProject.extra["caffeine2Version"] = "2.9.2"
rootProject.extra["caffeine3Version"] = "3.0.4"
Expand Down Expand Up @@ -105,14 +105,14 @@ val DEPENDENCIES = listOf(
"commons-lang:commons-lang:2.6",
"commons-logging:commons-logging:1.2",
"commons-validator:commons-validator:1.7",
"info.solidsoft.spock:spock-global-unroll:0.5.1",
"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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"org.scala-lang:scala-library:2.11.12",
"org.springframework.boot:spring-boot-dependencies:2.3.1.RELEASE",
"io.grpc:grpc-netty-shaded:${grpcVersion}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already fixed, just not released yet: spockframework/spock#1333

Probably makes sense to link that PR/issue here.

// https://github.com/spockframework/spock/pull/1333
// def (queueUrl, topicArn) = setupTestInfrastructure(queueName, topicName)
Tuple tuple = setupTestInfrastructure(queueName, topicName)
def queueUrl = tuple.get(0)
def topicArn = tuple.get(1)
waitAndClearSetupTraces(queueUrl, queueName)

when:
Expand Down Expand Up @@ -61,7 +66,12 @@ 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
// https://github.com/spockframework/spock/pull/1333
// def (queueUrl, topicArn) = setupTestInfrastructure(queueName, topicName)
Tuple tuple = setupTestInfrastructure(queueName, topicName)
def queueUrl = tuple.get(0)
def topicArn = tuple.get(1)
waitAndClearSetupTraces(queueUrl, queueName)

when:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,17 @@ class Aws0ClientTest extends AgentInstrumentationSpecification {
request.request().headers().get("traceparent") == null

where:
service | operation | method | path | handlerCount | client | additionalAttributes | call | body
"S3" | "CreateBucket" | "PUT" | "/testbucket/" | 1 | new AmazonS3Client().withEndpoint("${server.httpUri()}") | ["aws.bucket.name": "testbucket"] | { client -> client.setS3ClientOptions(S3ClientOptions.builder().setPathStyleAccess(true).build()); client.createBucket("testbucket") } | ""
"S3" | "GetObject" | "GET" | "/someBucket/someKey" | 1 | new AmazonS3Client().withEndpoint("${server.httpUri()}") | ["aws.bucket.name": "someBucket"] | { client -> client.getObject("someBucket", "someKey") } | ""
"EC2" | "AllocateAddress" | "POST" | "/" | 4 | new AmazonEC2Client().withEndpoint("${server.httpUri()}") | [:] | { client -> client.allocateAddress() } | """
service | operation | method | path | handlerCount | client | additionalAttributes | call | body
"S3" | "CreateBucket" | "PUT" | "/testbucket/" | 1 | new AmazonS3Client().withEndpoint("${server.httpUri()}") | ["aws.bucket.name": "testbucket"] | { c -> c.setS3ClientOptions(S3ClientOptions.builder().setPathStyleAccess(true).build()); c.createBucket("testbucket") } | ""
"S3" | "GetObject" | "GET" | "/someBucket/someKey" | 1 | new AmazonS3Client().withEndpoint("${server.httpUri()}") | ["aws.bucket.name": "someBucket"] | { c -> c.getObject("someBucket", "someKey") } | ""
"EC2" | "AllocateAddress" | "POST" | "/" | 4 | new AmazonEC2Client().withEndpoint("${server.httpUri()}") | [:] | { c -> c.allocateAddress() } | """
<AllocateAddressResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">
<requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId>
<publicIp>192.0.2.1</publicIp>
<domain>standard</domain>
</AllocateAddressResponse>
"""
"RDS" | "DeleteOptionGroup" | "POST" | "/" | 1 | new AmazonRDSClient().withEndpoint("${server.httpUri()}") | [:] | { client -> client.deleteOptionGroup(new DeleteOptionGroupRequest()) } | """
"RDS" | "DeleteOptionGroup" | "POST" | "/" | 1 | new AmazonRDSClient().withEndpoint("${server.httpUri()}") | [:] | { c -> c.deleteOptionGroup(new DeleteOptionGroupRequest()) } | """
<DeleteOptionGroupResponse xmlns="http://rds.amazonaws.com/doc/2014-09-01/">
<ResponseMetadata>
<RequestId>0ac9cda2-bbf4-11d3-f92b-31fa5e8dbc99</RequestId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test failed to compile claiming that there already is a variable named observer in current scope

@Override
void onNext(Helloworld.Response value) {
clientReceived << value.message
Expand All @@ -99,9 +99,9 @@ abstract class AbstractGrpcStreamingTest extends InstrumentationSpecification {

clientRange.each {
def message = Helloworld.Response.newBuilder().setMessage("call $it").build()
observer.onNext(message)
observer2.onNext(message)
}
observer.onCompleted()
observer2.onCompleted()

then:
assertTraces(1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class GwtTest extends AgentInstrumentationSpecification implements HttpServerTes
}

def setupSpec() {
setupServer()

Testcontainers.exposeHostPorts(port)

browser = new BrowserWebDriverContainer<>()
Expand All @@ -62,6 +64,7 @@ class GwtTest extends AgentInstrumentationSpecification implements HttpServerTes
}

def cleanupSpec() {
cleanupServer()
browser?.stop()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.hibernate.SessionFactory
import org.hibernate.cfg.Configuration
import org.hibernate.exception.SQLGrammarException
import org.hibernate.procedure.ProcedureCall
import org.junit.Assume
import org.junit.jupiter.api.Assumptions
import spock.lang.Shared

import javax.persistence.ParameterMode
Expand Down Expand Up @@ -62,7 +62,7 @@ class ProcedureCallTest extends AgentInstrumentationSpecification {
call.getOutputs()
} catch (Exception exception) {
// ignore failures on hibernate 6 where this functionality has not been implemented yet
Assume.assumeFalse("org.hibernate.NotYetImplementedFor6Exception" == exception.getClass().getName())
Assumptions.assumeFalse("org.hibernate.NotYetImplementedFor6Exception" == exception.getClass().getName())
throw exception
}
}
Expand Down Expand Up @@ -127,7 +127,7 @@ class ProcedureCallTest extends AgentInstrumentationSpecification {

ProcedureCall call = session.createStoredProcedureCall("TEST_PROC")
def parameterRegistration = call.registerParameter("nonexistent", Long, ParameterMode.IN)
Assume.assumeTrue(parameterRegistration.metaClass.getMetaMethod("bindValue", Object) != null)
Assumptions.assumeTrue(parameterRegistration.metaClass.getMetaMethod("bindValue", Object) != null)
parameterRegistration.bindValue(420L)
try {
callProcedure(call)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,11 @@ class HttpUrlConnectionTest extends HttpClientTest<HttpURLConnection> implements
setup:
def url = resolveAddress("/success").toURL()
HttpURLConnection connection = runWithSpan("someTrace") {
HttpURLConnection connection = url.openConnection()
connection.setRequestProperty("Connection", "close")
HttpURLConnection con = url.openConnection()
con.setRequestProperty("Connection", "close")
assert Span.current().getSpanContext().isValid()
assert connection.getResponseCode() == STATUS
return connection
assert con.getResponseCode() == STATUS
return con
}

expect:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ dependencies {
api(project(":testing-common"))
implementation("io.opentelemetry:opentelemetry-api")

val arquillianVersion = "1.4.0.Final"
implementation("org.jboss.arquillian.junit:arquillian-junit-container:$arquillianVersion")
val arquillianVersion = "1.7.0.Alpha10"
implementation("org.jboss.arquillian.junit5:arquillian-junit5-container:$arquillianVersion")
implementation("org.jboss.arquillian.protocol:arquillian-protocol-servlet:$arquillianVersion")
implementation("org.jboss.arquillian.spock:arquillian-spock-container:1.0.0.CR1")
api("org.jboss.shrinkwrap:shrinkwrap-impl-base:1.2.6")
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

import static org.assertj.core.api.Assertions.assertThat;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite a bit of groovy -> Java - I'll take what I can get :-)


import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension;
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
import io.opentelemetry.testing.internal.armeria.client.WebClient;
import io.opentelemetry.testing.internal.armeria.common.AggregatedHttpResponse;
import java.net.URI;
import org.jboss.arquillian.container.test.api.Deployment;
import org.jboss.arquillian.container.test.api.RunAsClient;
import org.jboss.arquillian.junit5.ArquillianExtension;
import org.jboss.arquillian.test.api.ArquillianResource;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.EmptyAsset;
import org.jboss.shrinkwrap.api.spec.WebArchive;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import test.CdiRestResource;
import test.EjbRestResource;
import test.RestApplication;

@ExtendWith(ArquillianExtension.class)
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@RunAsClient
public abstract class AbstractArquillianRestTest {

@RegisterExtension
static final InstrumentationExtension testing = AgentInstrumentationExtension.create();

public final WebClient client = WebClient.of();

@ArquillianResource public URI url;

@Deployment
static WebArchive createDeployment() {
return ShrinkWrap.create(WebArchive.class)
.addClass(RestApplication.class)
.addClass(CdiRestResource.class)
.addClass(EjbRestResource.class)
.addAsWebInfResource(EmptyAsset.INSTANCE, "beans.xml");
}

private String getContextRoot() {
return url.getPath();
}

@ParameterizedTest
@CsvSource({"rest-app/cdiHello, CdiRestResource", "rest-app/ejbHello, EjbRestResource"})
public void testHelloRequest(String path, String className) {
AggregatedHttpResponse response = client.get(url.resolve(path).toString()).aggregate().join();

assertThat(response.status().code()).isEqualTo(200);
assertThat(response.contentUtf8()).isEqualTo("hello");

testing.waitAndAssertTraces(
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName(getContextRoot() + path).hasKind(SpanKind.SERVER).hasNoParent(),
span -> span.hasName(className + ".hello").hasParent(trace.getSpan(0))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ import static Resource.Test3

class CxfFilterTest extends JaxRsFilterTest implements HttpServerTestTrait<Server> {

def setupSpec() {
setupServer()
}

def cleanupSpec() {
cleanupServer()
}

@Override
boolean testAbortPrematch() {
false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@
* SPDX-License-Identifier: Apache-2.0
*/

class WildflyRestTest extends ArquillianRestTest {
}
public class PayaraArquillianRestTest extends AbstractArquillianRestTest {}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import org.junit.Assume
import org.junit.jupiter.api.Assumptions
import spock.lang.Shared
import spock.lang.Unroll

Expand Down Expand Up @@ -51,7 +51,7 @@ abstract class JaxRsFilterTest extends AgentInstrumentationSpecification {
}

def "test #resource, #abortNormal, #abortPrematch"() {
Assume.assumeTrue(!abortPrematch || testAbortPrematch())
Assumptions.assumeTrue(!abortPrematch || testAbortPrematch())

given:
simpleRequestFilter.abort = abortNormal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEn
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static java.util.concurrent.TimeUnit.SECONDS
import static org.junit.Assume.assumeTrue
import static org.junit.jupiter.api.Assumptions.assumeTrue

abstract class JaxRsHttpServerTest<S> extends HttpServerTest<S> implements AgentTestTrait {

Expand Down
Loading