Repository: fppt/jedis-mock
Pull request #90 (2020-05-05 16:29:23)

testcontainers for testing and jedis upgrade
I replaced embedded redis with testcontainers setup. This has the advantage that the testing does not rely on the operating system any more, only on docker.

I disabled two test with a comment.

inponomarev: Thanks @mwiede  for your great contribution. This project needed TestContainers, we can now compare the mock behaviour with fresh versions of Redis. Sorry for such a long wait for review and merge :-)

Patch:
diff --git a/pom.xml b/pom.xml
index 3d0f6cd..fc05f6a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
 
     <parent>
@@ -39,8 +40,8 @@
         <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
         <guava.version>19.0</guava.version>
         <junit.version>5.5.2</junit.version>
-        <redis.version>2.9.0</redis.version>
-        <redis-embedded.version>0.6</redis-embedded.version>
+        <jedis.version>3.6.0</jedis.version>
+        <testcontainers.version>1.15.3</testcontainers.version>
         <findbugs.version>3.0.1</findbugs.version>
         <slf4j.version>1.7.20</slf4j.version>
         <auto-value.version>1.6.2</auto-value.version>
@@ -73,14 +74,20 @@
         <dependency>
             <groupId>redis.clients</groupId>
             <artifactId>jedis</artifactId>
-            <version>${redis.version}</version>
+            <version>${jedis.version}</version>
             <scope>test</scope>
         </dependency>
-        <!-- Used for creating an embedded Redis for testing and comparing behaviour-->
+        <!-- Used for creating an testcontainer of Redis for testing and comparing behaviour-->
         <dependency>
