From 38052e77b710a0fa373483bc6e55d68c26e6ec38 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Tue, 24 Sep 2019 13:43:33 +0200 Subject: [PATCH] Fix bug in WebClientDataBufferAllocatingTests Prior to this commit, the parameterized DataBufferFactory was never actually used when setting up the WebClient for each test. This was due to an oversight when migrating from JUnit 4 to JUnit Jupiter. See: https://github.com/reactor/reactor-netty/issues/860 This commit fixes this by converting the existing @BeforeEach method to a local setup method that is invoked from each @ParameterizedDataBufferAllocatingTest method. In addition, in order to avoid the 2 second "quiet period" that is incurred when destroying the ReactorResourceFactory, this commit moves the setup and destruction of the ReactorResourceFactory to new @BeforeAll and @AfterAll methods. The test instance lifecycle has also been switched to PER_CLASS to avoid static state in the test class. --- .../WebClientDataBufferAllocatingTests.java | 76 +++++++++++-------- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java index af8277532b..21d1f44734 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java @@ -25,8 +25,10 @@ import io.netty.buffer.ByteBufAllocator; import io.netty.channel.ChannelOption; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.TestInstance; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -42,6 +44,7 @@ import org.springframework.http.client.reactive.ReactorResourceFactory; import org.springframework.web.reactive.function.UnsupportedMediaTypeException; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; /** * WebClient integration tests focusing on data buffer management. @@ -49,23 +52,29 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Rossen Stoyanchev * @author Sam Brannen */ -public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAllocatingTests { +@TestInstance(PER_CLASS) +class WebClientDataBufferAllocatingTests extends AbstractDataBufferAllocatingTests { private static final Duration DELAY = Duration.ofSeconds(5); - private final ReactorResourceFactory factory = new ReactorResourceFactory(); - - private final MockWebServer server = new MockWebServer(); - + private MockWebServer server; private WebClient webClient; - @BeforeEach - public void setUp() { - this.factory.setUseGlobalResources(false); - this.factory.afterPropertiesSet(); + @BeforeAll + void setUpReactorResourceFactory() { + factory.afterPropertiesSet(); + } + @AfterAll + void destroyReactorResourceFactory() { + factory.destroy(); + } + + private void setUp(DataBufferFactory bufferFactory) { + super.bufferFactory = bufferFactory; + this.server = new MockWebServer(); this.webClient = WebClient .builder() .clientConnector(initConnector()) @@ -74,9 +83,11 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } private ReactorClientHttpConnector initConnector() { - if (bufferFactory instanceof NettyDataBufferFactory) { - ByteBufAllocator allocator = ((NettyDataBufferFactory) bufferFactory).getByteBufAllocator(); - return new ReactorClientHttpConnector(this.factory, httpClient -> + assertThat(super.bufferFactory).isNotNull(); + + if (super.bufferFactory instanceof NettyDataBufferFactory) { + ByteBufAllocator allocator = ((NettyDataBufferFactory) super.bufferFactory).getByteBufAllocator(); + return new ReactorClientHttpConnector(factory, httpClient -> httpClient.tcpConfiguration(tcpClient -> tcpClient.option(ChannelOption.ALLOCATOR, allocator))); } else { @@ -85,15 +96,14 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } @AfterEach - public void shutDown() throws InterruptedException { + void shutDown() throws InterruptedException { waitForDataBufferRelease(Duration.ofSeconds(2)); - this.factory.destroy(); } @ParameterizedDataBufferAllocatingTest - public void bodyToMonoVoid(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void bodyToMonoVoid(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); this.server.enqueue(new MockResponse() .setResponseCode(201) @@ -110,8 +120,8 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } @ParameterizedDataBufferAllocatingTest // SPR-17482 - public void bodyToMonoVoidWithoutContentType(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void bodyToMonoVoidWithoutContentType(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); this.server.enqueue(new MockResponse() .setResponseCode(HttpStatus.ACCEPTED.value()) @@ -127,40 +137,40 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } @ParameterizedDataBufferAllocatingTest - public void onStatusWithBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void onStatusWithBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); RuntimeException ex = new RuntimeException("response error"); testOnStatus(ex, response -> Mono.just(ex)); } @ParameterizedDataBufferAllocatingTest - public void onStatusWithBodyConsumed(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void onStatusWithBodyConsumed(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); RuntimeException ex = new RuntimeException("response error"); testOnStatus(ex, response -> response.bodyToMono(Void.class).thenReturn(ex)); } @ParameterizedDataBufferAllocatingTest // SPR-17473 - public void onStatusWithMonoErrorAndBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void onStatusWithMonoErrorAndBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); RuntimeException ex = new RuntimeException("response error"); testOnStatus(ex, response -> Mono.error(ex)); } @ParameterizedDataBufferAllocatingTest - public void onStatusWithMonoErrorAndBodyConsumed(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void onStatusWithMonoErrorAndBodyConsumed(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); RuntimeException ex = new RuntimeException("response error"); testOnStatus(ex, response -> response.bodyToMono(Void.class).then(Mono.error(ex))); } @ParameterizedDataBufferAllocatingTest // gh-23230 - public void onStatusWithImmediateErrorAndBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void onStatusWithImmediateErrorAndBodyNotConsumed(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); RuntimeException ex = new RuntimeException("response error"); testOnStatus(ex, response -> { @@ -169,8 +179,8 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } @ParameterizedDataBufferAllocatingTest - public void releaseBody(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void releaseBody(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); this.server.enqueue(new MockResponse() .setResponseCode(200) @@ -188,8 +198,8 @@ public class WebClientDataBufferAllocatingTests extends AbstractDataBufferAlloca } @ParameterizedDataBufferAllocatingTest - public void exchangeToBodilessEntity(String displayName, DataBufferFactory bufferFactory) { - super.bufferFactory = bufferFactory; + void exchangeToBodilessEntity(String displayName, DataBufferFactory bufferFactory) { + setUp(bufferFactory); this.server.enqueue(new MockResponse() .setResponseCode(201)