Bug fix for smove command.

wilkenstein · Mar 23, 2015 · bfade30 · bfade30
1 parent b77d4b1
commit bfade30
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 10 deletions.
diff --git a/README.md b/README.md
@@ -382,6 +382,8 @@ JavaScript engines, a somewhat specific style is required:
   - HyperLogLog support.
 
 ## Versions
+* 0.0.11-3
+  - Bug fix for smove command.
 * 0.0.11-2
   - Implement pub/sub in earnest.
   - Unit test pub/sub.

diff --git a/package.json b/package.json
@@ -6,7 +6,7 @@
         "url": "git://github.com/wilkenstein/redis-mock-js.git"
     },
     "main": "redis-mock.js",
-    "version": "0.0.11-2",
+    "version": "0.0.11-3",
     "devDependencies": {
         "jshint": "2.3.0",
         "q": "~1.0",

diff --git a/redis-mock.js b/redis-mock.js
@@ -1428,13 +1428,21 @@
 
     redismock.smove = function (source, dest, member, callback) {
         var r, e;
+        if (this.exists(source) && this.type(source) !== "set") {
+            return wrongType(callback);
+        }
+        if (this.exists(dest) && this.type(dest) !== "set") {
+            return wrongType(callback);
+        }
         r = redismock.srem(source, member);
         if (r instanceof Error) {
             return cb(callback)(r);
         }
-        e = redismock.sadd(dest, member);
-        if (e instanceof Error) {
-            return cb(callback)(e);
+        if (r === 1) {
+            e = redismock.sadd(dest, member);
+            if (e instanceof Error) {
+                return cb(callback)(e);
+            }
         }
         return cb(callback)(null, r);
     };

diff --git a/test/mocha/redismockSetTest.js b/test/mocha/redismockSetTest.js
@@ -398,7 +398,7 @@
                 done();
             });
         });
-        it('to return an error for a destination key that is not a set', function (done) {
+        it('should return an error for a destination key that is not a set', function (done) {
             var k1 = randkey(), k2 = randkey();
             var v = 'v';
             redismock.sadd(k1, v);
@@ -412,17 +412,29 @@
                 done();
             });
         });
-        it('to do nothing if the element is not in the source set', function (done) {
+        it('should do nothing if the element is not in the source set', function (done) {
             var k1 = randkey(), k2 = randkey();
-            var v = 'v';
-            expect(redismock.smove(k1, k2, v)).to.equal(0);
-            redismock.smove(k1, k2, v, function (err, reply) {
+            var v1 = 'v1', v2 = 'v2';
+            redismock.sadd(k1, v1);
+            redismock.sadd(k2, v2);
+            expect(redismock.smove(k1, k2, v2)).to.equal(0);
+            expect(redismock.scard(k1)).to.equal(1);
+            expect(redismock.sismember(k1, v1)).to.equal(1);
+            expect(redismock.scard(k2)).to.equal(1);
+            expect(redismock.sismember(k2, v2)).to.equal(1);
+            expect(redismock.sismember(k2, v1)).to.equal(0);
+            redismock.smove(k2, k1, v1, function (err, reply) {
                 expect(err).to.not.exist;
                 expect(reply).to.equal(0);
+                expect(redismock.scard(k1)).to.equal(1);
+                expect(redismock.sismember(k1, v1)).to.equal(1);
+                expect(redismock.scard(k2)).to.equal(1);
+                expect(redismock.sismember(k2, v2)).to.equal(1);
+                expect(redismock.sismember(k2, v1)).to.equal(0);
                 done();
             });
         });
-        it('to move the element from the source to the destination', function (done) {
+        it('should move the element from the source to the destination', function (done) {
             var k1 = randkey(), k2 = randkey();
             var v = 'v', v1 = 'v1';
             redismock.sadd(k1, v);