phpunit: Clear job queues in MediaWikiTestCase::setUp()
authorTimo Tijhof <krinklemail@gmail.com>
Mon, 23 Apr 2018 23:46:11 +0000 (00:46 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Mon, 23 Apr 2018 23:46:19 +0000 (00:46 +0100)
This should fix the flaky unit test 'SiteStatsTest::testJobsCountGetCached',
which fails locally as follows, when run in isolation.

> 1) SiteStatsTest::testJobsCountGetCached
> A single job enqueued bumps jobscount stat to 1
> Failed asserting that 2 matches expected 1.
>
> /var/www/mediawiki/tests/phpunit/includes/SiteStatsTest.php:22
> /var/www/mediawiki/tests/phpunit/MediaWikiTestCase.php:421
> /var/www/mediawiki/maintenance/doMaintenance.php:94

Instrumenting JobQueueMemory::doBatchPush reveals the following
jobs to have been queued.

- MediaWikiTestCase->run/->addCoreDBData/::getTestSysop/..
  ../User->addGroup/UserGroupMembership->insert/..
  > UserGroupExpiryJob (2)
- MediaWikiTestCase->run/->addCoreDBData/WikiPage->doEditContent/..
  ../WikiPage->{closure}/WikiPage->doEditUpdates/JobQueueGroup->lazyPush/..
  > CategoryMembershipChangeJob
  > HTMLCacheUpdateJob (2)

Fix this by adding clearing of job queues to doLightweightServiceReset()
in MediaWikiTestCase.

Also:
- Move the call to doLightweightServiceReset() from run() to setUp(),
  where it is easier to understand in context. It still runs at the same
  logical point because PHPUnit calls setUp() right before run().

- Remove redundant reset for WANObjectCache->clearProcessCache that
  was both in setUp() and in doLightweightServiceReset().

- Simplify SiteStatsTest by removing the hardcoded delete() calls.
  An alternative fix for the flaky unit test would've been to add
  a delete() call to categoryMembershipChange, but rather than hardcoding
  all possible jobs that TestCase or another test could make, it's easier
  to just reset/delete them all between tests.

- Simplify SiteStatsTest by using the $cache reference directly instead
  of roundtripping through MediaWikiServices. If for some reason
  setService() didn't work, the test will fail either way because it must
  match the one used by JobQueueGroup (TODO: Use injection!), and besides
  the setService() method already has its own unit test.

Change-Id: Ia4b7871221c76c65eacf31915b515705a36940d5

tests/phpunit/MediaWikiTestCase.php
tests/phpunit/includes/SiteStatsTest.php

index 87ca918..a27e9f9 100644 (file)
@@ -360,15 +360,20 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * @see resetGlobalServices()
         */
        private function doLightweightServiceReset() {
-               global $wgRequest;
+               global $wgRequest, $wgJobClasses;
 
+               foreach ( $wgJobClasses as $type => $class ) {
+                       JobQueueGroup::singleton()->get( $type )->delete();
+               }
                JobQueueGroup::destroySingletons();
+
                ObjectCache::clear();
                $services = MediaWikiServices::getInstance();
                $services->resetServiceForTesting( 'MainObjectStash' );
                $services->resetServiceForTesting( 'LocalServerObjectCache' );
                $services->getMainWANObjectCache()->clearProcessCache();
                FileBackendGroup::destroySingleton();
+               DeferredUpdates::clearPendingUpdates();
 
                // TODO: move global state into MediaWikiServices
                RequestContext::resetMain();
@@ -382,9 +387,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
        }
 
        public function run( PHPUnit_Framework_TestResult $result = null ) {
-               // Reset all caches between tests.
-               $this->doLightweightServiceReset();
-
                $needsResetDB = false;
 
                if ( !self::$dbSetup || $this->needsDB() ) {
@@ -515,8 +517,8 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
                        }
                }
 
-               DeferredUpdates::clearPendingUpdates();
-               ObjectCache::getMainWANInstance()->clearProcessCache();
+               // Reset all caches between tests.
+               $this->doLightweightServiceReset();
 
                // XXX: reset maintenance triggers
                // Hook into period lag checks which often happen in long-running scripts
@@ -1147,6 +1149,8 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase {
         * setupTestDB() was called. Useful if we need to perform database operations
         * after the test run has finished (such as saving logs or profiling info).
         *
+        * This is called by phpunit/bootstrap.php after the last test.
+        *
         * @since 1.21
         */
        public static function teardownTestDB() {
index 56bde5d..a6a92c6 100644 (file)
@@ -6,17 +6,10 @@ class SiteStatsTest extends MediaWikiTestCase {
         * @covers SiteStats::jobs
         */
        function testJobsCountGetCached() {
-               $this->setService( 'MainWANObjectCache',
-                       new WANObjectCache( [ 'cache' => new HashBagOStuff() ] ) );
-               $cache = \MediaWiki\MediaWikiServices::getInstance()->getMainWANObjectCache();
+               $cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ] );
+               $this->setService( 'MainWANObjectCache', $cache );
                $jobq = JobQueueGroup::singleton();
 
-               // Delete jobs that might have been left behind by other tests
-               $jobq->get( 'htmlCacheUpdate' )->delete();
-               $jobq->get( 'recentChangesUpdate' )->delete();
-               $jobq->get( 'userGroupExpiry' )->delete();
-               $cache->delete( $cache->makeKey( 'SiteStats', 'jobscount' ) );
-
                $jobq->push( new NullJob( Title::newMainPage(), [] ) );
                $this->assertEquals( 1, SiteStats::jobs(),
                         'A single job enqueued bumps jobscount stat to 1' );