Krinkle has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/350508 )
Change subject: [WIP] Add unit test for EtcdConfig
......................................................................
[WIP] Add unit test for EtcdConfig
Change-Id: I6e647985b98724b5448831a6fae1b2d2521fd363
---
M includes/config/EtcdConfig.php
A tests/phpunit/includes/config/EtcdConfigTest.php
M tests/phpunit/suite.xml
3 files changed, 266 insertions(+), 5 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/08/350508/1
diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php
index d3fbd65..09d1761 100644
--- a/includes/config/EtcdConfig.php
+++ b/includes/config/EtcdConfig.php
@@ -88,7 +88,7 @@
$this->timeout = $params['timeout'];
if ( !isset( $params['cache'] ) ) {
- $this->srvCache = new HashBagOStuff( [] );
+ $this->srvCache = new HashBagOStuff();
} elseif ( $params['cache'] instanceof BagOStuff ) {
$this->srvCache = $params['cache'];
} else {
@@ -122,6 +122,37 @@
return $this->procCache['config'][$name];
}
+ /**
+ * Scenarios:
+ * - Cache exists:
+ * - If not expired:
+ * - Use the cached value. (fresh cache)
+ * - If expired:
+ * - If another process already has the lock:
+ * - Use the cached value. (stale cache)
+ * - If we get the lock:
+ * - Backend fetch succeeds:
+ * - Use the fetched value. (fresh fetch)
+ * - Backend fetch fails:
+ * - Throws!
+ * - Backend fetch times out:
+ * - Retries.
+ * - Retry loop times out:
+ * - Throws!
+ * - Cache is empty:
+ * - If another process already has the lock:
+ * - Retry reading the cache.
+ * - If we get the lock:
+ * - Backend fetch succeeds:
+ * - Use the fetched value. (fresh fetch)
+ * - Backend fetch fails:
+ * - Throws!
+ * - Backend fetch times out:
+ * - Retries.
+ * - Retry loop times out:
+ * - Throws!
+ * @throws ConfigException
+ */
private function load() {
if ( $this->procCache !== null ) {
return; // already loaded
diff --git a/tests/phpunit/includes/config/EtcdConfigTest.php
b/tests/phpunit/includes/config/EtcdConfigTest.php
new file mode 100644
index 0000000..f526693
--- /dev/null
+++ b/tests/phpunit/includes/config/EtcdConfigTest.php
@@ -0,0 +1,233 @@
+<?php
+
+class EtcConfigTest extends PHPUnit_Framework_TestCase {
+
+ private function createConfigMock( array $options = [] ) {
+ return $this->getMockBuilder( EtcdConfig::class )
+ ->setConstructorArgs( [ $options + [
+ 'host' => 'etcd-tcp.example.net',
+ 'directory' => '/',
+ 'timeout' => 0.1,
+ ] ] )
+ ->setMethods( [ 'fetchAllFromEtcd' ] )
+ ->getMock();
+ }
+
+ private function createSimpleConfigMock( array $config ) {
+ $mock = $this->createConfigMock();
+ $mock->expects( $this->once() )
+ ->method( 'fetchAllFromEtcd' )
+ ->willReturn( [
+ $config,
+ null, // error
+ false // retry?
+ ] );
+ return $mock;
+ }
+
+ /**
+ * @covers EtcdConfig::has
+ * @covers EtcdConfig::get
+ */
+ public function testKnownKey() {
+ $config = $this->createSimpleConfigMock( [
+ 'known' => 'value'
+ ] );
+
+ $this->assertSame(
+ true,
+ $config->has( 'known' ),
+ 'Has key'
+ );
+
+ $this->assertSame(
+ 'value',
+ $config->get( 'known' ),
+ 'Get key'
+ );
+ }
+
+ /**
+ * @covers EtcdConfig::has
+ */
+ public function testHasUnknown() {
+ $config = $this->createSimpleConfigMock( [
+ 'known' => 'value'
+ ] );
+
+ $this->assertSame(
+ false,
+ $config->has( 'unknown' ),
+ 'Has key'
+ );
+ }
+
+ /**
+ * @covers EtcdConfig::get
+ */
+ public function testGetUnknown() {
+ $config = $this->createSimpleConfigMock( [
+ 'known' => 'value'
+ ] );
+
+ $this->setExpectedException( ConfigException::class );
+ $config->get( 'unknown' );
+ }
+
+ public static function provideLoadScenarios() {
+ // Format:
+ // - cache: true, false, or "expired"
+ // - lock: true, or false
+ // - backend: "success", "fail", "retry"
+ // - retry: true, or false (Whether a retry should succeed)
+ // - expected:
+ // - "fresh" Fetched from a backend.
+ // - "fresh-retried" Fetched from a backend after retry.
+ // - "cache" Fetched from cache (not expired).
+ // - "stale-cache" Fetched from cache but known stale.
+ return [
+ // cache miss, gets lock
+ [ [
+ 'cache' => false, 'lock' => true, 'backend' =>
'success',
+ 'expected' => 'fresh',
+ ] ],
+ [ [
+ 'cache' => false, 'lock' => true, 'backend' =>
'fail',
+ 'expected' => false,
+ ] ],
+ [ [
+ 'cache' => false, 'lock' => true, 'backend' =>
'retry', 'retry' => true,
+ 'expected' => 'fresh-retried',
+ ] ],
+ [ [
+ 'cache' => false, 'lock' => true, 'backend' =>
'retry', 'retry' => false,
+ 'expected' => false,
+ ] ],
+ // cache miss, doesn't get lock
+ [ [
+ 'cache' => false, 'lock' => false, 'retry' =>
true,
+ // Populated by another process after retry
+ 'expected' => 'cache',
+ ] ],
+ [ [
+ 'cache' => false, 'lock' => false, 'retry' =>
false,
+ 'expected' => false,
+ ] ],
+ // cache hit
+ [ [
+ 'cache' => true,
+ 'expected' => 'cache',
+ ] ],
+ // cache expired, gets lock
+ [ [
+ 'cache' => 'expired', 'lock' => true, 'backend'
=> 'success',
+ 'expected' => 'fresh',
+ ] ],
+ [ [
+ 'cache' => 'expired', 'lock' => true, 'backend'
=> 'fail',
+ 'expected' => false,
+ ] ],
+ // cache expired, doesn't get lock
+ [ [
+ 'cache' => 'expired', 'lock' => false,
+ 'expected' => 'cache-stale',
+ ] ],
+ ];
+ }
+
+ /**
+ * @dataProvider provideLoadScenarios
+ */
+ public function testLoadScenarios( array $case ) {
+ $case += [ 'lock' => false, 'backend' => null ];
+
+ $cache = $this->getMockBuilder( HashBagOStuff::class )
+ ->setConstructorArgs( [] )
+ ->setMethods( [ 'get', 'lock' ] )
+ ->getMock();
+ $cacheKey = $cache->makeKey( 'variable', sha1( '/' ) );
+
+ $cacheValue = [
+ 'config' => [ 'key' => 'cache' ],
+ 'expires' => time() * 2,
+ ];
+ $cacheStaleValue = [
+ 'config' => [ 'key' => 'cache-stale' ],
+ 'expires' => 1,
+ ];
+
+ // Prepare lock
+ $cache->expects( $this->any() )
+ ->method( 'lock' )
+ ->willReturn( $case['lock'] );
+
+
+ // Prepare cache
+ if ( $case['cache'] === true ) {
+ $firstCache = $cacheValue;
+ } elseif ( $case['cache'] === 'expired' ) {
+ $firstCache = $cacheStaleValue;
+ } else {
+ $firstCache = false;
+ }
+ if ( $case['lock'] === false && $case['cache'] === false &&
$case['retry'] === true ) {
+ $cache->expects( $this->exactly( 2 ) )
+ ->method( 'get' )
+ ->will( $this->onConsecutiveCalls(
+ $firstCache,
+ $cacheValue
+ ) );
+ } else {
+ $cache->expects( $this->any() )
+ ->method( 'get' )
+ ->willReturn( $firstCache );
+ }
+
+ // Create mock
+ $mock = $this->createConfigMock( [
+ 'cache' => $cache,
+ ] );
+
+ // Set up mock backend
+ if ( $case['backend'] === null ) {
+ $mock->expects( $this->never() )
+ ->method( 'fetchAllFromEtcd' );
+ } else {
+ if ( $case['backend'] === 'success' ) {
+ $fetch = [ [ 'key' => 'fresh' ], null, false ];
+ $mock->expects( $this->once() )
+ ->method( 'fetchAllFromEtcd' )
+ ->willReturn( $fetch );
+ } elseif ( $case['backend'] === 'fail' ) {
+ $fetch = [ null, 'Fake failure', false ];
+ $mock->expects( $this->once() )
+ ->method( 'fetchAllFromEtcd' )
+ ->willReturn( $fetch );
+ } elseif ( $case['backend'] === 'retry' ) {
+ $firstFetch = [ null, 'Fake retryable failure',
true ];
+ if ( $case['retry'] === true ) {
+ $secondFetch = [ [ 'key' =>
'fresh-retried' ], null, false ];
+ } else {
+ $secondFetch = $firstFetch;
+ }
+ $mock->expects( $this->atLeastOnce() )
+ ->method( 'fetchAllFromEtcd' )
+ ->will( $this->onConsecutiveCalls(
+ $firstFetch,
+ $secondFetch
+ ) );
+ }
+ }
+
+ if ( $case['expected'] == false ) {
+ $this->setExpectedException( ConfigException::class );
+ $mock->get( 'key' );
+ } else {
+ $this->assertSame(
+ $case['expected'],
+ $mock->get( 'key' ),
+ 'Get key'
+ );
+ }
+ }
+}
diff --git a/tests/phpunit/suite.xml b/tests/phpunit/suite.xml
index 7babcac..63c2568 100644
--- a/tests/phpunit/suite.xml
+++ b/tests/phpunit/suite.xml
@@ -64,10 +64,7 @@
</groups>
<filter>
<whitelist addUncoveredFilesFromWhitelist="true">
- <directory suffix=".php">../../includes</directory>
- <directory suffix=".php">../../languages</directory>
- <directory suffix=".php">../../maintenance</directory>
- <directory suffix=".php">../../skins</directory>
+ <directory
suffix=".php">../../includes/config</directory>
</whitelist>
</filter>
</phpunit>
--
To view, visit https://gerrit.wikimedia.org/r/350508
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e647985b98724b5448831a6fae1b2d2521fd363
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
