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.

Sign up for GitHub

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

Jump to bottom

Migrate MockWebServer to JUnit 5 #1817

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Migrate MockWebServer to JUnit 5 #1817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f7107a
Mirgrate from JUnit 4 to JUnit 5
schlosna Nov 11, 2022
d0a2d72
JUnit 5.9.1
schlosna Nov 11, 2022
9e12be5
Merge remote-tracking branch 'origin/develop' into ds/junit
schlosna Nov 11, 2022
0b9c346
MockWebServer JUnit 5
schlosna Nov 11, 2022
bb41646
Merge remote-tracking branch 'origin/develop' into ds/test-deps
schlosna Nov 11, 2022
16d4778
write-locks
schlosna Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dialogue-annotations-processor/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ dependencies {
testImplementation 'com.google.guava:guava'
testImplementation 'com.google.testing.compile:compile-testing'
testImplementation 'com.palantir.safe-logging:preconditions-assertj'
testImplementation 'com.squareup.okhttp3:mockwebserver'
testImplementation 'com.squareup.okhttp3:mockwebserver3-junit5'
testImplementation 'org.assertj:assertj-core'
testImplementation 'org.junit.jupiter:junit-jupiter'
}
9 changes: 8 additions & 1 deletion ...ache-hc5-client/src/test/java/com/palantir/dialogue/hc5/ApacheHttpClientChannelsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,17 @@
import java.util.Map;
import java.util.Optional;
import java.util.OptionalLong;
import okhttp3.mockwebserver.RecordedRequest;
import mockwebserver3.MockWebServer;
import mockwebserver3.RecordedRequest;
import mockwebserver3.junit5.internal.MockWebServerExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;

