Merge pull request #6 from gabelimon/TOOLING-255-make-pause-quieter

TOOLING-255: Fail more quietly

Author: gabelimon

Tests/Integration/PauseTest.php

@@ -65,7 +65,7 @@ public function testPause()
         $this->assertFalse($this->pauser->pause('upgrade:test'));
 
         // Pause non-existent queue
-        $this->assertFalse($this->pauser->pause('upgrade:test2'));
+        $this->assertTrue($this->pauser->pause('upgrade:test2'));
     }
 
     public function testResume()
@@ -79,10 +79,10 @@ public function testResume()
 
         // Resume non-paused queue
         Resque::enqueue('upgrade:test4', 'test');
-        $this->assertFalse($this->pauser->resume('upgrade:test4'));
+        $this->assertTrue($this->pauser->resume('upgrade:test4'));
 
         // Resume non-existent queue
-        $this->assertFalse($this->pauser->resume('upgrade:idontexist'));
+        $this->assertTrue($this->pauser->resume('upgrade:idontexist'));
     }
 
     public function testIsPaused()

Tests/Unit/JobPauserTest.php

@@ -24,13 +24,14 @@ public function setUp()
                 'sadd',
                 'srem',
                 'getPrefix',
+                'llen',
                 'rename',
                 'sismember',
             ))
             ->getMock();
     }
 
-    public function createDataProvider()
+    public function pauseDataProvider()
     {
         return array(
             array(true, true),
@@ -39,10 +40,10 @@ public function createDataProvider()
     }
 
     /**
-     * @dataProvider createDataProvider
+     * @dataProvider pauseDataProvider
      * @param bool $saddSuccess
      */
-    public function testCreate($saddSuccess, $returnSuccess)
+    public function testPause($saddSuccess, $returnSuccess)
     {
         $this->redisMock
             ->expects($this->once())
@@ -54,22 +55,32 @@ public function testCreate($saddSuccess, $returnSuccess)
         $this->assertEquals($returnSuccess, $pauser->pause('upgrade:test'));
     }
 
-    public function removeDataProvider()
+    public function resumeDataProvider()
     {
         return array(
-            array(true, true),
-            array(false, false),
+            array(true, true, true),
+            array(true, false, false),
+            array(false, true, true),
+            array(false, false, true),
         );
     }
 
     /**
-     * @dataProvider removeDataProvider
+     * @dataProvider resumeDataProvider
+     * @param bool $isPaused
      * @param bool $sremSuccess
+     * @param bool $returnSuccess
      */
-    public function testRemove($sremSuccess, $returnSuccess)
+    public function testResume($isPaused, $sremSuccess, $returnSuccess)
     {
         $this->redisMock
             ->expects($this->once())
+            ->method('sismember')
+            ->with($this->equalTo('pauses'))
+            ->willReturn($isPaused);
+
+        $this->redisMock
+            ->expects($isPaused ? $this->once() : $this->never())
             ->method('srem')
             ->with($this->equalTo('pauses'), $this->equalTo('upgrade:test2'))
             ->willReturn($sremSuccess);
@@ -81,20 +92,29 @@ public function testRemove($sremSuccess, $returnSuccess)
     public function renameDataprovider()
     {
         return array(
-            array(true, true),
-            array(false, false),
+            array(1, true, true),
+            array(1, false, false),
+            array(0, true, true),
+            array(0, false, true),
         );
     }
 
     /**
      * @dataProvider renameDataProvider
+     * @param int $queueLength
      * @param bool $renameSuccess
      * @param bool $expectedResult
      */
-    public function testRenameToTemp($renameSuccess, $expectedResult)
+    public function testRenameToTemp($queueLength, $renameSuccess, $expectedResult)
     {
         $this->redisMock
             ->expects($this->once())
+            ->method('llen')
+            ->with($this->equalTo('queue:upgrade:test3'))
+            ->willReturn($queueLength);
+
+        $this->redisMock
+            ->expects($queueLength ? $this->once() : $this->never())
             ->method('rename')
             ->with($this->equalTo('queue:upgrade:test3'), $this->equalTo('resqueFaker:temp:upgrade:test3'))
             ->willReturn($renameSuccess);
@@ -105,13 +125,20 @@ public function testRenameToTemp($renameSuccess, $expectedResult)
 
     /**
      * @dataProvider renameDataProvider Reuses the renaming provider because the cases are the same
+     * @param int $queueLength
      * @param bool $renameSuccess
      * @param bool $expectedResult
      */
-    public function testRenameBackFromTemp($renameSuccess, $expectedResult)
+    public function testRenameBackFromTemp($queueLength, $renameSuccess, $expectedResult)
     {
         $this->redisMock
             ->expects($this->once())
+            ->method('llen')
+            ->with($this->equalTo('temp:upgrade:test3'))
+            ->willReturn($queueLength);
+
+        $this->redisMock
+            ->expects($queueLength ? $this->once() : $this->never())
             ->method('rename')
             ->with($this->equalTo('temp:upgrade:test3'), $this->equalTo('resqueFaker:queue:upgrade:test3'))
             ->willReturn($renameSuccess);
@@ -120,7 +147,7 @@ public function testRenameBackFromTemp($renameSuccess, $expectedResult)
         $this->assertEquals($expectedResult, $pauser->renameBackFromTemp('upgrade:test3'));
     }
 
-    public function existsDataProvider()
+    public function isPausedDataProvider()
     {
         return array(
             array(true, true),
@@ -129,11 +156,11 @@ public function existsDataProvider()
     }
 
     /**
-     * @dataProvider existsDataProvider
+     * @dataProvider isPausedDataProvider
      * @param bool $existsSuccess
      * @param bool $expectedResult
      */
-    public function testExists($existsSuccess, $expectedResult)
+    public function testIsPaused($existsSuccess, $expectedResult)
     {
         $this->redisMock
             ->expects($this->once())

src/JobPauser.php

@@ -46,6 +46,9 @@ public function pause($queue)
      */
     public function resume($queue)
     {
+        if (!$this->isPaused($queue)) {
+            return true;
+        }
         return $this->redis->srem(self::$pausedSetName, $queue);
     }
 
@@ -58,6 +61,9 @@ public function resume($queue)
      */
     public function renameToTemp($queue, $queuePrefix = 'queue:')
     {
+        if ($this->queueIsEmpty($queuePrefix . $queue)) {
+            return true;
+        }
         return $this->redis->rename(
             $queuePrefix . $queue,
             $this->resqueRedisPrefix . self::$tempQueuePrefix . $queue
@@ -73,6 +79,9 @@ public function renameToTemp($queue, $queuePrefix = 'queue:')
      */
     public function renameBackFromTemp($queue, $queuePrefix = 'queue:')
     {
+        if ($this->queueIsEmpty(self::$tempQueuePrefix . $queue)) {
+            return true;
+        }
         return $this->redis->rename(
             self::$tempQueuePrefix . $queue,
             $this->resqueRedisPrefix . $queuePrefix . $queue
@@ -105,4 +114,15 @@ public function isPaused($queue)
     {
         return (bool)$this->redis->sismember(self::$pausedSetName, $queue);
     }
+
+    /**
+     * Check if a queue has anything in it which is the same as an existence check
+     *
+     * @param $queue
+     * @return bool
+     */
+    public function queueIsEmpty($queue)
+    {
+        return $this->redis->llen($queue) === 0;
+    }
 }