From 31c461e75e4265b55e30844102136b6dc35ee32a Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 12 Oct 2017 19:58:33 +0100 Subject: [PATCH] deferred: Improve DeferredUpdates test coverage From 1% of lines to 12% in deferred/. From 6% of lines to 68% in DeferredUpdates.php. * Adding relevant @covers tags to existing tests. * Add coverage for MWCallableUpdate. * Add coverage for MergeableUpdate. Also: * Make MergeableUpdate extend DeferrableUpdate. 1. Because PHPUnit doesn't support implementing multiple interfaces in a mock, and would make the mock fail the typehint at run-time. 2. It DeferredUpdates doesn't support having a MergeableUpdate that isn't itself a DeferrableUpdate given the only way to reach that code is past methods that are type-hinted with DeferrableUpdate. 3. Making the interface extend DeferrableUpdate helps produce better and earlier errors. Instead of run-time error: > Argument 1 passed to addUpdate() must implement interface DeferrableUpdate > instance of MergeableUpdate given We get: > Fatal error: Class .. contains 1 abstract method and must therefore be > declared abstract or implement the remaining methods (doUpdate) Change-Id: Ie384bf849a96bb37dc3e4a4154da2b02889e9fc8 --- includes/deferred/MergeableUpdate.php | 2 +- .../includes/deferred/CdnCacheUpdateTest.php | 4 + .../includes/deferred/DeferredUpdatesTest.php | 77 ++++++++++++++++++- .../deferred/MWCallableUpdateTest.php | 34 ++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) create mode 100644 tests/phpunit/includes/deferred/MWCallableUpdateTest.php diff --git a/includes/deferred/MergeableUpdate.php b/includes/deferred/MergeableUpdate.php index 70760ce49c..8eeef13bbe 100644 --- a/includes/deferred/MergeableUpdate.php +++ b/includes/deferred/MergeableUpdate.php @@ -6,7 +6,7 @@ * * @since 1.27 */ -interface MergeableUpdate { +interface MergeableUpdate extends DeferrableUpdate { /** * Merge this update with $update * diff --git a/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php b/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php index 11b869a7db..f3c949d3d0 100644 --- a/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php +++ b/tests/phpunit/includes/deferred/CdnCacheUpdateTest.php @@ -3,6 +3,10 @@ use Wikimedia\TestingAccessWrapper; class CdnCacheUpdateTest extends MediaWikiTestCase { + + /** + * @covers CdnCacheUpdate::merge + */ public function testPurgeMergeWeb() { $this->setMwGlobals( 'wgCommandLineMode', false ); diff --git a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php index 3b423563ca..999ad03935 100644 --- a/tests/phpunit/includes/deferred/DeferredUpdatesTest.php +++ b/tests/phpunit/includes/deferred/DeferredUpdatesTest.php @@ -2,11 +2,65 @@ class DeferredUpdatesTest extends MediaWikiTestCase { + /** + * @covers DeferredUpdates::addUpdate + * @covers DeferredUpdates::push + * @covers DeferredUpdates::doUpdates + * @covers DeferredUpdates::execute + * @covers DeferredUpdates::runUpdate + */ + public function testAddAndRun() { + $update = $this->getMockBuilder( DeferrableUpdate::class ) + ->setMethods( [ 'doUpdate' ] )->getMock(); + $update->expects( $this->once() )->method( 'doUpdate' ); + + DeferredUpdates::addUpdate( $update ); + DeferredUpdates::doUpdates(); + } + + /** + * @covers DeferredUpdates::addUpdate + * @covers DeferredUpdates::push + */ + public function testAddMergeable() { + $this->setMwGlobals( 'wgCommandLineMode', false ); + + $update1 = $this->getMockBuilder( MergeableUpdate::class ) + ->setMethods( [ 'merge', 'doUpdate' ] )->getMock(); + $update1->expects( $this->once() )->method( 'merge' ); + $update1->expects( $this->never() )->method( 'doUpdate' ); + + $update2 = $this->getMockBuilder( MergeableUpdate::class ) + ->setMethods( [ 'merge', 'doUpdate' ] )->getMock(); + $update2->expects( $this->never() )->method( 'merge' ); + $update2->expects( $this->never() )->method( 'doUpdate' ); + + DeferredUpdates::addUpdate( $update1 ); + DeferredUpdates::addUpdate( $update2 ); + } + + /** + * @covers DeferredUpdates::addCallableUpdate + * @covers MWCallableUpdate::getOrigin + */ + public function testAddCallableUpdate() { + $this->setMwGlobals( 'wgCommandLineMode', true ); + + $ran = 0; + DeferredUpdates::addCallableUpdate( function () use ( &$ran ) { + $ran++; + } ); + DeferredUpdates::doUpdates(); + + $this->assertSame( 1, $ran, 'Update ran' ); + } + /** * @covers DeferredUpdates::getPendingUpdates + * @covers DeferredUpdates::clearPendingUpdates */ public function testGetPendingUpdates() { - # Prevent updates from running + // Prevent updates from running $this->setMwGlobals( 'wgCommandLineMode', false ); $pre = DeferredUpdates::PRESEND; @@ -34,6 +88,11 @@ class DeferredUpdatesTest extends MediaWikiTestCase { $this->assertCount( 0, DeferredUpdates::getPendingUpdates() ); } + /** + * @covers DeferredUpdates::doUpdates + * @covers DeferredUpdates::execute + * @covers DeferredUpdates::addUpdate + */ public function testDoUpdatesWeb() { $this->setMwGlobals( 'wgCommandLineMode', false ); @@ -126,6 +185,11 @@ class DeferredUpdatesTest extends MediaWikiTestCase { $this->assertEquals( "Marychu", $y, "POSTSEND update ran" ); } + /** + * @covers DeferredUpdates::doUpdates + * @covers DeferredUpdates::execute + * @covers DeferredUpdates::addUpdate + */ public function testDoUpdatesCLI() { $this->setMwGlobals( 'wgCommandLineMode', true ); $updates = [ @@ -140,7 +204,8 @@ class DeferredUpdatesTest extends MediaWikiTestCase { '3-2-1' => "deferred update 1 within deferred update 2 with deferred update 3;\n", ]; - wfGetLBFactory()->commitMasterChanges( __METHOD__ ); // clear anything + // clear anything + wfGetLBFactory()->commitMasterChanges( __METHOD__ ); DeferredUpdates::addCallableUpdate( function () use ( $updates ) { @@ -193,12 +258,18 @@ class DeferredUpdatesTest extends MediaWikiTestCase { DeferredUpdates::doUpdates(); } + /** + * @covers DeferredUpdates::doUpdates + * @covers DeferredUpdates::execute + * @covers DeferredUpdates::addUpdate + */ public function testPresendAddOnPostsendRun() { $this->setMwGlobals( 'wgCommandLineMode', true ); $x = false; $y = false; - wfGetLBFactory()->commitMasterChanges( __METHOD__ ); // clear anything + // clear anything + wfGetLBFactory()->commitMasterChanges( __METHOD__ ); DeferredUpdates::addCallableUpdate( function () use ( &$x, &$y ) { diff --git a/tests/phpunit/includes/deferred/MWCallableUpdateTest.php b/tests/phpunit/includes/deferred/MWCallableUpdateTest.php new file mode 100644 index 0000000000..6995bf8b08 --- /dev/null +++ b/tests/phpunit/includes/deferred/MWCallableUpdateTest.php @@ -0,0 +1,34 @@ +assertSame( 0, $ran ); + $update->doUpdate(); + $this->assertSame( 1, $ran ); + } + + public function testCancel() { + // Prepare update and DB + $db = new DatabaseTestHelper( __METHOD__ ); + $db->begin( __METHOD__ ); + $ran = 0; + $update = new MWCallableUpdate( function () use ( &$ran ) { + $ran++; + }, __METHOD__, $db ); + + // Emulate rollback + $db->rollback( __METHOD__ ); + + // Ensure it was cancelled + $update->doUpdate(); + $this->assertSame( 0, $ran ); + } +} -- 2.20.1