Unit tests & multi/exec fixes (#89)

* Suppress errors on cleanup (and avoid potentially deleting the config files) * The pubsub test crassh on phpredis 3.13 and below, but works with 3.1.4RCx. * All basic tests pass (except pubsub with phpredis which explicitly fails) * Fix-up testMasterSlave test * Give Redis time to exit * Fixup testGetHostAndPort test * Allow php7.x opcache to optimize the array to be vastly more compact, and use an O(1) lookup rater than a array scan of strings. * When in standalone multi() mode, exec() now decodes the result, and discard() destroys any pipelining or multi setup. * Rearange testPipeline & testPipelineTransaction to have the same test functionality. Add tests for zrangebyscore/zrevrangebyscore * Fix pipeline/multi modes for more commands * Shim zRange/zRevRange arguments before they are sent to phpredis & expand test coverage * remove unneeded variable * In fluent mode, credis returns itself * Add test for watch/unwatch, ensure the connection is closed/cleaned up when a pipeline&multi or watch fails. * Use diskless replication and add some delays for initial setup time. * Implement info() command for Sentinel * sentinel tests now wait for replication setup to finish * Run all tests * Restore php5.6 support in Credis_cluster * Compile redis from source so redis-sentinel is available * Tweak travis build * Move binaries to the right directory * Update PATH environment variable rather than move binaries around. * Remove debugging code * Better tolerance of slop in the timeout * Push common code into CredisTestCommon * Use role() instead of info() to reliably get the slave connecting state * Move sleep after connect check * Add waitForSlaveReplication for reliable waiting for master-slave replication
colinmollenhour · Sep 26, 2017 · e8e9b20 · e8e9b20
1 parent e0a208d
commit e8e9b20
Show file tree

Hide file tree

Showing 12 changed files with 668 additions and 361 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -12,11 +12,12 @@ matrix:
     - php: 7.1
     - php: 7.2
 
-services:
-  - redis-server
-
 install:
   - yes '' | pecl install -f redis
+  - wget http://download.redis.io/releases/redis-3.2.11.tar.gz
+  - tar -xzf redis-3.2.11.tar.gz
+  - make -s -C redis-3.2.11 -j4
+  - export PATH=$PATH:$PWD/redis-3.2.11/src/
 
 script:
-  - phpunit --testsuite Basic --exclude-group Auth,UnixSocket
+  - phpunit
diff --git a/Client.php b/Client.php
diff --git a/Cluster.php b/Cluster.php
@@ -31,19 +31,19 @@ class Credis_Cluster
    * @var array
    */
   protected $aliases;
-  
+
   /**
    * Hash ring of Redis server nodes
    * @var array
    */
   protected $ring;
-  
+
   /**
    * Individual nodes of pointers to Redis servers on the hash ring
    * @var array
    */
   protected $nodes;
-  
+
   /**
    * The commands that are not subject to hashing
    * @var array
@@ -276,64 +276,65 @@ public function hash($key)
     return $this->ring[$server];
   }
 
+  const readOnlyCommands = array(
+    'DBSIZE' => true,
+    'INFO' => true,
+    'MONITOR' => true,
+    'EXISTS' => true,
+    'TYPE' => true,
+    'KEYS' => true,
+    'SCAN' => true,
+    'RANDOMKEY' => true,
+    'TTL' => true,
+    'GET' => true,
+    'MGET' => true,
+    'SUBSTR' => true,
+    'STRLEN' => true,
+    'GETRANGE' => true,
+    'GETBIT' => true,
+    'LLEN' => true,
+    'LRANGE' => true,
+    'LINDEX' => true,
+    'SCARD' => true,
+    'SISMEMBER' => true,
+    'SINTER' => true,
+    'SUNION' => true,
+    'SDIFF' => true,
+    'SMEMBERS' => true,
+    'SSCAN' => true,
+    'SRANDMEMBER' => true,
+    'ZRANGE' => true,
+    'ZREVRANGE' => true,
+    'ZRANGEBYSCORE' => true,
+    'ZREVRANGEBYSCORE' => true,
+    'ZCARD' => true,
+    'ZSCORE' => true,
+    'ZCOUNT' => true,
+    'ZRANK' => true,
+    'ZREVRANK' => true,
+    'ZSCAN' => true,
+    'HGET' => true,
+    'HMGET' => true,
+    'HEXISTS' => true,
+    'HLEN' => true,
+    'HKEYS' => true,
+    'HVALS' => true,
+    'HGETALL' => true,
+    'HSCAN' => true,
+    'PING' => true,
+    'AUTH' => true,
+    'SELECT' => true,
+    'ECHO' => true,
+    'QUIT' => true,
+    'OBJECT' => true,
+    'BITCOUNT' => true,
+    'TIME' => true,
+    'SORT' => true,
+    );
+
   public function isReadOnlyCommand($command)
   {
-      $readOnlyCommands = array(
-          'DBSIZE',
-          'INFO',
-          'MONITOR',
-          'EXISTS',
-          'TYPE',
-          'KEYS',
-          'SCAN',
-          'RANDOMKEY',
-          'TTL',
-          'GET',
-          'MGET',
-          'SUBSTR',
-          'STRLEN',
-          'GETRANGE',
-          'GETBIT',
-          'LLEN',
-          'LRANGE',
-          'LINDEX',
-          'SCARD',
-          'SISMEMBER',
-          'SINTER',
-          'SUNION',
-          'SDIFF',
-          'SMEMBERS',
-          'SSCAN',
-          'SRANDMEMBER',
-          'ZRANGE',
-          'ZREVRANGE',
-          'ZRANGEBYSCORE',
-          'ZREVRANGEBYSCORE',
-          'ZCARD',
-          'ZSCORE',
-          'ZCOUNT',
-          'ZRANK',
-          'ZREVRANK',
-          'ZSCAN',
-          'HGET',
-          'HMGET',
-          'HEXISTS',
-          'HLEN',
-          'HKEYS',
-          'HVALS',
-          'HGETALL',
-          'HSCAN',
-          'PING',
-          'AUTH',
-          'SELECT',
-          'ECHO',
-          'QUIT',
-          'OBJECT',
-          'BITCOUNT',
-          'TIME',
-          'SORT'
-      );
-      return in_array(strtoupper($command),$readOnlyCommands);
+      return array_key_exists(strtoupper($command), static::readOnlyCommands);
   }
 }
 
diff --git a/Sentinel.php b/Sentinel.php
@@ -268,6 +268,22 @@ public function __call($name, $args)
         return call_user_func(array($this->_client,'sentinel'),$args);
     }
 
+    /**
+     * get information block for the sentinel instance
+     *
+     * @param string|NUll $section
+     *
+     * @return array
+     */
+    public function info($section = null)
+    {
+        if ($section)
+        {
+            return $this->_client->info($section);
+        }
+        return $this->_client->info();
+    }
+
     /**
      * Return information about all registered master servers
      * @return mixed

diff --git a/tests/CredisClusterTest.php b/tests/CredisClusterTest.php
@@ -6,47 +6,23 @@
 
 class CredisClusterTest extends CredisTestCommon
 {
-
   /** @var Credis_Cluster */
   protected $cluster;
 
-  protected $config;
-
-  protected $useStandalone = FALSE;
-
   protected function setUp()
   {
-    if($this->config === NULL) {
-      $configFile = dirname(__FILE__).'/redis_config.json';
-      if( ! file_exists($configFile) || ! ($config = file_get_contents($configFile))) {
-        $this->markTestSkipped('Could not load '.$configFile);
-        return;
-      }
-      $this->config = json_decode($config);
-      $arrayConfig = array();
-      foreach($this->config as $config) {
-          $arrayConfig[] = (array)$config;
-      }
-      $this->config = $arrayConfig;
-      if(count($this->config) < 6) {
-          $this->markTestSkipped('Config file '.$configFile.' should contain at least 6 entries');
-          return;
-      }
-    }
+    parent::setUp();
 
-    if($this->useStandalone && !extension_loaded('redis')) {
-      $this->fail('The Redis extension is not loaded.');
-    }
-    $clients = array_slice($this->config,0,4);
+    $clients = array_slice($this->redisConfig,0,4);
     $this->cluster = new Credis_Cluster($clients,2,$this->useStandalone);
   }
 
   protected function tearDown()
   {
     if($this->cluster) {
+      $this->cluster->flushAll();
       foreach($this->cluster->clients() as $client){
         if($client->isConnected()) {
-            $client->flushAll();
             $client->close();
         }
       }
@@ -57,7 +33,7 @@ protected function tearDown()
   public function testKeyHashing()
   {
       $this->tearDown();
-      $this->cluster = new Credis_Cluster(array_slice($this->config,0,3),2,$this->useStandalone);
+      $this->cluster = new Credis_Cluster(array_slice($this->redisConfig, 0, 3), 2, $this->useStandalone);
       $keys = array();
       $lines = explode("\n", file_get_contents("keys.test"));
       foreach ($lines as $line) {
@@ -69,7 +45,7 @@ public function testKeyHashing()
       foreach ($keys as $key => $value) {
           $this->assertTrue($this->cluster->set($key, $value));
       }
-      $this->cluster = new Credis_Cluster(array_slice($this->config,0,4),2,true,$this->useStandalone);
+      $this->cluster = new Credis_Cluster(array_slice($this->redisConfig, 0, 4), 2, true, $this->useStandalone);
       $hits = 0;
       foreach ($keys as $key => $value) {
           if ($this->cluster->all('get',$key)) {
@@ -80,7 +56,7 @@ public function testKeyHashing()
   }
   public function testAlias()
   {
-      $slicedConfig = array_slice($this->config,0,4);
+      $slicedConfig = array_slice($this->redisConfig, 0, 4);
       foreach($slicedConfig as $config) {
           $this->assertEquals($config['port'],$this->cluster->client($config['alias'])->getPort());
       }
@@ -94,8 +70,9 @@ public function testAlias()
   public function testMasterSlave()
   {
       $this->tearDown();
-      $this->cluster = new Credis_Cluster(array($this->config[0],$this->config[6]),2,$this->useStandalone);
+      $this->cluster = new Credis_Cluster(array($this->redisConfig[0],$this->redisConfig[6]), 2, $this->useStandalone);
       $this->assertTrue($this->cluster->client('master')->set('key','value'));
+      $this->waitForSlaveReplication();
       $this->assertEquals('value',$this->cluster->client('slave')->get('key'));
       $this->assertEquals('value',$this->cluster->get('key'));
       try
@@ -108,18 +85,20 @@ public function testMasterSlave()
       }
 
       $this->tearDown();
-      $writeOnlyConfig = $this->config[0];
+      $writeOnlyConfig = $this->redisConfig[0];
       $writeOnlyConfig['write_only'] = true;
-      $this->cluster = new Credis_Cluster(array($writeOnlyConfig,$this->config[6]),2,$this->useStandalone);
+      $this->cluster = new Credis_Cluster(array($writeOnlyConfig,$this->redisConfig[6]), 2, $this->useStandalone);
       $this->assertTrue($this->cluster->client('master')->set('key','value'));
+      $this->waitForSlaveReplication();
       $this->assertEquals('value',$this->cluster->client('slave')->get('key'));
-      $this->assertFalse($this->cluster->client('slave')->set('key2','value'));
       $this->assertEquals('value',$this->cluster->get('key'));
+      $this->expectException('CredisException','read-only slaves should not be writeable');
+      $this->assertFalse($this->cluster->client('slave')->set('key2','value'));
   }
   public function testMasterWithoutSlavesAndWriteOnlyFlag()
   {
       $this->tearDown();
-      $writeOnlyConfig = $this->config[0];
+      $writeOnlyConfig = $this->redisConfig[0];
       $writeOnlyConfig['write_only'] = true;
       $this->cluster = new Credis_Cluster(array($writeOnlyConfig),2,$this->useStandalone);
       $this->assertTrue($this->cluster->set('key','value'));
@@ -200,9 +179,9 @@ public function testRwsplit()
   public function testCredisClientInstancesInConstructor()
   {
       $this->tearDown();
-      $two = new Credis_Client($this->config[1]['host'],$this->config[1]['port']);
-      $three = new Credis_Client($this->config[2]['host'],$this->config[2]['port']);
-      $four = new Credis_Client($this->config[3]['host'],$this->config[3]['port']);
+      $two = new Credis_Client($this->redisConfig[1]['host'], $this->redisConfig[1]['port']);
+      $three = new Credis_Client($this->redisConfig[2]['host'], $this->redisConfig[2]['port']);
+      $four = new Credis_Client($this->redisConfig[3]['host'], $this->redisConfig[3]['port']);
       $this->cluster = new Credis_Cluster(array($two,$three,$four),2,$this->useStandalone);
       $this->assertTrue($this->cluster->set('key','value'));
       $this->assertEquals('value',$this->cluster->get('key'));
@@ -212,18 +191,18 @@ public function testCredisClientInstancesInConstructor()
   public function testSetMasterClient()
   {
       $this->tearDown();
-      $master = new Credis_Client($this->config[0]['host'],$this->config[0]['port']);
-      $slave = new Credis_Client($this->config[6]['host'],$this->config[6]['port']);
+      $master = new Credis_Client($this->redisConfig[0]['host'], $this->redisConfig[0]['port']);
+      $slave = new Credis_Client($this->redisConfig[6]['host'], $this->redisConfig[6]['port']);
 
       $this->cluster = new Credis_Cluster(array($slave),2,$this->useStandalone);
       $this->assertInstanceOf('Credis_Cluster',$this->cluster->setMasterClient($master));
       $this->assertCount(2,$this->cluster->clients());
-      $this->assertEquals($this->config[6]['port'],$this->cluster->client(0)->getPort());
-      $this->assertEquals($this->config[0]['port'],$this->cluster->client('master')->getPort());
+      $this->assertEquals($this->redisConfig[6]['port'], $this->cluster->client(0)->getPort());
+      $this->assertEquals($this->redisConfig[0]['port'], $this->cluster->client('master')->getPort());
 
-      $this->cluster = new Credis_Cluster(array($this->config[0]),2,$this->useStandalone);
-      $this->assertInstanceOf('Credis_Cluster',$this->cluster->setMasterClient(new Credis_Client($this->config[1]['host'],$this->config[1]['port'])));
-      $this->assertEquals($this->config[0]['port'],$this->cluster->client('master')->getPort());
+      $this->cluster = new Credis_Cluster(array($this->redisConfig[0]), 2, $this->useStandalone);
+      $this->assertInstanceOf('Credis_Cluster',$this->cluster->setMasterClient(new Credis_Client($this->redisConfig[1]['host'], $this->redisConfig[1]['port'])));
+      $this->assertEquals($this->redisConfig[0]['port'], $this->cluster->client('master')->getPort());
 
       $this->cluster = new Credis_Cluster(array($slave),2,$this->useStandalone);
       $this->assertInstanceOf('Credis_Cluster',$this->cluster->setMasterClient($master,true));

diff --git a/tests/CredisSentinelTest.php b/tests/CredisSentinelTest.php
@@ -7,17 +7,14 @@
 
 class CredisSentinelTest extends CredisTestCommon
 {
-
   /** @var Credis_Sentinel */
   protected $sentinel;
 
   protected $sentinelConfig;
-  protected $redisConfig;
-
-  protected $useStandalone = FALSE;
 
   protected function setUp()
   {
+    parent::setUp();
     if($this->sentinelConfig === NULL) {
       $configFile = dirname(__FILE__).'/sentinel_config.json';
       if( ! file_exists($configFile) || ! ($config = file_get_contents($configFile))) {
@@ -27,26 +24,12 @@ protected function setUp()
       $this->sentinelConfig = json_decode($config);
     }
 
-    if($this->redisConfig === NULL) {
-        $configFile = dirname(__FILE__).'/redis_config.json';
-        if( ! file_exists($configFile) || ! ($config = file_get_contents($configFile))) {
-            $this->markTestSkipped('Could not load '.$configFile);
-            return;
-        }
-        $this->redisConfig = json_decode($config);
-        $arrayConfig = array();
-        foreach($this->redisConfig as $config) {
-            $arrayConfig[] = (array)$config;
-        }
-        $this->redisConfig = $arrayConfig;
-    }
     $sentinelClient = new Credis_Client($this->sentinelConfig->host, $this->sentinelConfig->port);
     $this->sentinel = new Credis_Sentinel($sentinelClient);
     if($this->useStandalone) {
       $this->sentinel->forceStandalone();
-    } else if ( ! extension_loaded('redis')) {
-      $this->fail('The Redis extension is not loaded.');
     }
+    $this->waitForSlaveReplication();
   }
   protected function tearDown()
   {
@@ -172,7 +155,7 @@ public function testGetHostAndPort()
       $host = 'localhost';
       $port = '123456';
 
-      $client = $this->getMock('\Credis_Client');
+      $client = $this->createMock('\Credis_Client');
       $sentinel = new Credis_Sentinel($client);
 
       $client->expects($this->once())->method('getHost')->willReturn($host);