From a01224960bd4bce5121b8610ae2b090dd95b695c Mon Sep 17 00:00:00 2001 From: Gabe Limon Date: Fri, 19 Jun 2015 15:14:37 -0700 Subject: [PATCH 1/2] TECHOPS-17109: Added support for a Predis Resque backend --- .../{PauseTest.php => PauseTestBase.php} | 33 +------ Tests/Integration/PauseWithPredisTest.php | 51 ++++++++++ .../Integration/PauseWithResqueRedisTest.php | 43 +++++++++ Tests/Unit/JobPauserTest.php | 96 +++++++++++++++---- composer.json | 3 +- phpunit.xml | 1 + src/JobPauser.php | 20 ++-- src/Pause.php | 3 +- 8 files changed, 193 insertions(+), 57 deletions(-) rename Tests/Integration/{PauseTest.php => PauseTestBase.php} (72%) create mode 100644 Tests/Integration/PauseWithPredisTest.php create mode 100644 Tests/Integration/PauseWithResqueRedisTest.php diff --git a/Tests/Integration/PauseTest.php b/Tests/Integration/PauseTestBase.php similarity index 72% rename from Tests/Integration/PauseTest.php rename to Tests/Integration/PauseTestBase.php index f073b92..f0e729d 100644 --- a/Tests/Integration/PauseTest.php +++ b/Tests/Integration/PauseTestBase.php @@ -1,5 +1,5 @@ * @license http://www.opensource.org/licenses/mit-license.php */ -class PauseTest extends PHPUnit_Framework_TestCase +class PauseTestBase extends PHPUnit_Framework_TestCase { /** @var Pause */ protected $pauser = null; - public static function setUpBeforeClass() - { - $testMisc = realpath(__DIR__ . '/misc/'); - $redisConf = "$testMisc/redis.conf"; - - // Attempt to start our own redis instance for tesitng. - exec('which redis-server', $output, $returnVar); - if ($returnVar != 0) { - echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; - exit(1); - } - - exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); - usleep(500000); - if ($returnVar != 0) { - echo "Cannot start redis-server.\n"; - exit(1); - } - - // Get redis port from conf - $config = file_get_contents($redisConf); - if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { - echo "Could not determine redis port from redis.conf"; - exit(1); - } - - Resque::setBackend('localhost:' . $matches[1]); - } - public function setUp() { Resque::redis()->flushAll(); diff --git a/Tests/Integration/PauseWithPredisTest.php b/Tests/Integration/PauseWithPredisTest.php new file mode 100644 index 0000000..31677e1 --- /dev/null +++ b/Tests/Integration/PauseWithPredisTest.php @@ -0,0 +1,51 @@ + + * @license http://www.opensource.org/licenses/mit-license.php + */ +class PauseWithPredisTest extends PauseTestBase +{ + public static function setUpBeforeClass() + { + $testMisc = realpath(__DIR__ . '/misc/'); + $redisConf = "$testMisc/redis.conf"; + + // Attempt to start our own redis instance for tesitng. + exec('which redis-server', $output, $returnVar); + if ($returnVar != 0) { + echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; + exit(1); + } + + exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); + usleep(500000); + if ($returnVar != 0) { + echo "Cannot start redis-server.\n"; + exit(1); + } + + // Get redis port from conf + $config = file_get_contents($redisConf); + if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { + echo "Could not determine redis port from redis.conf"; + exit(1); + } + + $clientParams = array( + 'host' => 'localhost', + 'port' => $matches[1], + 'prefix' => 'resque:', + ); + Resque::setBackend(function () use ($clientParams) { + return new Client($clientParams); + }); + } +} diff --git a/Tests/Integration/PauseWithResqueRedisTest.php b/Tests/Integration/PauseWithResqueRedisTest.php new file mode 100644 index 0000000..a9fa7ab --- /dev/null +++ b/Tests/Integration/PauseWithResqueRedisTest.php @@ -0,0 +1,43 @@ + + * @license http://www.opensource.org/licenses/mit-license.php + */ +class PauseWithResqueRedisTest extends PauseTestBase +{ + public static function setUpBeforeClass() + { + $testMisc = realpath(__DIR__ . '/misc/'); + $redisConf = "$testMisc/redis.conf"; + + // Attempt to start our own redis instance for tesitng. + exec('which redis-server', $output, $returnVar); + if ($returnVar != 0) { + echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; + exit(1); + } + + exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); + usleep(500000); + if ($returnVar != 0) { + echo "Cannot start redis-server.\n"; + exit(1); + } + + // Get redis port from conf + $config = file_get_contents($redisConf); + if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { + echo "Could not determine redis port from redis.conf"; + exit(1); + } + + Resque::setBackend('localhost:' . $matches[1]); + } +} diff --git a/Tests/Unit/JobPauserTest.php b/Tests/Unit/JobPauserTest.php index add0c9b..4d21e62 100644 --- a/Tests/Unit/JobPauserTest.php +++ b/Tests/Unit/JobPauserTest.php @@ -1,5 +1,5 @@ redisMock = $this->getMockBuilder('Resque_Redis') + $this->resqueRedisMock = $this->getMockBuilder('Resque_Redis') + ->disableOriginalConstructor() + ->setMethods(array( + 'sadd', + 'srem', + 'getPrefix', + 'llen', + 'rename', + 'sismember', + )) + ->getMock(); + + $this->predisMock = $this->getMockBuilder('Predis\Client') ->disableOriginalConstructor() ->setMethods(array( 'sadd', @@ -42,16 +56,17 @@ public function pauseDataProvider() /** * @dataProvider pauseDataProvider * @param bool $saddSuccess + * @param bool $returnSuccess */ public function testPause($saddSuccess, $returnSuccess) { - $this->redisMock + $this->resqueRedisMock ->expects($this->once()) ->method('sadd') ->with($this->equalTo('pauses'), $this->equalTo('upgrade:test')) ->willReturn($saddSuccess); - $pauser = new JobPauser($this->redisMock, 'resqueFaker:'); + $pauser = new JobPauser($this->resqueRedisMock, 'resqueFaker:'); $this->assertEquals($returnSuccess, $pauser->pause('upgrade:test')); } @@ -73,19 +88,19 @@ public function resumeDataProvider() */ public function testResume($isPaused, $sremSuccess, $returnSuccess) { - $this->redisMock + $this->resqueRedisMock ->expects($this->once()) ->method('sismember') ->with($this->equalTo('pauses')) ->willReturn($isPaused); - $this->redisMock + $this->resqueRedisMock ->expects($isPaused ? $this->once() : $this->never()) ->method('srem') ->with($this->equalTo('pauses'), $this->equalTo('upgrade:test2')) ->willReturn($sremSuccess); - $pauser = new JobPauser($this->redisMock, 'resqueFaker:'); + $pauser = new JobPauser($this->resqueRedisMock, 'resqueFaker:'); $this->assertEquals($returnSuccess, $pauser->resume('upgrade:test2')); } @@ -107,19 +122,44 @@ public function renameDataprovider() */ public function testRenameToTemp($queueLength, $renameSuccess, $expectedResult) { - $this->redisMock + $this->resqueRedisMock ->expects($this->once()) ->method('llen') ->with($this->equalTo('queue:upgrade:test3')) ->willReturn($queueLength); - $this->redisMock + $this->resqueRedisMock ->expects($queueLength ? $this->once() : $this->never()) ->method('rename') ->with($this->equalTo('queue:upgrade:test3'), $this->equalTo('resqueFaker:temp:upgrade:test3')) ->willReturn($renameSuccess); - $pauser = new JobPauser($this->redisMock, 'resqueFaker:'); + $pauser = new JobPauser($this->resqueRedisMock, 'resqueFaker:'); + $this->assertEquals($expectedResult, $pauser->renameToTemp('upgrade:test3')); + } + + + /** + * @dataProvider renameDataProvider + * @param int $queueLength + * @param bool $renameSuccess + * @param bool $expectedResult + */ + public function testRenameToTempWithPredis($queueLength, $renameSuccess, $expectedResult) + { + $this->predisMock + ->expects($this->once()) + ->method('llen') + ->with($this->equalTo('queue:upgrade:test3')) + ->willReturn($queueLength); + + $this->predisMock + ->expects($queueLength ? $this->once() : $this->never()) + ->method('rename') + ->with($this->equalTo('queue:upgrade:test3'), $this->equalTo('temp:upgrade:test3')) + ->willReturn($renameSuccess); + + $pauser = new JobPauser($this->predisMock, 'resqueFaker:'); $this->assertEquals($expectedResult, $pauser->renameToTemp('upgrade:test3')); } @@ -131,19 +171,43 @@ public function testRenameToTemp($queueLength, $renameSuccess, $expectedResult) */ public function testRenameBackFromTemp($queueLength, $renameSuccess, $expectedResult) { - $this->redisMock + $this->resqueRedisMock ->expects($this->once()) ->method('llen') ->with($this->equalTo('temp:upgrade:test3')) ->willReturn($queueLength); - $this->redisMock + $this->resqueRedisMock ->expects($queueLength ? $this->once() : $this->never()) ->method('rename') ->with($this->equalTo('temp:upgrade:test3'), $this->equalTo('resqueFaker:queue:upgrade:test3')) ->willReturn($renameSuccess); - $pauser = new JobPauser($this->redisMock, 'resqueFaker:'); + $pauser = new JobPauser($this->resqueRedisMock, 'resqueFaker:'); + $this->assertEquals($expectedResult, $pauser->renameBackFromTemp('upgrade:test3')); + } + + /** + * @dataProvider renameDataProvider Reuses the renaming provider because the cases are the same + * @param int $queueLength + * @param bool $renameSuccess + * @param bool $expectedResult + */ + public function testRenameBackFromTempWithPredis($queueLength, $renameSuccess, $expectedResult) + { + $this->predisMock + ->expects($this->once()) + ->method('llen') + ->with($this->equalTo('temp:upgrade:test3')) + ->willReturn($queueLength); + + $this->predisMock + ->expects($queueLength ? $this->once() : $this->never()) + ->method('rename') + ->with($this->equalTo('temp:upgrade:test3'), $this->equalTo('queue:upgrade:test3')) + ->willReturn($renameSuccess); + + $pauser = new JobPauser($this->predisMock, 'resqueFaker:'); $this->assertEquals($expectedResult, $pauser->renameBackFromTemp('upgrade:test3')); } @@ -162,13 +226,13 @@ public function isPausedDataProvider() */ public function testIsPaused($existsSuccess, $expectedResult) { - $this->redisMock + $this->resqueRedisMock ->expects($this->once()) ->method('sismember') ->with($this->equalTo('pauses'), $this->equalTo('upgrade:test8')) ->willReturn($existsSuccess); - $pauser = new JobPauser($this->redisMock, 'resqueFaker:'); + $pauser = new JobPauser($this->resqueRedisMock, 'resqueFaker:'); $this->assertEquals($expectedResult, $pauser->isPaused('upgrade:test8')); } } diff --git a/composer.json b/composer.json index 420313e..f5d88fd 100644 --- a/composer.json +++ b/composer.json @@ -11,7 +11,8 @@ }, "require-dev": { "phpunit/phpunit": "4.5.*", - "squizlabs/php_codesniffer": "2.3.0" + "squizlabs/php_codesniffer": "2.3.0", + "predis/predis": "^1.0" }, "autoload": { "psr-4": { diff --git a/phpunit.xml b/phpunit.xml index 64cf626..8b2b35f 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -13,6 +13,7 @@ ./Tests/ + ./Tests/Integration/PauseTestBase.php diff --git a/src/JobPauser.php b/src/JobPauser.php index 110f1be..2a4db5f 100755 --- a/src/JobPauser.php +++ b/src/JobPauser.php @@ -1,6 +1,8 @@ redis = $redis; - $this->resqueRedisPrefix = $resqueRedisPrefix; + // If we're using Resque_Redis we need to add the queue prefix unless it's the first argument + $this->resqueRedisPrefix = $redis instanceof Resque_Redis ? $resqueRedisPrefix : ''; } /** @@ -53,16 +59,15 @@ public function resume($queue) * Rename original queue to temp queue * * @param string $queue - * @param string $queuePrefix original queue's prefix , by default if you use PHP-resque it's 'queue' * @return bool */ - public function renameToTemp($queue, $queuePrefix = 'queue:') + public function renameToTemp($queue) { - if ($this->queueIsEmpty($queuePrefix . $queue)) { + if ($this->queueIsEmpty(self::$queuePrefix . $queue)) { return true; } return $this->redis->rename( - $queuePrefix . $queue, + self::$queuePrefix . $queue, $this->resqueRedisPrefix . self::$tempQueuePrefix . $queue ); } @@ -71,17 +76,16 @@ public function renameToTemp($queue, $queuePrefix = 'queue:') * Rename back from temp to original * * @param string $queue - * @param string $queuePrefix original queue's prefix , by default if you use PHP-resque it's 'queue' * @return bool */ - public function renameBackFromTemp($queue, $queuePrefix = 'queue:') + public function renameBackFromTemp($queue) { if ($this->queueIsEmpty(self::$tempQueuePrefix . $queue)) { return true; } return $this->redis->rename( self::$tempQueuePrefix . $queue, - $this->resqueRedisPrefix . $queuePrefix . $queue + $this->resqueRedisPrefix . self::$queuePrefix . $queue ); } diff --git a/src/Pause.php b/src/Pause.php index 7a9409e..ba6e200 100644 --- a/src/Pause.php +++ b/src/Pause.php @@ -3,6 +3,7 @@ use Resque; use Resque_Event; +use Resque_Redis; use Resque_Job_DontCreate; /** @@ -23,7 +24,7 @@ class Pause public function __construct() { // Create object for pausing jobs - $this->pauser = new JobPauser(Resque::redis(), \Resque_Redis::getPrefix()); + $this->pauser = new JobPauser(Resque::redis(), Resque_Redis::getPrefix()); $pauser = $this->pauser; // Listen for enqueue and move any new jobs to temp queue From 38a25a5e0326ebfe0cdaa248c0e5f48a4d612805 Mon Sep 17 00:00:00 2001 From: Gabe Limon Date: Sun, 21 Jun 2015 20:56:43 -0700 Subject: [PATCH 2/2] Fixed up docblocks, fixed up redis bootstrapping, and updated author --- .travis.yml | 7 +--- Tests/Integration/PauseTestBase.php | 41 ++++++++++++++++--- Tests/Integration/PauseWithPredisTest.php | 33 ++------------- .../Integration/PauseWithResqueRedisTest.php | 34 ++------------- Tests/Integration/misc/redis.conf | 7 ++-- Tests/Unit/JobPauserTest.php | 7 +--- composer.json | 4 +- src/JobPauser.php | 6 +-- src/Pause.php | 7 +--- 9 files changed, 56 insertions(+), 90 deletions(-) diff --git a/.travis.yml b/.travis.yml index a1f7a73..7490ee8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,12 +4,7 @@ php: - 5.4 - 5.5 - 5.6 -env: - - REDIS_STANDALONE=0 -before_script: - - sh -c "if [ $REDIS_STANDALONE -eq 0 ]; then wget https://github.com/nicolasff/phpredis/archive/2.2.3.zip -O php-redis.zip && unzip php-redis.zip; fi" - - sh -c "if [ $REDIS_STANDALONE -eq 0 ]; then cd phpredis-2.2.3/ && phpize && ./configure && make && make install; fi" - - sh -c "if [ $REDIS_STANDALONE -eq 0 ]; then echo \"extension=redis.so\" >> `php --ini | grep \"Loaded Configuration\" | sed -e \"s|.*:\s*||\"`; fi" +install: - composer install script: - composer lint diff --git a/Tests/Integration/PauseTestBase.php b/Tests/Integration/PauseTestBase.php index f0e729d..3590247 100644 --- a/Tests/Integration/PauseTestBase.php +++ b/Tests/Integration/PauseTestBase.php @@ -6,17 +6,48 @@ use PHPUnit_Framework_TestCase; /** - * Pause tests. - * - * @package PHP Resque Pause - * @author Wedy Chainy - * @license http://www.opensource.org/licenses/mit-license.php + * Class PauseTestBase Provides all cases for testing the Pause class + * @package Resque\Plugins\Tests\Integration */ class PauseTestBase extends PHPUnit_Framework_TestCase { /** @var Pause */ protected $pauser = null; + protected static function setupRedis() + { + $testMisc = realpath(__DIR__ . '/misc/'); + $redisConf = "$testMisc/redis.conf"; + + // Attempt to start our own redis instance for tesitng. + exec('which redis-server', $output, $returnVar); + if ($returnVar != 0) { + echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; + exit(1); + } + + exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); + usleep(500000); // Wait to for the server to start + if ($returnVar != 0) { + echo "Cannot start redis-server.\n"; + exit(1); + } + } + + protected static function getRedisPort() + { + $testMisc = realpath(__DIR__ . '/misc/'); + $redisConf = "$testMisc/redis.conf"; + + // Get redis port from conf + $config = file_get_contents($redisConf); + if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { + echo "Could not determine redis port from redis.conf"; + exit(1); + } + return $matches[1]; + } + public function setUp() { Resque::redis()->flushAll(); diff --git a/Tests/Integration/PauseWithPredisTest.php b/Tests/Integration/PauseWithPredisTest.php index 31677e1..8fbb147 100644 --- a/Tests/Integration/PauseWithPredisTest.php +++ b/Tests/Integration/PauseWithPredisTest.php @@ -5,43 +5,18 @@ use Predis\Client; /** - * Pause tests. - * - * @package PHP Resque Pause - * @author Wedy Chainy - * @license http://www.opensource.org/licenses/mit-license.php + * Class PauseWithPredisTest Runs the PauseTestBase cases with a Predis backend + * @package Resque\Plugins\Tests\Integration */ class PauseWithPredisTest extends PauseTestBase { public static function setUpBeforeClass() { - $testMisc = realpath(__DIR__ . '/misc/'); - $redisConf = "$testMisc/redis.conf"; - - // Attempt to start our own redis instance for tesitng. - exec('which redis-server', $output, $returnVar); - if ($returnVar != 0) { - echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; - exit(1); - } - - exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); - usleep(500000); - if ($returnVar != 0) { - echo "Cannot start redis-server.\n"; - exit(1); - } - - // Get redis port from conf - $config = file_get_contents($redisConf); - if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { - echo "Could not determine redis port from redis.conf"; - exit(1); - } + static::setupRedis(); $clientParams = array( 'host' => 'localhost', - 'port' => $matches[1], + 'port' => static::getRedisPort(), 'prefix' => 'resque:', ); Resque::setBackend(function () use ($clientParams) { diff --git a/Tests/Integration/PauseWithResqueRedisTest.php b/Tests/Integration/PauseWithResqueRedisTest.php index a9fa7ab..f38f305 100644 --- a/Tests/Integration/PauseWithResqueRedisTest.php +++ b/Tests/Integration/PauseWithResqueRedisTest.php @@ -4,40 +4,14 @@ use Resque; /** - * Pause tests. - * - * @package PHP Resque Pause - * @author Wedy Chainy - * @license http://www.opensource.org/licenses/mit-license.php + * Class PauseWithResqueRedisTest Runs the PauseTestBase cases with a Resque_Redis backend + * @package Resque\Plugins\Tests\Integration */ class PauseWithResqueRedisTest extends PauseTestBase { public static function setUpBeforeClass() { - $testMisc = realpath(__DIR__ . '/misc/'); - $redisConf = "$testMisc/redis.conf"; - - // Attempt to start our own redis instance for tesitng. - exec('which redis-server', $output, $returnVar); - if ($returnVar != 0) { - echo "Cannot find redis-server in path. Please make sure redis is installed.\n"; - exit(1); - } - - exec("cd $testMisc; redis-server $redisConf", $output, $returnVar); - usleep(500000); - if ($returnVar != 0) { - echo "Cannot start redis-server.\n"; - exit(1); - } - - // Get redis port from conf - $config = file_get_contents($redisConf); - if (!preg_match('#^\s*port\s+([0-9]+)#m', $config, $matches)) { - echo "Could not determine redis port from redis.conf"; - exit(1); - } - - Resque::setBackend('localhost:' . $matches[1]); + static::setupRedis(); + Resque::setBackend('localhost:' . static::getRedisPort()); } } diff --git a/Tests/Integration/misc/redis.conf b/Tests/Integration/misc/redis.conf index 971f66e..b3561b2 100644 --- a/Tests/Integration/misc/redis.conf +++ b/Tests/Integration/misc/redis.conf @@ -1,8 +1,7 @@ daemonize yes -pidfile ./redis.pid -port 6379 +pidfile redis.pid +port 63791 bind 127.0.0.1 timeout 300 -dbfilename dump.rdb dir ./ -loglevel debug \ No newline at end of file +loglevel debug diff --git a/Tests/Unit/JobPauserTest.php b/Tests/Unit/JobPauserTest.php index 4d21e62..d1c7c45 100644 --- a/Tests/Unit/JobPauserTest.php +++ b/Tests/Unit/JobPauserTest.php @@ -5,11 +5,8 @@ use PHPUnit_Framework_TestCase; /** - * Job tests. - * - * @package Resque/Tests - * @author Wedy Chainy - * @license http://www.opensource.org/licenses/mit-license.php + * Class JobPauserTest + * @package Resque\Plugins\Tests\Unit */ class JobPauserTest extends PHPUnit_Framework_TestCase { diff --git a/composer.json b/composer.json index f5d88fd..fd2f6fe 100644 --- a/composer.json +++ b/composer.json @@ -3,8 +3,8 @@ "type": "library", "description": "php-resque-pause is a PHP port of resque-pause, which adds support for pausing / unpausing Resque jobs.", "authors": [{ - "name": "Wedy Chainy", - "email": "wedy.chainy@bigcommerce.com" + "name": "The Bigcommerce tooling team", + "email": "tools@bigcommerce.com" }], "require": { "chrisboulton/php-resque": "dev-master" diff --git a/src/JobPauser.php b/src/JobPauser.php index 2a4db5f..72d3cad 100755 --- a/src/JobPauser.php +++ b/src/JobPauser.php @@ -4,10 +4,8 @@ use Resque_Redis; /** - * Resque Pause job. - * - * @author Wedy Chainy - * @license http://www.opensource.org/licenses/mit-license.php + * Class JobPauser Interacts with Redis for Resque\Plugins\Pause + * @package Resque\Plugins */ class JobPauser { diff --git a/src/Pause.php b/src/Pause.php index ba6e200..9f1380e 100644 --- a/src/Pause.php +++ b/src/Pause.php @@ -7,11 +7,8 @@ use Resque_Job_DontCreate; /** - * Base Resque Pause class - * - * @package PHP Resque Pause - * @author Wedy Chainy - * @licence http://www.opensource.org/licenses/mit-license.php + * Class Pause Pauses Resque queues + * @package Resque\Plugins */ class Pause {