Skip to content

Commit

Permalink
Add RecordedRequests testing utility class (#499)
Browse files Browse the repository at this point in the history
* Add test utility class for getting RecordedRequests from an OkHttp
  MockWebServer.
* Because XmlUnit does some shenanigans when JUnit 4 is in the class
  path, several tests in KiwiXmlAssertTest started failing, because they were
  getting a JUnit 4 ComparisonFailure (which is a subclass of AssertionError).
  So, I changed the assertions to only check that an instance of AssertionError
  was thrown. See below for the Gory Details.

Gory Details:

okhttp3.mockwebserver version 4.12.0 depends on JUnit 4 (and
MockWebServer actually extends ExternalResource). This caused some
tests in KiwiXmlAssertTest to fail because they were expecting errors to
be exactly instance of AssertionError. But xmlunit-assertj (deep inside
org.xmlunit.assertj.error.ComparisonFailureErrorFactory) actually
changes the error it throws to be an org,junit.ComparisonFailure if that class
is available at runtime, whereas it simply delegates to AssertJ and throws
a regular AssertionError if it is not available. So, by mockwebserver
depending on JUnit 4.x, it makes ComparisonFailure available on the
class path, so the throwable changes to ComparisonFailure instead of
a raw AssertionError. ComparisonFailure is a subclass of AssertionError
so we can fix it by changing our assertions in KiwiXmlAssertTest to
only check that a Throwable is an instance of (not exactly)
AssertionError, so it will pass whether ComparisonFailure exists or not.

I don't know for sure, but am guessing they have to do it this way
for JUnit 4.x environments to work properly. But I'm not at all sure.

Closes #492
  • Loading branch information
sleberknight authored Jun 20, 2024
1 parent 7064ab2 commit 96b785a
Show file tree
Hide file tree
Showing 4 changed files with 290 additions and 6 deletions.
11 changes: 11 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@

<!-- Versions for provided dependencies -->
<jakarta.persistence-api.version>3.2.0</jakarta.persistence-api.version>

<!-- This version must match the okhttp version in kiwi-bom. TODO: Consider adding this to kiwi-bom. -->
<ohttp3.mockwebserver.version>4.12.0</ohttp3.mockwebserver.version>

<xmlunit.version>2.10.0</xmlunit.version>

<!-- Sonar properties -->
Expand Down Expand Up @@ -189,6 +193,13 @@
<scope>provided</scope>
</dependency>

<dependency>
<groupId>com.squareup.okhttp3</groupId>
<artifactId>mockwebserver</artifactId>
<version>${ohttp3.mockwebserver.version}</version>
<scope>provided</scope>
</dependency>

<dependency>
<groupId>org.liquibase</groupId>
<artifactId>liquibase-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package org.kiwiproject.test.okhttp3.mockwebserver;

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

import lombok.experimental.UtilityClass;
import lombok.extern.slf4j.Slf4j;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.kiwiproject.base.UncheckedInterruptedException;

import java.util.Optional;
import java.util.concurrent.TimeUnit;

/**
* Contains test assertions for {@link RecordedRequest} when using {@link MockWebServer}.
*/
@UtilityClass
@Slf4j
public class RecordedRequests {

/**
* Get the next available {@link RecordedRequest} or throw an exception.
*
* @param mockWebServer the {@link MockWebServer} expected to contain the recorded request
* @return the next available {@link RecordedRequest}
* @throws IllegalStateException if there isn't an available {@link RecordedRequest}
*/
public static RecordedRequest takeRequiredRequest(MockWebServer mockWebServer) {
return takeRequestOrEmpty(mockWebServer)
.orElseThrow(() -> new IllegalStateException("no request is currently available"));
}

/**
* Get the next available {@link RecordedRequest} or return an empty Optional.
*
* @param mockWebServer the {@link MockWebServer} which may contain the recorded request
* @return Optional containing the next available {@link RecordedRequest}, or an empty Optional
*/
public static Optional<RecordedRequest> takeRequestOrEmpty(MockWebServer mockWebServer) {
return Optional.ofNullable(takeRequestOrNull(mockWebServer));
}

/**
* Assert there are not any available {@link RecordedRequest} instances.
*
* @param mockWebServer the {@link MockWebServer} to verify
*/
public static void assertNoMoreRequests(MockWebServer mockWebServer) {
assertThat(takeRequestOrNull(mockWebServer))
.describedAs("There should not be any more requests, but (at least) one was found")
.isNull();
}

/**
* Get the next available {@link RecordedRequest} or return {@code null}.
* <p>
* Unlike {@link MockWebServer#takeRequest()}, does not block. Instead, waits
* a brief amount of time (10 milliseconds) for the next request. And unlike
* {@link MockWebServer#takeRequest(long, TimeUnit)}, converts a checked
* InterruptedException to an {@link UncheckedInterruptedException} so that
* tests don't need to constantly declare "throws" clauses.
*
* @param mockWebServer the {@link MockWebServer} which may contain the recorded request
* @return the next available {@link RecordedRequest}, or null if not available
* @throws UncheckedInterruptedException if the call to get the next request throws InterruptedException
*/
public static RecordedRequest takeRequestOrNull(MockWebServer mockWebServer) {
try {
return mockWebServer.takeRequest(10, TimeUnit.MILLISECONDS);
} catch (InterruptedException e) {
LOG.info("Interrupted waiting to get next request", e);
Thread.currentThread().interrupt();
throw new UncheckedInterruptedException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package org.kiwiproject.test.okhttp3.mockwebserver;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.kiwiproject.test.assertj.KiwiAssertJ.assertPresentAndGet;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.only;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.apache.commons.lang3.RandomStringUtils;
import org.awaitility.Durations;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.kiwiproject.base.UncheckedInterruptedException;
import org.kiwiproject.io.KiwiIO;

import java.io.IOException;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse.BodyHandlers;
import java.time.Duration;
import java.util.concurrent.Callable;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

@DisplayName("RecordedRequests")
class RecordedRequestsTest {

private MockWebServer server;

@BeforeEach
void setUp() throws IOException {
server = new MockWebServer();
server.start();
}

@AfterEach
void tearDown() {
KiwiIO.closeQuietly(server);
}

@Nested
class TakeRequiredRequest {

@Test
void shouldReturnTheAvailableRequest() {
server.enqueue(new MockResponse().setResponseCode(200));

var path = randomPath();
makeRequest(path);

var recordedRequest = RecordedRequests.takeRequiredRequest(server);

assertRequest(recordedRequest, path);
}

@Test
void shouldThrowIllegalState_WhenNoRequestIsAvailable() {
assertThatIllegalStateException()
.isThrownBy(() -> RecordedRequests.takeRequiredRequest(server))
.withMessage("no request is currently available");
}
}

@Nested
class TakeRequestOrEmpty {

@Test
void shouldReturnTheAvailableRequest() {
server.enqueue(new MockResponse().setResponseCode(202));

var path = randomPath();
makeRequest(path);

var recordedRequestOpt = RecordedRequests.takeRequestOrEmpty(server);

var recordedRequest = assertPresentAndGet(recordedRequestOpt);
assertRequest(recordedRequest, path);
}

@Test
void shouldReturnEmptyOptional_WhenNoRequestIsAvailable() {
assertThat(RecordedRequests.takeRequestOrEmpty(server)).isEmpty();
}
}

@Nested
class AssertNoMoreRequests {

@Test
void shouldPass_WhenThereIsNoRequestAvailable() {
assertThatCode(() -> RecordedRequests.assertNoMoreRequests(server))
.doesNotThrowAnyException();
}

@Test
void shouldFail_WhenAnyRequestIsAvailable() {
server.enqueue(new MockResponse().setResponseCode(202));

var path = randomPath();
makeRequest(path);

assertThatThrownBy(() -> RecordedRequests.assertNoMoreRequests(server))
.isNotNull()
.hasMessageContaining(
"There should not be any more requests, but (at least) one was found");
}
}

@Nested
class TakeRequestOrNull {

@Test
void shouldReturnTheAvailableRequest() {
server.enqueue(new MockResponse().setResponseCode(204));

var path = randomPath();
makeRequest(path);

var recordedRequestOpt = RecordedRequests.takeRequestOrEmpty(server);

var recordedRequest = assertPresentAndGet(recordedRequestOpt);
assertRequest(recordedRequest, path);
}

@Test
void shouldReturnNull_WhenNoRequestIsAvailable() {
assertThat(RecordedRequests.takeRequestOrNull(server)).isNull();
}

@Test
void shouldThrowUncheckedInterruptedException_IfInterruptedExceptionIsThrown() throws InterruptedException {
var mockMockWebServer = mock(MockWebServer.class);
when(mockMockWebServer.takeRequest(anyLong(), any(TimeUnit.class)))
.thenThrow(new InterruptedException("I interrupt you!"));

// Execute in separate thread so that the "Thread.currentThread().interrupt()" call
// does not interrupt the test thread.
var executor = Executors.newSingleThreadExecutor();
try {
Callable<RecordedRequest> callable = () -> RecordedRequests.takeRequestOrNull(mockMockWebServer);
var future = executor.submit(callable);
await().atMost(Durations.FIVE_HUNDRED_MILLISECONDS).until(future::isDone);

assertThatThrownBy(future::get)
.cause()
.isExactlyInstanceOf(UncheckedInterruptedException.class)
.hasMessageEndingWith("I interrupt you!");

verify(mockMockWebServer, only()).takeRequest(10, TimeUnit.MILLISECONDS);
} finally {
executor.shutdownNow();
}
}
}

private static String randomPath() {
return "/" + RandomStringUtils.randomAlphabetic(10);
}

private void makeRequest(String path) {
var httpClient = HttpClient.newBuilder()
.connectTimeout(Duration.ofMillis(100))
.build();

var url = server.url(path).toString();
var uri = URI.create(url);

try {
var request = HttpRequest.newBuilder().GET().uri(uri).build();
httpClient.send(request, BodyHandlers.ofString());
} catch (IOException | InterruptedException e) {
throw new RuntimeException(e);
}
}

private static void assertRequest(RecordedRequest request, String expectedPath) {
assertAll(
() -> assertThat(request.getMethod()).isEqualTo("GET"),
() -> assertThat(request.getPath()).isEqualTo(expectedPath)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void shouldAcceptTestNameAsString() {
.withTestName("custom-test-name")
.and(otherXml)
.areIdentical())
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}

@Test
Expand All @@ -63,7 +63,7 @@ void shouldAcceptTestNameFromTestInfo(TestInfo testInfo) {
.withTestNameFrom(testInfo)
.and(otherXml)
.areIdentical())
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}
}

Expand Down Expand Up @@ -108,7 +108,7 @@ void shouldThrowAssertionErrorWhenXmlIsDifferent() {

assertThatThrownBy(() ->
KiwiXmlAssert.assertThat(xml).isIdenticalTo(otherXml))
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}
}

Expand Down Expand Up @@ -136,7 +136,7 @@ void shouldThrowAssertionErrorWhenXmlIsDifferent() {

assertThatThrownBy(() ->
KiwiXmlAssert.assertThat(xml).isIdenticalToIgnoringWhitespace(otherXml))
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}
}

Expand Down Expand Up @@ -165,7 +165,7 @@ void canaryDoesNotIgnoreCommentsBetweenTags() {

assertThatThrownBy(() ->
KiwiXmlAssert.assertThat(xml).isIdenticalToIgnoringComments(otherXml))
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}
}

Expand All @@ -189,7 +189,7 @@ void shouldThrowAssertionErrorWhenXmlIsDifferent() {

assertThatThrownBy(() ->
KiwiXmlAssert.assertThat(xml).isIdenticalToIgnoringWhitespaceAndComments(otherXml))
.isExactlyInstanceOf(AssertionError.class);
.isInstanceOf(AssertionError.class);
}
}
}
Expand Down

0 comments on commit 96b785a

Please sign in to comment.