Skip to content

Commit

Permalink
Clean up all warnings in Java client test (#1213)
Browse files Browse the repository at this point in the history 
* Clean up all warnings in Java client test

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>

* Clean FfiTest.java of warnings

Signed-off-by: Andrew Carbonetto <[email protected]>

* Clean RedisClusterClientTest.java of compiler warnings

Signed-off-by: Andrew Carbonetto <[email protected]>

* Clean up more warnings in client tests

Signed-off-by: Andrew Carbonetto <[email protected]>

* Spotless

Signed-off-by: Andrew Carbonetto <[email protected]>

* Fix UT + update IT.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Update suppression.

Signed-off-by: Yury-Fridlyand <[email protected]>

* Remove suppress warnings

Signed-off-by: Andrew Carbonetto <[email protected]>

---------

Signed-off-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
  • Loading branch information
@acarbonetto @Yury-Fridlyand
acarbonetto and Yury-Fridlyand authored Apr 10, 2024
1 parent 527e1ec commit fec74b8
Show file tree
Hide file tree
Showing 9 changed files with 300 additions and 299 deletions.
287 changes: 143 additions & 144 deletions java/client/src/test/java/glide/api/RedisClientTest.java
View file

Large diffs are not rendered by default.

203 changes: 106 additions & 97 deletions java/client/src/test/java/glide/api/RedisClusterClientTest.java
View file

Large diffs are not rendered by default.

19 changes: 10 additions & 9 deletions java/client/src/test/java/glide/connection/ConnectionWithGlideMockTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,16 @@ public void rethrow_error_on_read_when_malformed_packet_received() {
@Test
@SneakyThrows
public void rethrow_error_if_UDS_channel_closed() {
var client = new TestClient(channelHandler);
stopRustCoreLibMock();
try {
var exception =
assertThrows(ExecutionException.class, () -> client.customCommand(new String[0]).get());
assertTrue(exception.getCause() instanceof ClosingException);
} finally {
// restart mock to let other tests pass if this one failed
startRustCoreLibMock(null);
try (var client = new TestClient(channelHandler)) {
stopRustCoreLibMock();
try {
var exception =
assertThrows(ExecutionException.class, () -> client.customCommand(new String[0]).get());
assertTrue(exception.getCause() instanceof ClosingException);
} finally {
// restart mock to let other tests pass if this one failed
startRustCoreLibMock(null);
}
}
}

Expand Down
24 changes: 10 additions & 14 deletions java/client/src/test/java/glide/connectors/resources/ThreadPoolResourceAllocatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.internal.verification.VerificationModeFactory.times;

import io.netty.channel.EventLoop;
import io.netty.channel.EventLoopGroup;
Expand All @@ -15,8 +13,6 @@

public class ThreadPoolResourceAllocatorTest {

ThreadPoolResourceAllocator service;

@BeforeEach
public void init() {
var threadPoolResource = ThreadPoolResourceAllocator.getOrCreate(() -> null);
Expand All @@ -30,19 +26,19 @@ public void init() {
public void getOrCreate_returns_default_after_repeated_calls() {
ThreadPoolResource mockedThreadPoolResource = mock(ThreadPoolResource.class);
EventLoopGroup mockedEventLoopGroup = mock(EventLoop.class);
Supplier<ThreadPoolResource> threadPoolSupplier = mock(Supplier.class);

Supplier<ThreadPoolResource> threadPoolSupplier = () -> mockedThreadPoolResource;

when(mockedThreadPoolResource.getEventLoopGroup()).thenReturn(mockedEventLoopGroup);
when(mockedEventLoopGroup.isShuttingDown()).thenReturn(false);
when(threadPoolSupplier.get()).thenReturn(mockedThreadPoolResource);

ThreadPoolResource theResource = service.getOrCreate(threadPoolSupplier);
ThreadPoolResource theResource = ThreadPoolResourceAllocator.getOrCreate(threadPoolSupplier);
assertEquals(mockedThreadPoolResource, theResource);

// Ensure that supplier only is invoked once to set up the shared resource
ThreadPoolResource theSameResource = service.getOrCreate(threadPoolSupplier);
ThreadPoolResource theSameResource =
ThreadPoolResourceAllocator.getOrCreate(threadPoolSupplier);
assertEquals(mockedThreadPoolResource, theSameResource);
verify(threadPoolSupplier, times(1)).get();

// teardown
when(mockedEventLoopGroup.isShuttingDown()).thenReturn(true);
Expand All @@ -52,19 +48,19 @@ public void getOrCreate_returns_default_after_repeated_calls() {
public void getOrCreate_returns_new_thread_pool_after_shutdown() {
ThreadPoolResource mockedThreadPoolResource = mock(ThreadPoolResource.class);
EventLoopGroup mockedEventLoopGroup = mock(EventLoop.class);
Supplier<ThreadPoolResource> threadPoolSupplier = mock(Supplier.class);

Supplier<ThreadPoolResource> threadPoolSupplier = () -> mockedThreadPoolResource;

when(mockedThreadPoolResource.getEventLoopGroup()).thenReturn(mockedEventLoopGroup);
when(mockedEventLoopGroup.isShuttingDown()).thenReturn(true);
when(threadPoolSupplier.get()).thenReturn(mockedThreadPoolResource);

ThreadPoolResource theResource = service.getOrCreate(threadPoolSupplier);
ThreadPoolResource theResource = ThreadPoolResourceAllocator.getOrCreate(threadPoolSupplier);
assertEquals(mockedThreadPoolResource, theResource);

// Ensure that supplier only is invoked once to set up the shared resource
ThreadPoolResource theSameResource = service.getOrCreate(threadPoolSupplier);
ThreadPoolResource theSameResource =
ThreadPoolResourceAllocator.getOrCreate(threadPoolSupplier);
assertEquals(mockedThreadPoolResource, theSameResource);
verify(threadPoolSupplier, times(2)).get();

// teardown
when(mockedEventLoopGroup.isShuttingDown()).thenReturn(true);
Expand Down
10 changes: 5 additions & 5 deletions java/client/src/test/java/glide/ffi/FfiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void redisValueToJavaValue_Okay() {
}

@ParameterizedTest
@ValueSource(longs = {0L, 100L, 774L, Integer.MAX_VALUE + 1, Integer.MIN_VALUE - 1})
@ValueSource(longs = {0L, 100L, 774L, Integer.MAX_VALUE + 1L, Integer.MIN_VALUE - 1L})
public void redisValueToJavaValue_Int(Long input) {
long ptr = FfiTest.createLeakedInt(input);
Object longValue = RedisValueResolver.valueFromPointer(ptr);
Expand Down Expand Up @@ -98,8 +98,8 @@ public void redisValueToJavaValue_Map() {
long[] values = {1L, 2L, 3L};
long ptr = FfiTest.createLeakedMap(keys, values);
Object mapValue = RedisValueResolver.valueFromPointer(ptr);
assertTrue(mapValue instanceof HashMap);
HashMap<Object, Object> result = (HashMap<Object, Object>) mapValue;
assertTrue(mapValue instanceof HashMap<?, ?>);
HashMap<?, ?> result = (HashMap<?, ?>) mapValue;
assertAll(
() -> assertEquals(1L, result.get(12L)),
() -> assertEquals(2L, result.get(14L)),
Expand Down Expand Up @@ -134,8 +134,8 @@ public void redisValueToJavaValue_Set() {
long[] array = {1L, 2L, 2L};
long ptr = FfiTest.createLeakedLongSet(array);
Object longSetValue = RedisValueResolver.valueFromPointer(ptr);
assertTrue(longSetValue instanceof HashSet);
HashSet<Long> result = (HashSet<Long>) longSetValue;
assertTrue(longSetValue instanceof HashSet<?>);
HashSet<?> result = (HashSet<?>) longSetValue;
assertAll(
() -> assertTrue(result.contains(1L)),
() -> assertTrue(result.contains(2L)),
Expand Down
20 changes: 10 additions & 10 deletions java/client/src/test/java/glide/managers/ConnectionManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,21 +40,21 @@ public class ConnectionManagerTest {

ChannelHandler channel;

private static String HOST = "aws.com";
private static int PORT = 9999;
private static final String HOST = "aws.com";
private static final int PORT = 9999;

private static String USERNAME = "JohnDoe";
private static String PASSWORD = "Password1";
private static final String USERNAME = "JohnDoe";
private static final String PASSWORD = "Password1";

private static int NUM_OF_RETRIES = 5;
private static int FACTOR = 10;
private static int EXPONENT_BASE = 50;
private static final int NUM_OF_RETRIES = 5;
private static final int FACTOR = 10;
private static final int EXPONENT_BASE = 50;

private static int DATABASE_ID = 1;
private static final int DATABASE_ID = 1;

private static int REQUEST_TIMEOUT = 3;
private static final int REQUEST_TIMEOUT = 3;

private static String CLIENT_NAME = "ClientName";
private static final String CLIENT_NAME = "ClientName";

@BeforeEach
public void setUp() {
Expand Down
17 changes: 6 additions & 11 deletions java/client/src/test/java/glide/utils/RustCoreMock.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.file.Files;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import lombok.NonNull;
import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import redis_request.RedisRequestOuterClass.RedisRequest;
Expand Down Expand Up @@ -67,13 +68,6 @@ public static Response.Builder OK() {
}
}

public abstract static class GlideMockConnectAll extends GlideMockProtobuf {
@Override
public Response connection(ConnectionRequest request) {
return Response.newBuilder().build();
}
}

/** Thread pool supplied to <em>Netty</em> to perform all async IO. */
private final EventLoopGroup group;

Expand Down Expand Up @@ -113,7 +107,7 @@ private RustCoreMock() {
new ChannelInitializer<DomainSocketChannel>() {

@Override
protected void initChannel(DomainSocketChannel ch) throws Exception {
protected void initChannel(@NonNull DomainSocketChannel ch) {
ch.pipeline()
// https://netty.io/4.1/api/io/netty/handler/codec/protobuf/ProtobufEncoder.html
.addLast("frameDecoder", new ProtobufVarint32FrameDecoder())
Expand Down Expand Up @@ -155,7 +149,8 @@ private class UdsServer extends ChannelInboundHandlerAdapter {
private final AtomicBoolean anybodyConnected = new AtomicBoolean(false);

@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
public void channelRead(@NonNull ChannelHandlerContext ctx, @NonNull Object msg)
throws Exception {
var buf = (ByteBuf) msg;
var bytes = new byte[buf.readableBytes()];
buf.readBytes(bytes);
Expand All @@ -165,7 +160,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
return;
}
var handler = (GlideMockProtobuf) messageProcessor;
Response response = null;
Response response;
if (!anybodyConnected.get()) {
var connection = ConnectionRequest.parseFrom(bytes);
response = handler.connection(connection);
Expand All @@ -180,7 +175,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
}

@Override
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
cause.printStackTrace();
ctx.close();
failed.setPlain(true);
Expand Down
3 changes: 2 additions & 1 deletion java/integTest/src/test/java/glide/SharedClientTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ private static Stream<Arguments> clientAndDataSize() {
@MethodSource("clientAndDataSize")
public void client_can_handle_concurrent_workload(BaseClient client, int valueSize) {
ExecutorService executorService = Executors.newCachedThreadPool();
CompletableFuture[] futures = new CompletableFuture[100];
@SuppressWarnings("unchecked")
CompletableFuture<Void>[] futures = new CompletableFuture[100];

for (int i = 0; i < 100; i++) {
futures[i] =
Expand Down
16 changes: 8 additions & 8 deletions java/integTest/src/test/java/glide/SharedCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import lombok.SneakyThrows;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
Expand Down Expand Up @@ -1127,7 +1128,7 @@ public void zadd_and_zaddIncr_with_NX_XX(BaseClient client) {

assertEquals(0, client.zadd(key, membersScores, onlyIfExistsOptions).get());
assertEquals(3, client.zadd(key, membersScores, onlyIfDoesNotExistOptions).get());
assertEquals(null, client.zaddIncr(key, "one", 5, onlyIfDoesNotExistOptions).get());
assertNull(client.zaddIncr(key, "one", 5, onlyIfDoesNotExistOptions).get());
assertEquals(6, client.zaddIncr(key, "one", 5, onlyIfExistsOptions).get());
}

Expand Down Expand Up @@ -1156,25 +1157,24 @@ public void zadd_and_zaddIncr_with_GT_LT(BaseClient client) {
assertEquals(1, client.zadd(key, membersScores, scoreGreaterThanOptions, true).get());
assertEquals(0, client.zadd(key, membersScores, scoreLessThanOptions, true).get());
assertEquals(7, client.zaddIncr(key, "one", -3, scoreLessThanOptions).get());
assertEquals(null, client.zaddIncr(key, "one", -3, scoreGreaterThanOptions).get());
assertNull(client.zaddIncr(key, "one", -3, scoreGreaterThanOptions).get());
}

@SneakyThrows
@ParameterizedTest
@MethodSource("getClients")
public void zadd_illegal_arguments(BaseClient client) {
// TODO move to another class
@Test
public void zadd_illegal_arguments() {
ZaddOptions existsGreaterThanOptions =
ZaddOptions.builder()
.conditionalChange(ZaddOptions.ConditionalChange.ONLY_IF_DOES_NOT_EXIST)
.updateOptions(ZaddOptions.UpdateOptions.SCORE_GREATER_THAN_CURRENT)
.build();
assertThrows(IllegalArgumentException.class, () -> existsGreaterThanOptions.toArgs());
assertThrows(IllegalArgumentException.class, existsGreaterThanOptions::toArgs);
ZaddOptions existsLessThanOptions =
ZaddOptions.builder()
.conditionalChange(ZaddOptions.ConditionalChange.ONLY_IF_DOES_NOT_EXIST)
.updateOptions(ZaddOptions.UpdateOptions.SCORE_LESS_THAN_CURRENT)
.build();
assertThrows(IllegalArgumentException.class, () -> existsLessThanOptions.toArgs());
assertThrows(IllegalArgumentException.class, existsLessThanOptions::toArgs);
ZaddOptions options =
ZaddOptions.builder()
.conditionalChange(ZaddOptions.ConditionalChange.ONLY_IF_DOES_NOT_EXIST)
Expand Down

0 comments on commit fec74b8

Please sign in to comment.