jenkins-bot has submitted this change and it was merged.
Change subject: Reset WatchedItemStore default instance after tests
......................................................................
Reset WatchedItemStore default instance after tests
Prior to this change in tests the overridden store
would remain in the instance static and thus could
be used in other places.
This patch introduces the used of ScopedCallbacks
in the override methods in WatchedItemStore.
This means that any instance of WatchedItemStore
should return to a regular state after each test.
This is better than requiring the tests to reset
the value back to the origional as this would likely
be forgotten and result in long hunts for failing
tests.
This was found while writing more tests...
Change-Id: I9aa71425642174ae9ea2c6d4f85dcd07d724af11
---
M includes/WatchedItemStore.php
M tests/phpunit/includes/WatchedItemStoreUnitTest.php
M tests/phpunit/includes/WatchedItemUnitTest.php
3 files changed, 61 insertions(+), 11 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php
index 1a9868e..11eb60c 100644
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -64,8 +64,10 @@
* This is intended for use while testing and will fail if
MW_PHPUNIT_TEST is not defined.
*
* @param callable $callback
+ *
* @see DeferredUpdates::addCallableUpdate for callback signiture
*
+ * @return ScopedCallback to reset the overridden value
* @throws MWException
*/
public function overrideDeferredUpdatesAddCallableUpdateCallback(
$callback ) {
@@ -75,7 +77,12 @@
);
}
Assert::parameterType( 'callable', $callback, '$callback' );
+
+ $previousValue =
$this->deferredUpdatesAddCallableUpdateCallback;
$this->deferredUpdatesAddCallableUpdateCallback = $callback;
+ return new ScopedCallback( function() use ( $previousValue ) {
+ $this->deferredUpdatesAddCallableUpdateCallback =
$previousValue;
+ } );
}
/**
@@ -85,6 +92,7 @@
* @param callable $callback
* @see Revision::getTimestampFromId for callback signiture
*
+ * @return ScopedCallback to reset the overridden value
* @throws MWException
*/
public function overrideRevisionGetTimestampFromIdCallback( $callback )
{
@@ -94,24 +102,38 @@
);
}
Assert::parameterType( 'callable', $callback, '$callback' );
+
+ $previousValue = $this->revisionGetTimestampFromIdCallback;
$this->revisionGetTimestampFromIdCallback = $callback;
+ return new ScopedCallback( function() use ( $previousValue ) {
+ $this->revisionGetTimestampFromIdCallback =
$previousValue;
+ } );
}
/**
* Overrides the default instance of this class
* This is intended for use while testing and will fail if
MW_PHPUNIT_TEST is not defined.
*
- * @param WatchedItemStore $store
+ * If this method is used it MUST also be called with null after a test
to ensure a new
+ * default instance is created next time getDefaultInstance is called.
*
+ * @param WatchedItemStore|null $store
+ *
+ * @return ScopedCallback to reset the overridden value
* @throws MWException
*/
- public static function overrideDefaultInstance( WatchedItemStore $store
) {
+ public static function overrideDefaultInstance( WatchedItemStore $store
= null ) {
if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
throw new MWException(
'Cannot override ' . __CLASS__ . 'default
instance in operation.'
);
}
+
+ $previousValue = self::$instance;
self::$instance = $store;
+ return new ScopedCallback( function() use ( $previousValue ) {
+ self::$instance = $previousValue;
+ } );
}
/**
diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
index 1dd819b..e483580 100644
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -81,6 +81,17 @@
$this->assertSame( $instanceOne, $instanceTwo );
}
+ public function testOverrideDefaultInstance() {
+ $instance = WatchedItemStore::getDefaultInstance();
+ $scopedOverride = $instance->overrideDefaultInstance( null );
+
+ $this->assertNotSame( $instance,
WatchedItemStore::getDefaultInstance() );
+
+ unset( $scopedOverride );
+
+ $this->assertSame( $instance,
WatchedItemStore::getDefaultInstance() );
+ }
+
public function testCountWatchers() {
$titleValue = new TitleValue( 0, 'SomeDbKey' );
@@ -1299,7 +1310,7 @@
$callableCallCounter++;
$this->assertInternalType( 'callable', $callable );
};
- $store->overrideDeferredUpdatesAddCallableUpdateCallback(
$mockCallback );
+ $scopedOverride =
$store->overrideDeferredUpdatesAddCallableUpdateCallback( $mockCallback );
$this->assertTrue(
$store->resetNotificationTimestamp(
@@ -1308,6 +1319,8 @@
)
);
$this->assertEquals( 1, $callableCallCounter );
+
+ ScopedCallback::consume( $scopedOverride );
}
public function testResetNotificationTimestamp_noItemForced() {
@@ -1337,7 +1350,7 @@
$callableCallCounter++;
$this->assertInternalType( 'callable', $callable );
};
- $store->overrideDeferredUpdatesAddCallableUpdateCallback(
$mockCallback );
+ $scopedOverride =
$store->overrideDeferredUpdatesAddCallableUpdateCallback( $mockCallback );
$this->assertTrue(
$store->resetNotificationTimestamp(
@@ -1347,6 +1360,8 @@
)
);
$this->assertEquals( 1, $callableCallCounter );
+
+ ScopedCallback::consume( $scopedOverride );
}
/**
@@ -1397,7 +1412,7 @@
// Note: This does not actually assert the job is correct
$callableCallCounter = 0;
- $store->overrideDeferredUpdatesAddCallableUpdateCallback(
+ $scopedOverride =
$store->overrideDeferredUpdatesAddCallableUpdateCallback(
function( $callable ) use ( &$callableCallCounter ) {
$callableCallCounter++;
$this->assertInternalType( 'callable',
$callable );
@@ -1413,6 +1428,8 @@
)
);
$this->assertEquals( 1, $callableCallCounter );
+
+ ScopedCallback::consume( $scopedOverride );
}
public function
testResetNotificationTimestamp_oldidSpecifiedNotLatestRevisionForced() {
@@ -1455,7 +1472,7 @@
// Note: This does not actually assert the job is correct
$addUpdateCallCounter = 0;
- $store->overrideDeferredUpdatesAddCallableUpdateCallback(
+ $scopedOverrideDeferred =
$store->overrideDeferredUpdatesAddCallableUpdateCallback(
function( $callable ) use ( &$addUpdateCallCounter ) {
$addUpdateCallCounter++;
$this->assertInternalType( 'callable',
$callable );
@@ -1463,7 +1480,7 @@
);
$getTimestampCallCounter = 0;
- $store->overrideRevisionGetTimestampFromIdCallback(
+ $scopedOverrideRevision =
$store->overrideRevisionGetTimestampFromIdCallback(
function( $titleParam, $oldidParam ) use (
&$getTimestampCallCounter, $title, $oldid ) {
$getTimestampCallCounter++;
$this->assertEquals( $title, $titleParam );
@@ -1481,6 +1498,9 @@
);
$this->assertEquals( 1, $addUpdateCallCounter );
$this->assertEquals( 1, $getTimestampCallCounter );
+
+ ScopedCallback::consume( $scopedOverrideDeferred );
+ ScopedCallback::consume( $scopedOverrideRevision );
}
public function testUpdateNotificationTimestamp_watchersExist() {
diff --git a/tests/phpunit/includes/WatchedItemUnitTest.php
b/tests/phpunit/includes/WatchedItemUnitTest.php
index bc37311..58984cf 100644
--- a/tests/phpunit/includes/WatchedItemUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemUnitTest.php
@@ -51,13 +51,15 @@
->method( 'loadWatchedItem' )
->with( $user, $linkTarget )
->will( $this->returnValue( new WatchedItem( $user,
$linkTarget, $timestamp ) ) );
- WatchedItemStore::overrideDefaultInstance( $store );
+ $scopedOverride = WatchedItemStore::overrideDefaultInstance(
$store );
$item = WatchedItem::fromUserTitle( $user, $linkTarget,
User::IGNORE_USER_RIGHTS );
$this->assertEquals( $user, $item->getUser() );
$this->assertEquals( $linkTarget, $item->getLinkTarget() );
$this->assertEquals( $timestamp,
$item->getNotificationTimestamp() );
+
+ ScopedCallback::consume( $scopedOverride );
}
/**
@@ -83,10 +85,12 @@
return true;
}
) );
- WatchedItemStore::overrideDefaultInstance( $store );
+ $scopedOverride = WatchedItemStore::overrideDefaultInstance(
$store );
$item = new WatchedItem( $user, $linkTarget, $timestamp );
$item->resetNotificationTimestamp( $force, $oldid );
+
+ ScopedCallback::consume( $scopedOverride );
}
public function testAddWatch() {
@@ -153,9 +157,11 @@
$store->expects( $this->once() )
->method( 'duplicateAllAssociatedEntries' )
->with( $oldTitle, $newTitle );
- WatchedItemStore::overrideDefaultInstance( $store );
+ $scopedOverride = WatchedItemStore::overrideDefaultInstance(
$store );
WatchedItem::duplicateEntries( $oldTitle, $newTitle );
+
+ ScopedCallback::consume( $scopedOverride );
}
public function testBatchAddWatch() {
@@ -175,9 +181,11 @@
$store->expects( $this->once() )
->method( 'addWatchBatch' )
->with( $userTargetCombinations );
- WatchedItemStore::overrideDefaultInstance( $store );
+ $scopedOverride = WatchedItemStore::overrideDefaultInstance(
$store );
WatchedItem::batchAddWatch( $items );
+
+ ScopedCallback::consume( $scopedOverride );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/277435
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9aa71425642174ae9ea2c6d4f85dcd07d724af11
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Florianschmidtwelzow <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Reedy <[email protected]>
Gerrit-Reviewer: WMDE-Fisch <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