-            <groupId>com.github.kstyrc</groupId>
-            <artifactId>embedded-redis</artifactId>
-            <version>${redis-embedded.version}</version>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>testcontainers</artifactId>
+            <version>${testcontainers.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>${testcontainers.version}</version>
             <scope>test</scope>
         </dependency>
         <!-- Used for finding common bugs in java projects -->
diff --git a/src/main/java/com/github/fppt/jedismock/operations/RO_hmset.java b/src/main/java/com/github/fppt/jedismock/operations/RO_hmset.java
index eef7708..42a8ca6 100644
--- a/src/main/java/com/github/fppt/jedismock/operations/RO_hmset.java
+++ b/src/main/java/com/github/fppt/jedismock/operations/RO_hmset.java
@@ -6,20 +6,14 @@ import com.github.fppt.jedismock.storage.RedisBase;
 
 import java.util.List;
 
-public class RO_hmset extends AbstractRedisOperation {
+public class RO_hmset extends RO_hset {
     public RO_hmset(RedisBase base, List<Slice> params) {
         super(base, params);
     }
 
     @Override
     Slice response() {
-        Slice hash = params().get(0);
-
-        for(int i = 1; i < params().size(); i = i + 2){
-            Slice field = params().get(i);
-            Slice value = params().get(i+1);
-            base().putValue(hash, field, value, -1L);
-        }
+        super.response();
 
         return Response.OK;
     }
diff --git a/src/main/java/com/github/fppt/jedismock/operations/RO_hset.java b/src/main/java/com/github/fppt/jedismock/operations/RO_hset.java
index 9fe567c..9c5660b 100644
--- a/src/main/java/com/github/fppt/jedismock/operations/RO_hset.java
+++ b/src/main/java/com/github/fppt/jedismock/operations/RO_hset.java
@@ -17,16 +17,21 @@ class RO_hset extends AbstractRedisOperation {
         return foundValue;
     }
 
+    @Override
     Slice response() {
-        Slice key1 = params().get(0);
-        Slice key2 = params().get(1);
-        Slice value = params().get(2);
-        Slice oldValue = hsetValue(key1, key2, value);
-
-        if(oldValue == null){
-            return Response.integer(1);
-        } else {
-            return Response.integer(0);
+        Slice hash = params().get(0);
+        int count = 0;
+
+        for(int i = 1; i < params().size(); i = i + 2){
+            Slice field = params().get(i);
+            Slice value = params().get(i+1);
+            Slice oldValue = hsetValue(hash, field, value);
+
+            if(oldValue == null) {
+                count++;
+            }
         }
+
+        return Response.integer(count);
     }
 }


Test patch:
diff --git a/src/test/java/com/github/fppt/jedismock/TestRedisOperationExecutor.java b/src/test/java/com/github/fppt/jedismock/TestRedisOperationExecutor.java
index ce43c0e..6ef7af8 100644
--- a/src/test/java/com/github/fppt/jedismock/TestRedisOperationExecutor.java
+++ b/src/test/java/com/github/fppt/jedismock/TestRedisOperationExecutor.java
@@ -285,6 +285,15 @@ public class TestRedisOperationExecutor {
         assertCommandArrayEquals(nullArray(), array("hmget", "h", "a"));
     }
 
+    @Test
+    public void testHset() {
+        assertCommandEquals(2, array("hset", "h", "a", "v1", "b", "v2"));
+        assertCommandEquals(1, array("hset", "h", "a", "v1", "c", "v3"));
+        assertCommandArrayEquals(array("v1"), array("hmget", "h", "a"));
+        assertCommandArrayEquals(array("v2"), array("hmget", "h", "b"));
+        assertCommandArrayEquals(array("v3"), array("hmget", "h", "c"));
+    }
+
     @Test
     public void testStrlen() throws ParseErrorException {
         assertCommandEquals(0, array("strlen", "a"));
diff --git a/src/test/java/com/github/fppt/jedismock/comparisontests/AdvanceOperationsTest.java b/src/test/java/com/github/fppt/jedismock/comparisontests/AdvanceOperationsTest.java
index a8afea6..3afff26 100644
--- a/src/test/java/com/github/fppt/jedismock/comparisontests/AdvanceOperationsTest.java
+++ b/src/test/java/com/github/fppt/jedismock/comparisontests/AdvanceOperationsTest.java
@@ -12,10 +12,7 @@ import redis.clients.jedis.Transaction;
 import redis.clients.jedis.exceptions.JedisDataException;
 
 import java.util.List;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
+import java.util.concurrent.*;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -43,7 +40,7 @@ public class AdvanceOperationsTest {
 
         //Start transaction
         jedis.multi();
-        assertEquals("Cannot use Jedis when in Multi. Please use Transation or reset jedis state.",
+        assertEquals("Cannot use Jedis when in Multi. Please use Transaction or reset jedis state.",
                 assertThrows(JedisDataException.class, () ->
                         jedis.get("oobity-oobity-boo")).getMessage());
     }
@@ -63,13 +60,13 @@ public class AdvanceOperationsTest {
         subsciberThread.submit(() -> subscriber.subscribe(mockSubscriber, channel));
 
         //Give some time to subscribe
-        Thread.sleep(50);
+        Thread.sleep(100);
 
         //publish message
         jedis.publish(channel, message);
 
         //Give some time for the message to go through
-        Thread.sleep(50);
+        Thread.sleep(100);
 
         assertEquals(channel, mockSubscriber.latestChannel());
         assertEquals(message, mockSubscriber.latestMessage());
@@ -274,22 +271,26 @@ public class AdvanceOperationsTest {
     }
 
     @TestTemplate
-    public void whenUsingBlpop_EnsureItTimeout(Jedis jedis) throws ExecutionException, InterruptedException {
+    public void whenUsingBlpop_EnsureItTimeout(Jedis jedis) throws ExecutionException, InterruptedException, TimeoutException {
         String list1key = "list1_kdjfnvdsu";
         String list2key = "list2_mbhkdushy";
         String list3key = "list3_qzkmpthju";
 
+        // init redisbase
+        jedis.lrange(list2key, 0, -1);
+
         //Block on performing the BLPOP
         Client client = jedis.getClient();
         Jedis blockedClient = new Jedis(client.getHost(), client.getPort());
         ExecutorService blockingThread = Executors.newSingleThreadExecutor();
         Future future = blockingThread.submit(() -> {
             List<String> result = blockedClient.blpop(1, list1key, list2key, list3key);
-            assertEquals(0, result.size());
+            assertNull(result);
         });
         //Check the list is not modified
+        client.setSoTimeout(2000);
         List<String> results = jedis.lrange(list2key, 0, -1);
         assertEquals(0, results.size());
-        future.get();
+        future.get(4, TimeUnit.SECONDS);
     }
 }
diff --git a/src/test/java/com/github/fppt/jedismock/comparisontests/ComparisonBase.java b/src/test/java/com/github/fppt/jedismock/comparisontests/ComparisonBase.java
index 8c1359f..b0cd9e3 100644
--- a/src/test/java/com/github/fppt/jedismock/comparisontests/ComparisonBase.java
+++ b/src/test/java/com/github/fppt/jedismock/comparisontests/ComparisonBase.java
@@ -1,8 +1,8 @@
 package com.github.fppt.jedismock.comparisontests;
 
 import com.github.fppt.jedismock.RedisServer;
-import com.github.fppt.jedismock.util.EmbeddedRedis;
 import org.junit.jupiter.api.extension.*;
+import org.testcontainers.containers.GenericContainer;
 import redis.clients.jedis.Jedis;
 
 import java.util.Collections;
@@ -13,20 +13,26 @@ public class ComparisonBase implements TestTemplateInvocationContextProvider,
         BeforeAllCallback, AfterAllCallback {
     private static RedisServer fakeServer;
 
+    private static GenericContainer redis = new GenericContainer<>("redis:5.0-alpine")
+            .withExposedPorts(6379);
+
+
     @Override
     public void beforeAll(ExtensionContext context) throws Exception {
-        //Start up the real redis server
-        EmbeddedRedis.start();
+
+        // Docker container:
+        redis.start();
 
         //Start up the fake redis server
-        fakeServer = RedisServer.newRedisServer(EmbeddedRedis.PORT + 1);
+        fakeServer = RedisServer.newRedisServer(redis.getFirstMappedPort() + 1);
         fakeServer.start();
     }
 
     @Override
     public void afterAll(ExtensionContext context) throws Exception {
-        //Kill the real redis server
-        EmbeddedRedis.stop();
+
+        // Docker container:
+        redis.stop();
 
         //Kill the fake redis server
         fakeServer.stop();
@@ -44,10 +50,10 @@ public class ComparisonBase implements TestTemplateInvocationContextProvider,
                 new JedisTestTemplateInvocationContext("mock",
                         new Jedis(fakeServer.getHost(), fakeServer.getBindPort(), 1000000)),
                 new JedisTestTemplateInvocationContext("real",
-                        new Jedis("localhost", EmbeddedRedis.PORT)));
+                        new Jedis(redis.getContainerIpAddress(), redis.getFirstMappedPort())));
     }
 
-    private class JedisTestTemplateInvocationContext implements TestTemplateInvocationContext{
+    private class JedisTestTemplateInvocationContext implements TestTemplateInvocationContext {
 
         private final String displayName;
         private final Jedis jedis;
diff --git a/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java b/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
index bd9ef79..24d5423 100644
--- a/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
+++ b/src/test/java/com/github/fppt/jedismock/comparisontests/SimpleOperationsTest.java
@@ -3,7 +3,6 @@ package com.github.fppt.jedismock.comparisontests;
 import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.ExtendWith;
 import redis.clients.jedis.*;
-import redis.clients.jedis.exceptions.JedisConnectionException;
 import redis.clients.jedis.exceptions.JedisDataException;
 
 import java.util.*;
@@ -136,15 +135,14 @@ public class SimpleOperationsTest {
     }
 
     @TestTemplate
-    public void whenUsingQuit_EnsureTheConnectionIsClosed(Jedis jedis) {
+    public void whenUsingQuit_EnsureTheResultIsOK(Jedis jedis) {
         //Create a new connection
         Client client = jedis.getClient();
         Jedis newJedis = new Jedis(client.getHost(), client.getPort());
         newJedis.set("A happy lucky key", "A sad value");
-        assertEquals("OK", newJedis.quit());
 
-        assertThrows(JedisConnectionException.class,
-            ()->newJedis.set("A happy lucky key", "A sad value 2"));
+        assertEquals("OK", newJedis.quit());
+        assertEquals("A sad value", jedis.get("A happy lucky key"));
     }
 
     @TestTemplate
@@ -553,7 +551,7 @@ public class SimpleOperationsTest {
 
         ScanResult<String> result = jedis.scan(ScanParams.SCAN_POINTER_START);
 
-        assertEquals(ScanParams.SCAN_POINTER_START, result.getStringCursor());
+        assertEquals(ScanParams.SCAN_POINTER_START, result.getCursor());
         assertEquals(2, result.getResult().size());
         assertTrue(result.getResult().contains(key));
         assertTrue(result.getResult().contains(key2));
@@ -573,7 +571,7 @@ public class SimpleOperationsTest {
         ScanResult<String> result = jedis.scan(ScanParams.SCAN_POINTER_START,
                 new ScanParams().match("scankeymatch:1*"));
 
-        assertEquals(ScanParams.SCAN_POINTER_START, result.getStringCursor());
+        assertEquals(ScanParams.SCAN_POINTER_START, result.getCursor());
         assertEquals(1, result.getResult().size());
         assertTrue(result.getResult().contains(key));
     }
@@ -591,7 +589,7 @@ public class SimpleOperationsTest {
         ScanResult<String> result = jedis.scan(ScanParams.SCAN_POINTER_START,
                 new ScanParams().match("scankeyi:1*").count(10));
 
-        assertNotEquals(ScanParams.SCAN_POINTER_START, result.getStringCursor());
+        assertNotEquals(ScanParams.SCAN_POINTER_START, result.getCursor());
     }
 
     @TestTemplate
@@ -627,7 +625,7 @@ public class SimpleOperationsTest {
         jedis.sadd(key, values);
 
         ScanResult<String> result = jedis.sscan(key, ScanParams.SCAN_POINTER_START, new ScanParams().count(13));
-        assertNotEquals(ScanParams.SCAN_POINTER_START, result.getStringCursor());
+        assertNotEquals(ScanParams.SCAN_POINTER_START, result.getCursor());
     }
 
     @TestTemplate
@@ -646,7 +644,7 @@ public class SimpleOperationsTest {
         ScanResult<String> result = jedis.sscan(key, ScanParams.SCAN_POINTER_START,
                 new ScanParams().match("21_value_0"));
 
-        assertEquals(ScanParams.SCAN_POINTER_START, result.getStringCursor());
+        assertEquals(ScanParams.SCAN_POINTER_START, result.getCursor());
         assertEquals(1, result.getResult().size());
         assertTrue(result.getResult().contains(values[0]));
     }
@@ -671,7 +669,7 @@ public class SimpleOperationsTest {
                 cursor = ScanParams.SCAN_POINTER_START;
             }
             ScanResult<String> result = jedis.sscan(key, cursor);
-            cursor = result.getStringCursor();
+            cursor = result.getCursor();
             results.addAll(result.getResult());
         }
 
@@ -965,6 +963,28 @@ public class SimpleOperationsTest {
 
         assertNull(result);
     }
+
+    @TestTemplate
+    void hsetwithMap(Jedis jedis) {
+        jedis.flushDB();
+
+        Map<String, String> hash = new HashMap<>();
+        hash.put("k1", "v1");
+        hash.put("k2", "v2");
+        final Long added = jedis.hset("key", hash);
+
+        assertEquals(2, added);
+
+        // identity
+        final Long added1 = jedis.hset("key", hash);
+        assertEquals(0, added1);
+
+        // update
+        hash.put("k2", "v3");
+        final Long added2 = jedis.hset("key", hash);
+        assertEquals(0, added2);
+
+    }
     
     @TestTemplate
     public void zscore(Jedis jedis) {
diff --git a/src/test/java/com/github/fppt/jedismock/util/EmbeddedRedis.java b/src/test/java/com/github/fppt/jedismock/util/EmbeddedRedis.java
deleted file mode 100644
index 754bc51..0000000
--- a/src/test/java/com/github/fppt/jedismock/util/EmbeddedRedis.java
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * Grakn - A Distributed Semantic Database
- * Copyright (C) 2016  Grakn Labs Limited
- *
- * Grakn is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Grakn is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with Grakn. If not, see <http://www.gnu.org/licenses/gpl.txt>.
- */
-
-package com.github.fppt.jedismock.util;
-
-import org.slf4j.LoggerFactory;
-import redis.embedded.RedisServer;
-import redis.embedded.exceptions.EmbeddedRedisException;
-
-/**
- * <p>
- *     Starts Embedded Redis
- * </p>
- *
- * <p>
- *     Helper class for starting and working with an embedded redis.
- *     This should be used for testing purposes only
- * </p>
- *
- * @author fppt
- *
- */
-public class EmbeddedRedis {
-    public static final int PORT = 7001;
-    private static final org.slf4j.Logger LOG = LoggerFactory.getLogger(EmbeddedRedis.class);
-    private static volatile RedisServer redisServer;
-
-    static {
-        Runtime.getRuntime().addShutdownHook(
-                new Thread(() -> {
-                    if (redisServer != null && redisServer.isActive()) {
-                        LOG.warn("Redis still running, stopping it on shutdown hook");
-                        redisServer.stop();
-                    }
-                }));
-    }
-
-    /**
-     * Starts an embedded redis on the provided port
-     *
-     * @param port The port to start redis on
-     */
-    private static void start(int port, boolean force){
-        try {
-            LOG.info("Starting redis...");
-            redisServer = RedisServer.builder()
-                    .port(port)
-                    // We have short running tests and sometimes we kill the connections
-                    .setting("timeout 360")
-                    .build();
-            if (force || !redisServer.isActive()) {
-                try {
-                    redisServer.start();
-                } catch (EmbeddedRedisException e) {
-                    LOG.warn("Unexpected Redis instance already running on port {}", port);
-                } catch (Exception e) {
-                    LOG.warn("Exception while trying to start Redis on port {}. Will attempt to continue.", port, e);
-                }
-                LOG.info("Redis started on {}", port);
-            } else {
-                LOG.warn("Redis already running.");
-            }
-        } catch (Exception e) {
-            LOG.warn("Failure to start redis on port {}, maybe running alredy", port, e);
-        }
-    }
-
-    public static void start(){
-        start(PORT, false);
-    }
-
-    /**
-     * Stops the embedded redis
-     */
-    public static void stop(){
-        try {
-            LOG.info("Stopping Redis...");
-            redisServer.stop();
-            LOG.info("Redis stopped.");
-        } catch (Exception e) {
-            LOG.warn("Failure while stopping redis", e);
-        }
-    }
-}