convert redisscala tests from groovy to java #12553
Conversation
| static void setUp() throws Exception { | ||
| redisServer = new GenericContainer<>("redis:6.2.3-alpine").withExposedPorts(6379); | ||
| redisServer.start(); | ||
| // cleanup.deferCleanup(redisServer::stop); |
| host = redisServer.getHost(); | ||
| port = redisServer.getMappedPort(6379); |
|
|
||
| @AfterAll | ||
| static void tearDown() throws Exception { | ||
| if (system != null) { |
There was a problem hiding this comment.
groovy version also calls redisServer.stop()
| import scala.Option; | ||
| import scala.concurrent.Future; | ||
|
|
||
| public class RediscalaClientTest { |
There was a problem hiding this comment.
usually we make test classes package private
shalk
force-pushed
the
test-rediscala
branch
from
0084ca7
to
b9c424c
Compare
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.extension.RegisterExtension; | ||
| import org.testcontainers.containers.GenericContainer; | ||
| import org.testcontainers.shaded.org.apache.commons.lang3.tuple.Pair; |
There was a problem hiding this comment.
we avoid using classes shaded into other libraries
| .hasKind(CLIENT) | ||
| .hasParent(trace.getSpan(0)) | ||
| .hasAttributesSatisfyingExactly( | ||
| equalTo(DbIncubatingAttributes.DB_SYSTEM, "redis"), |
There was a problem hiding this comment.
could you add a static import for DB_SYSTEM
There was a problem hiding this comment.
also could use DbIncubatingAttributes .DbIncubatingAttributes.REDIS
| return Pair.of(writeFuture, valueFuture); | ||
| }); | ||
|
|
||
| await().atMost(java.time.Duration.ofSeconds(3)).until(() -> result.getLeft().isCompleted()); |
There was a problem hiding this comment.
could import java.time.Duration
| false, | ||
| false, | ||
| serializer)); | ||
| await().atMost(java.time.Duration.ofSeconds(3)).until(value::isCompleted); |
There was a problem hiding this comment.
just wondering whether it would be better to use scala await class like the groovy test does or maybe even consider converting the whole test to scala
There was a problem hiding this comment.
i think java class is enough to cover the instrument case
| SettableFuture<Future<Object>> writeFutureRef = SettableFuture.create(); | ||
| ; |
There was a problem hiding this comment.
| SettableFuture<Future<Object>> writeFutureRef = SettableFuture.create(); | |
| ; | |
| SettableFuture<Future<Object>> writeFutureRef = SettableFuture.create(); |
#7195