Skip to content

Commit

Permalink
Fix "assume" statements in tests (#1574)
Browse files Browse the repository at this point in the history
Motivation:

During migration to jUnit5 in #1521, some of the "assume" statements
were moved before the initialization of parameters. As the result,
those "assume" statements did not work.

Modifications:

- `NettyHttpServerTest`: move `ignoreTestWhen` after `setUp`;
- `ServiceTalkToNettyContentCodingCompatibilityTest` execute `start()`
after "assume" checks;
- Minor cosmetic changes in other test classes: make methods `static`,
use static import, etc.

Result:

`NettyHttpServerTest` is kept ignored for now.
  • Loading branch information
idelpivnitskiy authored May 25, 2021
1 parent cbfb48e commit b6f1e22
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public void timerDurationCancel(ExecutorParam executorParam) throws InterruptedE
timerCancel(executor.timer(ofNanos(1)));
}

private void timerCancel(Completable timer) throws InterruptedException {
private static void timerCancel(Completable timer) throws InterruptedException {
AtomicReference<Throwable> refCause = new AtomicReference<>();
CountDownLatch latch = new CountDownLatch(1);
toSource(timer.afterCancel(latch::countDown))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ void nullSetCookieValue(final HttpMetaData metaData, @SuppressWarnings("unused")
assertThrows(IllegalArgumentException.class, () -> metaData.addSetCookie("foo", null));
}

private HttpRequestMetaData assumeRequestMeta(final HttpMetaData metaData) {
private static HttpRequestMetaData assumeRequestMeta(final HttpMetaData metaData) {
assumeTrue(metaData instanceof HttpRequestMetaData, "Test not applicable for response.");
return (HttpRequestMetaData) metaData;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
import io.netty.handler.codec.http2.Http2Settings;
import io.netty.handler.codec.http2.Http2SettingsAckFrame;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -1285,7 +1284,7 @@ private static void fullDuplexModeWrite(StreamingHttpClient client,
void clientRespectsSettingsFrame(HttpTestExecutionStrategy strategy,
boolean h2PriorKnowledge) throws Exception {
setUp(strategy, h2PriorKnowledge);
Assumptions.assumeTrue(h2PriorKnowledge, "Only HTTP/2 supports SETTINGS frames");
assumeTrue(h2PriorKnowledge, "Only HTTP/2 supports SETTINGS frames");

int expectedMaxConcurrent = 1;
BlockingQueue<FilterableStreamingHttpConnection> connectionQueue = new LinkedBlockingQueue<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import io.servicetalk.transport.netty.internal.IoThreadFactory;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

Expand All @@ -31,6 +30,7 @@
import static io.servicetalk.transport.netty.NettyIoExecutors.createIoExecutor;
import static io.servicetalk.transport.netty.internal.AddressUtils.newSocketAddress;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

class HttpUdsTest {
private static IoExecutor ioExecutor;
Expand All @@ -47,7 +47,7 @@ static void afterClass() throws ExecutionException, InterruptedException {

@Test
void udsRoundTrip() throws Exception {
Assumptions.assumeTrue(ioExecutor.isUnixDomainSocketSupported());
assumeTrue(ioExecutor.isUnixDomainSocketSupported());
try (ServerContext serverContext = HttpServers.forAddress(newSocketAddress()).ioExecutor(ioExecutor)
.listenBlockingAndAwait((ctx, request, responseFactory) -> responseFactory.ok())) {
try (BlockingHttpClient client = HttpClients.forResolvedAddress(serverContext.listenAddress())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -541,15 +541,14 @@ void testSingleError(ExecutorSupplier clientExecutorSupplier,
@ParameterizedTest(name = "{displayName} [{index}] client={0} server={1}")
@MethodSource("clientExecutors")
void testErrorBeforeRead(ExecutorSupplier clientExecutorSupplier,
ExecutorSupplier serverExecutorSupplier)
throws Exception {
ExecutorSupplier serverExecutorSupplier) throws Exception {
setUp(clientExecutorSupplier, serverExecutorSupplier);
// Flaky test: https://github.com/apple/servicetalk/issues/245
ignoreTestWhen(IMMEDIATE, IMMEDIATE);
ignoreTestWhen(IMMEDIATE, CACHED);
ignoreTestWhen(CACHED, IMMEDIATE);
ignoreTestWhen(CACHED, CACHED);

setUp(clientExecutorSupplier, serverExecutorSupplier);

final StreamingHttpRequest request = reqRespFactory.newRequest(GET, SVC_ERROR_BEFORE_READ).payloadBody(
getChunkPublisherFromStrings("Goodbye", "cruel", "world!"));

Expand Down Expand Up @@ -577,10 +576,9 @@ void testErrorBeforeRead(ExecutorSupplier clientExecutorSupplier,
@ParameterizedTest(name = "{displayName} [{index}] client={0} server={1}")
@MethodSource("clientExecutors")
void testErrorDuringRead(ExecutorSupplier clientExecutorSupplier,
ExecutorSupplier serverExecutorSupplier)
throws Exception {
ignoreTestWhen(CACHED, IMMEDIATE);
ExecutorSupplier serverExecutorSupplier) throws Exception {
setUp(clientExecutorSupplier, serverExecutorSupplier);
ignoreTestWhen(CACHED, IMMEDIATE);
final StreamingHttpRequest request = reqRespFactory.newRequest(GET, SVC_ERROR_DURING_READ).payloadBody(
getChunkPublisherFromStrings("Goodbye", "cruel", "world!"));
final StreamingHttpResponse response = makeRequest(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ void blocking(final ParamsSupplier paramSupplier) throws Exception {
@MethodSource("params")
void blockingStreaming(final ParamsSupplier paramSupplier) throws Exception {
params = paramSupplier.newParams();
assert params != null;
assumeFalse(params.isNoOffloadsStrategy(), "Ignoring no-offloads strategy for blocking-streaming.");
BlockingHttpClient client = params.startBlockingStreaming();
client.request(client.get("/")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,12 @@ void start() throws Exception {

@AfterEach
void finish() throws Exception {
client.close();
serverContext.close();
if (client != null) {
client.close();
}
if (serverContext != null) {
serverContext.close();
}
}

BlockingHttpClient client() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,18 @@ void start() {
client = newServiceTalkClient(HostAndPort.of(serverAddress), scenario, errors);
}

@Override
@AfterEach
void finish() throws Exception {
serverAcceptorChannel.close().syncUninterruptibly();
serverEventLoopGroup.shutdownGracefully(0, 0, MILLISECONDS).syncUninterruptibly();
client.close();
if (serverAcceptorChannel != null) {
serverAcceptorChannel.close().syncUninterruptibly();
}
if (serverEventLoopGroup != null) {
serverEventLoopGroup.shutdownGracefully(0, 0, MILLISECONDS).syncUninterruptibly();
}
if (client != null) {
client.close();
}
}

private Channel newNettyServer() {
Expand Down Expand Up @@ -101,10 +108,10 @@ void testCompatibility(final HttpProtocol protocol, final Codings serverCodings,
final Codings clientCodings, final Compression compression,
final boolean valid) throws Throwable {
setUp(protocol, serverCodings, clientCodings, compression, valid);
start();
assumeFalse(scenario.protocol.version.equals(HTTP_2_0), "Only testing H1 scenarios yet.");
assumeTrue(scenario.valid, "Only testing successful configurations; Netty doesn't have knowledge " +
"about unsupported compression types.");
start();

if (scenario.valid) {
assertSuccessful(scenario.requestEncoding);
Expand Down

0 comments on commit b6f1e22

Please sign in to comment.