@ExtendWith(MockWebServerExtension.class)
public final class ApacheHttpClientChannelsTest extends AbstractChannelTest {
ApacheHttpClientChannelsTest(MockWebServer server) {
super(server);
}

@Override
protected Channel createChannel(ClientConfiguration config) {
Expand Down
8 changes: 8 additions & 0 deletions ...ogue-apache-hc5-client/src/test/java/com/palantir/dialogue/hc5/ApacheProxyConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,16 @@
import com.palantir.conjure.java.client.config.ClientConfiguration;
import com.palantir.dialogue.AbstractProxyConfigTest;
import com.palantir.dialogue.Channel;
import mockwebserver3.MockWebServer;
import mockwebserver3.junit5.internal.MockWebServerExtension;
import org.junit.jupiter.api.extension.ExtendWith;

@ExtendWith(MockWebServerExtension.class)
public final class ApacheProxyConfigTest extends AbstractProxyConfigTest {
ApacheProxyConfigTest(MockWebServer server, MockWebServer proxyServer) {
super(server, proxyServer);
}

@Override
protected Channel create(ClientConfiguration config) {
return ApacheHttpClientChannels.create(config);
Expand Down
3 changes: 1 addition & 2 deletions dialogue-test-common/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ dependencies {
api 'com.palantir.conjure.java.runtime:client-config'
api 'com.palantir.conjure.java.runtime:keystores'
api 'com.palantir.tracing:tracing-test-utils'
api 'com.squareup.okhttp3:mockwebserver'
api 'com.squareup.okhttp3:mockwebserver3-junit5'
api 'org.assertj:assertj-core'
api 'org.assertj:assertj-guava'
api 'org.junit.jupiter:junit-jupiter'
api 'org.junit.jupiter:junit-jupiter-migrationsupport'
api 'org.mockito:mockito-core'
api 'org.mockito:mockito-junit-jupiter'
api 'io.undertow:undertow-core'
Expand Down
21 changes: 9 additions & 12 deletions dialogue-test-common/src/main/java/com/palantir/dialogue/AbstractChannelTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package com.palantir.dialogue;

// CHECKSTYLE:OFF // static import

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
Expand All @@ -38,21 +36,17 @@
import java.util.concurrent.CancellationException;
import java.util.concurrent.ExecutionException;
import java.util.function.BiConsumer;
import mockwebserver3.MockResponse;
import mockwebserver3.MockWebServer;
import mockwebserver3.RecordedRequest;
import okhttp3.HttpUrl;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import okio.Buffer;
import okio.GzipSink;
import org.junit.Rule;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

// CHECKSTYLE:ON
@SuppressWarnings({"checkstyle:avoidstaticimport", "checkstyle:VisibilityModifier", "FutureReturnValueIgnored"})
@EnableRuleMigrationSupport
public abstract class AbstractChannelTest {

protected static final byte[] CONTENT = "test".getBytes(StandardCharsets.UTF_8);
Expand All @@ -63,9 +57,6 @@ private Channel createChannel(URL baseUrl) {
return createChannel(TestConfigurations.create(baseUrl.toString()));
}

@Rule
public final MockWebServer server = new MockWebServer();

protected final RequestBody body = new RequestBody() {
@Override
public void writeTo(OutputStream output) throws IOException {
Expand Down Expand Up @@ -97,6 +88,12 @@ public void close() {}

protected Channel channel;

protected final MockWebServer server;

protected AbstractChannelTest(MockWebServer server) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see any tests out side this repo that extend this class, so while this is technically an API break I think we're fine

this.server = server;
}

@BeforeEach
public void before() {
channel = createChannel(server.url("").url());
Expand Down
23 changes: 13 additions & 10 deletions dialogue-test-common/src/main/java/com/palantir/dialogue/AbstractProxyConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.List;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.Rule;
import mockwebserver3.MockResponse;
import mockwebserver3.MockWebServer;
import mockwebserver3.RecordedRequest;
import mockwebserver3.junit5.internal.MockWebServerExtension;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;
import org.junit.jupiter.api.extension.ExtendWith;

@EnableRuleMigrationSupport
@ExtendWith(MockWebServerExtension.class)
public abstract class AbstractProxyConfigTest {

private static final Request request = Request.builder()
Expand All @@ -62,11 +62,14 @@ public void close() {}
})
.build();

@Rule
public final MockWebServer server = new MockWebServer();
private final MockWebServer server;

@Rule
public final MockWebServer proxyServer = new MockWebServer();
private final MockWebServer proxyServer;

protected AbstractProxyConfigTest(MockWebServer server, MockWebServer proxyServer) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see any tests out side this repo that extend this class, so while this is technically an API break I think we're fine

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t believe we publish this module 👍

this.server = server;
this.proxyServer = proxyServer;
}

protected abstract Channel create(ClientConfiguration config);

Expand Down
25 changes: 12 additions & 13 deletions ...ogue-test-common/src/main/java/com/palantir/dialogue/AbstractSampleServiceClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

package com.palantir.dialogue;

// CHECKSTYLE:OFF // static import

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

Expand All @@ -37,18 +35,16 @@
import java.time.OffsetDateTime;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.Rule;
import mockwebserver3.MockResponse;
import mockwebserver3.MockWebServer;
import mockwebserver3.RecordedRequest;
import mockwebserver3.junit5.internal.MockWebServerExtension;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

// CHECKSTYLE:ON
import org.junit.jupiter.api.extension.ExtendWith;

@EnableRuleMigrationSupport
@ExtendWith(MockWebServerExtension.class)
public abstract class AbstractSampleServiceClientTest {

abstract SampleServiceBlocking createBlockingClient(URL baseUrl, Duration timeout);
Expand All @@ -67,8 +63,11 @@ public abstract class AbstractSampleServiceClientTest {
static final SslConfiguration SSL_CONFIG = SslConfiguration.of(
Paths.get("src/test/resources/trustStore.jks"), Paths.get("src/test/resources/keyStore.jks"), "keystore");

@Rule
public final MockWebServer server = new MockWebServer();
private final MockWebServer server;

protected AbstractSampleServiceClientTest(MockWebServer server) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see any tests out side this repo that extend this class, so while this is technically an API break I think we're fine

this.server = server;
}

private SampleServiceBlocking blockingClient;
private SampleServiceAsync asyncClient;
Expand Down Expand Up @@ -107,7 +106,7 @@ public abstract class AbstractSampleServiceClientTest {

@BeforeEach
public void before() {
server.useHttps(SslSocketFactories.createSslSocketFactory(SSL_CONFIG), false);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the removed argument was disabling tunnel proxy:

https://github.com/square/okhttp/blob/parent-4.10.0/mockwebserver/src/main/kotlin/okhttp3/mockwebserver/MockWebServer.kt#L276-L280

tunnelProxy true to expect the HTTP CONNECT method before negotiating TLS.

server.useHttps(SslSocketFactories.createSslSocketFactory(SSL_CONFIG));
blockingClient = createBlockingClient(server.url("").url(), Duration.ofSeconds(1));
asyncClient = createAsyncClient(server.url("").url(), Duration.ofSeconds(1));
}
Expand Down
21 changes: 13 additions & 8 deletions versions.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ org.glassfish.hk2.external:jakarta.inject:2.6.1 (1 constraints: f410fcc2)
org.glassfish.jersey.core:jersey-common:2.31 (1 constraints: da04fa30)
org.hdrhistogram:HdrHistogram:2.1.12 (1 constraints: 3e103aa2)
org.immutables:value:2.8.8 (1 constraints: 14051536)
org.jetbrains:annotations:23.0.0 (1 constraints: 31115ed1)
org.jetbrains:annotations:23.0.0 (2 constraints: 0f20e4ff)
org.mpierce.metrics.reservoir:hdrhistogram-metrics-reservoir:1.1.3 (1 constraints: 0d10f991)
org.slf4j:slf4j-api:1.7.36 (8 constraints: b280891b)

Expand All @@ -71,14 +71,16 @@ com.google.truth:truth:1.1 (1 constraints: b71111d7)
com.palantir.safe-logging:preconditions-assertj:3.2.0 (1 constraints: 07050036)
com.palantir.tracing:tracing-test-utils:6.15.0 (1 constraints: 3e05563b)
com.spotify.dataenum:dataenum:1.4.1 (1 constraints: e9105ac1)
com.squareup.okhttp3:mockwebserver:3.13.1 (1 constraints: 3a053f3b)
com.squareup.okhttp3:okhttp:3.13.1 (2 constraints: a014ba9d)
com.squareup.okio:okio:1.17.2 (1 constraints: 850cc309)
com.squareup.okhttp3:mockwebserver3:5.0.0-alpha.10 (1 constraints: b514caae)
com.squareup.okhttp3:mockwebserver3-junit5:5.0.0-alpha.10 (1 constraints: c9071c72)
com.squareup.okhttp3:okhttp:5.0.0-alpha.10 (1 constraints: 3a053f3b)
com.squareup.okhttp3:okhttp-jvm:5.0.0-alpha.10 (2 constraints: 392169c2)
com.squareup.okio:okio-jvm:3.2.0 (1 constraints: c90d9c38)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are all [Test dependencies] changes (below line 64)

commons-logging:commons-logging:1.2 (2 constraints: 8215ead1)
de.erichseifert.vectorgraphics2d:VectorGraphics2D:0.13 (1 constraints: 8c0a80bb)
de.rototor.pdfbox:graphics2d:0.25 (1 constraints: 8f0a84bb)
io.undertow:undertow-core:2.2.18.Final (1 constraints: 57074361)
junit:junit:4.13.2 (4 constraints: 2d413ec7)
junit:junit:4.13.1 (2 constraints: dc1cc51a)
net.bytebuddy:byte-buddy:1.12.16 (2 constraints: ef164566)
net.bytebuddy:byte-buddy-agent:1.12.16 (1 constraints: 750baee9)
net.jcip:jcip-annotations:1.0 (1 constraints: 560ff165)
Expand All @@ -91,7 +93,7 @@ org.apache.logging.log4j:log4j-core:2.19.0 (2 constraints: 15169e25)
org.apache.logging.log4j:log4j-slf4j-impl:2.19.0 (1 constraints: 3e054a3b)
org.apache.pdfbox:fontbox:2.0.17 (1 constraints: 180b71d8)
org.apache.pdfbox:pdfbox:2.0.17 (1 constraints: b40c5915)
org.apiguardian:apiguardian-api:1.1.2 (6 constraints: 24695e60)
org.apiguardian:apiguardian-api:1.1.2 (5 constraints: 105480ac)
org.assertj:assertj-core:3.23.1 (3 constraints: 5025194c)
org.assertj:assertj-guava:3.3.0 (1 constraints: 08050336)
org.awaitility:awaitility:4.2.0 (1 constraints: 08050536)
Expand All @@ -102,12 +104,15 @@ org.jboss.logging:jboss-logging:3.4.1.Final (3 constraints: f03036d7)
org.jboss.threads:jboss-threads:3.1.0.Final (2 constraints: 561a9b42)
org.jboss.xnio:xnio-api:3.8.7.Final (2 constraints: 771a3146)
org.jboss.xnio:xnio-nio:3.8.7.Final (1 constraints: c80dcb30)
org.jetbrains.kotlin:kotlin-stdlib:1.6.21 (3 constraints: bd2f9e82)
org.jetbrains.kotlin:kotlin-stdlib-common:1.6.21 (3 constraints: 3929d928)
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.6.21 (1 constraints: e110f6d2)
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.6.21 (3 constraints: bc2d5d81)
org.jmock:jmock:2.12.0 (1 constraints: 3705353b)
org.jmock:jmock-testjar:2.12.0 (1 constraints: a507a272)
org.junit.jupiter:junit-jupiter:5.9.1 (1 constraints: 11052036)
org.junit.jupiter:junit-jupiter-api:5.9.1 (6 constraints: 6e58d58f)
org.junit.jupiter:junit-jupiter-api:5.9.1 (6 constraints: 4b55e412)
org.junit.jupiter:junit-jupiter-engine:5.9.1 (1 constraints: 0c0ee13b)
org.junit.jupiter:junit-jupiter-migrationsupport:5.9.1 (1 constraints: 11052036)
org.junit.jupiter:junit-jupiter-params:5.9.1 (2 constraints: 1c13903c)
org.junit.platform:junit-platform-commons:1.9.1 (2 constraints: dd200f4b)
org.junit.platform:junit-platform-engine:1.9.1 (1 constraints: ab1029b4)
Expand Down
2 changes: 1 addition & 1 deletion versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ org.jmock:jmock = 2.12.0
org.knowm.xchart:xchart = 3.6.1
com.palantir.conjure.verification:* = 0.19.0
io.undertow:undertow-core = 2.2.18.Final
com.squareup.okhttp3:mockwebserver = 3.13.1
com.squareup.okhttp3:mockwebserver3-junit5 = 5.0.0-alpha.10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There hasn't been a non-alpha release of mockwebserver3-junit5 yet, but there have been many alpha releases over the past year and a half:

From https://search.maven.org/artifact/com.squareup.okhttp3/mockwebserver3-junit5

Version Release Date
5.0.0-alpha.10 27-Jun-2022
5.0.0-alpha.9 16-Jun-2022
5.0.0-alpha.8 08-Jun-2022
5.0.0-alpha.7 26-Apr-2022
5.0.0-alpha.6 14-Mar-2022
5.0.0-alpha.5 21-Feb-2022
5.0.0-alpha.4 01-Feb-2022
5.0.0-alpha.3 22-Nov-2021
5.0.0-alpha.2 30-Jan-2021

org.openjdk.jmh:* = 1.35

# dependency-upgrader:OFF
Expand Down