Fix RoundRobinSelector Tests (#617)

Also adjusts the behavior of the selector slightly, so that the first of the connections is chosen instead of (previously) the second. Doesn't particularly matter since connections are shuffled before use, but is the "correct" way to do things regardless.
elastic · Aug 18, 2017 · 23a0ba8 · 23a0ba8
1 parent b5d9613
commit 23a0ba8
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 34 deletions.
diff --git a/src/Elasticsearch/ConnectionPool/Selectors/RoundRobinSelector.php b/src/Elasticsearch/ConnectionPool/Selectors/RoundRobinSelector.php
@@ -29,8 +29,10 @@ class RoundRobinSelector implements SelectorInterface
      */
     public function select($connections)
     {
+        $returnConnection = $connections[$this->current % count($connections)];
+
         $this->current += 1;
 
-        return $connections[$this->current % count($connections)];
+        return $returnConnection;
     }
 }
diff --git a/tests/Elasticsearch/Tests/ConnectionPool/Selectors/RoundRobinSelectorTest.php b/tests/Elasticsearch/Tests/ConnectionPool/Selectors/RoundRobinSelectorTest.php
@@ -5,6 +5,7 @@
 namespace Elasticsearch\Tests\ConnectionPool\Selectors;
 
 use Elasticsearch;
+use Elasticsearch\Connections\ConnectionInterface;
 
 /**
  * Class SnifferTest
@@ -29,19 +30,32 @@ public function testTenConnections()
     {
         $roundRobin = new Elasticsearch\ConnectionPool\Selectors\RoundRobinSelector();
 
-        $mockConnections = array();
-        foreach (range(0, 10) as $index) {
-            $mockConnections[$index] = $this->getMockBuilder('\Elasticsearch\Connections\CurlMultiConnection')
+        $mockConnections = [];
+        foreach (range(0, 9) as $index) {
+            $mockConnections[$index] = $this->getMockBuilder(ConnectionInterface::class)
                 ->disableOriginalConstructor()
                 ->getMock();
         }
 
-        foreach (range(0, 15) as $index) {
-            $retConnection = $roundRobin->select($mockConnections);
+        // select ten
+        $this->assertSame($mockConnections[0], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[1], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[2], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[3], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[4], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[5], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[6], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[7], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[8], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[9], $roundRobin->select($mockConnections));
+
+        // select five - should start from the first one (index: 0)
+        $this->assertSame($mockConnections[0], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[1], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[2], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[3], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[4], $roundRobin->select($mockConnections));
 
-            $nextIndex = ($index % 10) + 1;
-            $this->assertEquals($mockConnections[$nextIndex], $retConnection);
-        }
     }
 
     /**
@@ -52,33 +66,39 @@ public function testTenConnections()
      *
      * @return void
      */
-    public function testAddTenConnectionsestFiveTRemoveThree()
+    public function testAddTenConnectionsTestFiveRemoveThreeTestTen()
     {
         $roundRobin = new Elasticsearch\ConnectionPool\Selectors\RoundRobinSelector();
 
-        $mockConnections = array();
-        foreach (range(0, 10) as $index) {
-            $mockConnections[$index] = $this->getMockBuilder('\Elasticsearch\Connections\CurlMultiConnection')
+        $mockConnections = [];
+        foreach (range(0, 9) as $index) {
+            $mockConnections[$index] = $this->getMockBuilder(ConnectionInterface::class)
                 ->disableOriginalConstructor()
                 ->getMock();
         }
 
-        foreach (range(0, 4) as $index) {
-            $retConnection = $roundRobin->select($mockConnections);
-
-            $nextIndex = ($index % (count($mockConnections)-1)) + 1;
-            $this->assertEquals($mockConnections[$nextIndex], $retConnection);
-        }
+        // select five
+        $this->assertSame($mockConnections[0], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[1], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[2], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[3], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[4], $roundRobin->select($mockConnections));
 
+        // remove three
         unset($mockConnections[8]);
         unset($mockConnections[9]);
         unset($mockConnections[10]);
 
-        foreach (range(5, 15) as $index) {
-            $retConnection = $roundRobin->select($mockConnections);
-
-            $nextIndex = ($index % (count($mockConnections)-1)) + 1;
-            $this->assertEquals($mockConnections[$nextIndex], $retConnection);
-        }
+        // select ten after removal
+        $this->assertSame($mockConnections[5], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[6], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[7], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[0], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[1], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[2], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[3], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[4], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[5], $roundRobin->select($mockConnections));
+        $this->assertSame($mockConnections[6], $roundRobin->select($mockConnections));
     }
 }
diff --git a/tests/Elasticsearch/Tests/ConnectionPool/Selectors/StickyRoundRobinSelectorTest.php b/tests/Elasticsearch/Tests/ConnectionPool/Selectors/StickyRoundRobinSelectorTest.php
@@ -5,6 +5,7 @@
 namespace Elasticsearch\Tests\ConnectionPool\Selectors;
 
 use Elasticsearch;
+use Elasticsearch\Connections\ConnectionInterface;
 use Mockery as m;
 
 /**
@@ -28,40 +29,40 @@ public function testTenConnections()
     {
         $roundRobin = new Elasticsearch\ConnectionPool\Selectors\StickyRoundRobinSelector();
 
-        $mockConnections = array();
-        $mockConnections[] = m::mock('\Elasticsearch\Connections\GuzzleConnection')
+        $mockConnections = [];
+        $mockConnections[] = m::mock(ConnectionInterface::class)
                              ->shouldReceive('isAlive')->times(16)->andReturn(true)->getMock();
 
         foreach (range(0, 9) as $index) {
-            $mockConnections[] = m::mock('\Elasticsearch\Connections\GuzzleConnection');
+            $mockConnections[] = m::mock(ConnectionInterface::class);
         }
 
         foreach (range(0, 15) as $index) {
             $retConnection = $roundRobin->select($mockConnections);
 
-            $this->assertEquals($mockConnections[0], $retConnection);
+            $this->assertSame($mockConnections[0], $retConnection);
         }
     }
 
     public function testTenConnectionsFirstDies()
     {
         $roundRobin = new Elasticsearch\ConnectionPool\Selectors\StickyRoundRobinSelector();
 
-        $mockConnections = array();
-        $mockConnections[] = m::mock('\Elasticsearch\Connections\GuzzleConnection')
+        $mockConnections = [];
+        $mockConnections[] = m::mock(ConnectionInterface::class)
                              ->shouldReceive('isAlive')->once()->andReturn(false)->getMock();
 
-        $mockConnections[] = m::mock('\Elasticsearch\Connections\GuzzleConnection')
+        $mockConnections[] = m::mock(ConnectionInterface::class)
                              ->shouldReceive('isAlive')->times(15)->andReturn(true)->getMock();
 
         foreach (range(0, 8) as $index) {
-            $mockConnections[] = m::mock('\Elasticsearch\Connections\GuzzleConnection');
+            $mockConnections[] = m::mock(ConnectionInterface::class);
         }
 
         foreach (range(0, 15) as $index) {
             $retConnection = $roundRobin->select($mockConnections);
 
-            $this->assertEquals($mockConnections[1], $retConnection);
+            $this->assertSame($mockConnections[1], $retConnection);
         }
     }
 }