jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/328426 )

Change subject: Hygiene: Improve code quality by reaching 100% coverage
......................................................................


Hygiene: Improve code quality by reaching 100% coverage

Changes:
 - added missing unit tests
 - introduced PopupsContextTestWrapper, removed all unit-tests logic
   from Hooks/Context classes
 - removed getModule() from Popups.hooks.php, it's hooks responsibility
   to keep single context instance
 - removed class_exists calls, use ExtensionRegistry instead
 - pass ExtensionRegistry as a dependency so it's easy to mock
 - reach 100% code coverage
 - introduce PoupsContext::isBetaOn() as it was used in many places

Change-Id: Id014efe72edf24628539c76968e53eb3e06709f4
---
M Popups.hooks.php
M includes/PopupsContext.php
M tests/phpunit/PopupsContextTest.php
A tests/phpunit/PopupsContextTestWrapper.php
M tests/phpunit/PopupsHooksTest.php
5 files changed, 369 insertions(+), 105 deletions(-)

Approvals:
  jenkins-bot: Verified
  Phuedx: Looks good to me, approved



diff --git a/Popups.hooks.php b/Popups.hooks.php
index 042ab04..b537ac9 100644
--- a/Popups.hooks.php
+++ b/Popups.hooks.php
@@ -23,11 +23,9 @@
 class PopupsHooks {
        const PREVIEWS_PREFERENCES_SECTION = 'rendering/reading';
 
-       private static $context;
-
        static function onGetBetaPreferences( User $user, array &$prefs ) {
                global $wgExtensionAssetsPath;
-               if ( self::getModuleContext()->getConfig()->get( 
'PopupsBetaFeature' ) !== true ) {
+               if ( PopupsContext::getInstance()->isBetaFeatureEnabled() !== 
true ) {
                        return;
                }
                $prefs[PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME] = [
@@ -52,7 +50,7 @@
         * @param array $prefs
         */
        static function onGetPreferences( User $user, array &$prefs ) {
-               $module = self::getModuleContext();
+               $module = PopupsContext::getInstance();
 
                if ( !$module->showPreviewsOptInOnPreferencesPage() ) {
                        return;
@@ -70,28 +68,13 @@
                ];
        }
 
-       /**
-        * @return PopupsContext
-        */
-       private static function getModuleContext() {
-
-               if ( !self::$context ) {
-                       self::$context = new \Popups\PopupsContext();
-               }
-               return self::$context;
-       }
-
-       private static function areDependenciesMet() {
-               $registry = ExtensionRegistry::getInstance();
-               return $registry->isLoaded( 'TextExtracts' ) && class_exists( 
'ApiQueryPageImages' );
-       }
-
        public static function onBeforePageDisplay( OutputPage &$out, Skin 
&$skin ) {
-               $module = self::getModuleContext();
+               $module = PopupsContext::getInstance();
 
-               if ( !self::areDependenciesMet() ) {
+               if ( !$module->areDependenciesMet() ) {
                        $logger = $module->getLogger();
-                       $logger->error( 'Popups requires the PageImages and 
TextExtracts extensions.' );
+                       $logger->error( 'Popups requires the PageImages and 
TextExtracts extensions. '
+                               . 'If Beta mode is on it requires also 
BetaFeatures extension' );
                        return true;
                }
 
@@ -104,7 +87,6 @@
        /**
         * @param array &$testModules
         * @param ResourceLoader $resourceLoader
-        * @return bool
         */
        public static function onResourceLoaderTestModules( array &$testModules,
                ResourceLoader &$resourceLoader ) {
@@ -143,8 +125,7 @@
         * @param array $vars
         */
        public static function onResourceLoaderGetConfigVars( array &$vars ) {
-               $module = self::getModuleContext();
-               $conf = $module->getConfig();
+               $conf = PopupsContext::getInstance()->getConfig();
                $vars['wgPopupsSchemaPopupsSamplingRate'] = $conf->get( 
'SchemaPopupsSamplingRate' );
        }
 
@@ -165,31 +146,4 @@
                        $config->get( 'PopupsOptInDefaultState' );
        }
 
-       /**
-        * Inject Mocked context
-        * As there is no service registration this is used for tests only.
-        *
-        * @param PopupsContext $context
-        * @throws MWException
-        */
-       public static function injectContext( PopupsContext $context ) {
-               if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
-                       throw new MWException( 'injectContext() must not be 
used outside unit tests.' );
-               }
-               self::$context = $context;
-       }
-
-       /**
-        * Remove cached context.
-        * As there is no service registration this is used for tests only.
-        *
-        *
-        * @throws MWException
-        */
-       public static function resetContext() {
-               if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
-                       throw new MWException( 'injectContext() must not be 
used outside unit tests.' );
-               }
-               self::$context = null;
-       }
 }
diff --git a/includes/PopupsContext.php b/includes/PopupsContext.php
index b2c407f..873194d 100644
--- a/includes/PopupsContext.php
+++ b/includes/PopupsContext.php
@@ -21,6 +21,8 @@
 namespace Popups;
 
 use MediaWiki\Logger\LoggerFactory;
+use MediaWiki\MediaWikiServices;
+use ExtensionRegistry;
 
 /**
  * Popups Module
@@ -68,20 +70,46 @@
        private $config;
 
        /**
-        * Module constructor.
+        * @var PopupsContext
         */
-       public function __construct() {
-               $this->config = \MediaWiki\MediaWikiServices::getInstance()
-                       ->getConfigFactory()->makeConfig( 
PopupsContext::EXTENSION_NAME );
+       protected static $instance;
+       /**
+        * Module constructor.
+        * @param ExtensionRegistry $extensionRegistry
+        */
+       protected function __construct( ExtensionRegistry $extensionRegistry ) {
+               /** @todo Use MediaWikiServices Service Locator when it's ready 
*/
+               $this->extensionRegistry = $extensionRegistry;
+               $this->config = 
MediaWikiServices::getInstance()->getConfigFactory()
+                       ->makeConfig( PopupsContext::EXTENSION_NAME );
        }
 
+       /**
+        * Get a PopupsContext instance
+        *
+        * @return PopupsContext
+        */
+       public static function getInstance() {
+               if ( !self::$instance ) {
+                       self::$instance = new PopupsContext( 
ExtensionRegistry::getInstance() );
+               }
+               return self::$instance;
+       }
+       /**
+        * Is Beta Feature mode enabled
+        *
+        * @return bool
+        */
+       public function isBetaFeatureEnabled() {
+               return $this->config->get( 'PopupsBetaFeature' ) === true;
+       }
        /**
         * Are Page previews visible on User Preferences Page
         *
         * return @bool
         */
        public function showPreviewsOptInOnPreferencesPage() {
-               return $this->config->get( 'PopupsBetaFeature' ) === false
+               return !$this->isBetaFeatureEnabled()
                        && $this->config->get( 
'PopupsHideOptInOnPreferencesPage' ) === false;
        }
 
@@ -93,17 +121,25 @@
                if ( $user->isAnon() ) {
                        return true;
                }
-               if ( $this->config->get( 'PopupsBetaFeature' ) ) {
-                       if ( !class_exists( 'BetaFeatures' ) ) {
-                               $this->getLogger()->error( 'PopupsMode cannot 
be used as a beta feature unless ' .
-                                       'the BetaFeatures extension is 
present.' );
-                               return false;
-                       }
+               if ( $this->isBetaFeatureEnabled() ) {
                        return \BetaFeatures::isFeatureEnabled( $user, 
self::PREVIEWS_BETA_PREFERENCE_NAME );
                };
                return $user->getOption( self::PREVIEWS_OPTIN_PREFERENCE_NAME ) 
=== self::PREVIEWS_ENABLED;
        }
 
+       /**
+        * @return bool
+        */
+       public function areDependenciesMet() {
+               $areMet = $this->extensionRegistry->isLoaded( 'TextExtracts' )
+                       && $this->extensionRegistry->isLoaded( 'PageImages' );
+
+               if ( $this->isBetaFeatureEnabled() ) {
+                       $areMet = $areMet && 
$this->extensionRegistry->isLoaded( 'BetaFeatures' );
+               }
+
+               return $areMet;
+       }
        /**
         * Get module logger
         *
@@ -121,4 +157,5 @@
        public function getConfig() {
                return $this->config;
        }
+
 }
diff --git a/tests/phpunit/PopupsContextTest.php 
b/tests/phpunit/PopupsContextTest.php
index 82d43ab..4e02b04 100644
--- a/tests/phpunit/PopupsContextTest.php
+++ b/tests/phpunit/PopupsContextTest.php
@@ -1,4 +1,24 @@
 <?php
+/*
+ * This file is part of the MediaWiki extension Popups.
+ *
+ * Popups is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Popups is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Popups.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @file
+ * @ingroup extensions
+ */
+require_once ( 'PopupsContextTestWrapper.php' );
 
 use Popups\PopupsContext;
 
@@ -6,21 +26,35 @@
  * Popups module tests
  *
  * @group Popups
+ * @coversDefaultClass Popups\PopupsContext
  */
 class PopupsContextTest extends MediaWikiTestCase {
        /**
-        * @covers Popups\PopupsContext::showPreviewsOptInOnPreferencesPage
-        * @dataProvider provideConfigForShowPreviewsInOptIn
+        * Anonymous user id
+        * @see MediaWikiTestCase::addCoreDBData()
         */
-       public function testShowPreviewsPreferencesPage( $config, $expected ) {
-               $this->setMwGlobals( $config );
-               $module = new Popups\PopupsContext();
-               $this->assertEquals( $expected, 
$module->showPreviewsOptInOnPreferencesPage() );
+       const ANONYMOUS_USER = 0;
+
+       public function tearDown() {
+               PopupsContextTestWrapper::resetTestInstance();
+               parent::tearDown();
        }
 
        /**
-        * @covers Popups\PopupsContext::__construct
-        * @covers Popups\PopupsContext::getConfig
+        * @covers ::showPreviewsOptInOnPreferencesPage
+        * @dataProvider provideConfigForShowPreviewsInOptIn
+        * @param array $config
+        * @param bool $expected
+        */
+       public function testShowPreviewsPreferencesPage( $config, $expected ) {
+               $this->setMwGlobals( $config );
+               $context = PopupsContext::getInstance();
+               $this->assertEquals( $expected, 
$context->showPreviewsOptInOnPreferencesPage() );
+       }
+
+       /**
+        * @covers ::__construct
+        * @covers ::getConfig
         */
        public function testContextAndConfigInitialization() {
                $configMock = $this->getMock( Config::class );
@@ -36,8 +70,8 @@
                        return $configFactoryMock;
                } );
 
-               $module = new Popups\PopupsContext();
-               $this->assertSame( $module->getConfig(), $configMock );
+               $context = PopupsContext::getInstance();
+               $this->assertSame( $context->getConfig(), $configMock );
        }
 
        /**
@@ -51,15 +85,13 @@
                                        "wgPopupsHideOptInOnPreferencesPage" => 
false
                                ],
                                "expected" => true
-                       ],
-                       [
+                       ], [
                                "options" => [
                                        "wgPopupsBetaFeature" => true,
                                        "wgPopupsHideOptInOnPreferencesPage" => 
false
                                ],
                                "expected" => false
-                       ],
-                       [
+                       ], [
                                "options" => [
                                        "wgPopupsBetaFeature" => false,
                                        "wgPopupsHideOptInOnPreferencesPage" => 
true
@@ -70,17 +102,20 @@
        }
 
        /**
-        * @covers Popups\PopupsContext::isEnabledByUser
+        * @covers ::isEnabledByUser
+        * @covers ::isBetaFeatureEnabled
         * @dataProvider provideTestDataForIsEnabledByUser
+        * @param bool $optIn
+        * @param bool $expected
         */
        public function testIsEnabledByUser( $optIn, $expected ) {
                $this->setMwGlobals( [
                        "wgPopupsBetaFeature" => false
                ] );
-               $module = new PopupsContext();
+               $context = PopupsContext::getInstance();
                $user = $this->getMutableTestUser()->getUser();
                $user->setOption( 
PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME, $optIn );
-               $this->assertEquals( $module->isEnabledByUser( $user ), 
$expected );
+               $this->assertEquals( $context->isEnabledByUser( $user ), 
$expected );
        }
 
        /**
@@ -100,8 +135,11 @@
        }
 
        /**
-        * @covers Popups\PopupsContext::isEnabledByUser
+        * @covers ::isEnabledByUser
+        * @covers ::isBetaFeatureEnabled
         * @dataProvider provideTestDataForIsEnabledByUserWhenBetaEnabled
+        * @param bool $optIn
+        * @param bool $expected
         */
        public function testIsEnabledByUserWhenBetaEnabled( $optIn, $expected ) 
{
                if ( !class_exists( 'BetaFeatures' ) ) {
@@ -110,25 +148,27 @@
                $this->setMwGlobals( [
                        "wgPopupsBetaFeature" => true
                ] );
-               $module = new PopupsContext();
+               $context = PopupsContext::getInstance();
                $user = $this->getMutableTestUser()->getUser();
                $user->setOption( PopupsContext::PREVIEWS_BETA_PREFERENCE_NAME, 
$optIn );
-               $this->assertEquals( $module->isEnabledByUser( $user ), 
$expected );
+               $this->assertEquals( $context->isEnabledByUser( $user ), 
$expected );
        }
 
        /**
         * Check that Page Previews are disabled for anonymous user
+        * @covers ::isEnabledByUser
+        * @covers ::isBetaFeatureEnabled
         */
        public function testAnonUserHasDisabledPagePreviews() {
                $user = $this->getMutableTestUser()->getUser();
-               $user->setId( 0 );
+               $user->setId( self::ANONYMOUS_USER );
                $user->setOption( PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME,
                        PopupsContext::PREVIEWS_DISABLED );
                $this->setMwGlobals( [
                        "wgPopupsBetaFeature" => false
                ] );
 
-               $context = new PopupsContext();
+               $context = PopupsContext::getInstance();
                $this->assertEquals( true, $context->isEnabledByUser( $user ) );
        }
        /**
@@ -139,8 +179,7 @@
                        [
                                "optin" => PopupsContext::PREVIEWS_ENABLED,
                                'expected' => true
-                       ],
-                       [
+                       ], [
                                "optin" => PopupsContext::PREVIEWS_DISABLED,
                                'expected' => false
                        ]
@@ -148,14 +187,88 @@
        }
 
        /**
-        * @covers Popups\PopupsContext::getLogger
+        * @covers ::areDependenciesMet
+        * @dataProvider provideTestDataForTestAreDependenciesMet
+        * @param bool $betaOn
+        * @param bool $textExtracts
+        * @param bool $pageImages
+        * @param bool $betaFeatures
+        * @param bool $expected
+        */
+       public function testAreDependenciesMet( $betaOn, $textExtracts, 
$pageImages,
+               $betaFeatures, $expected ) {
+
+               $this->setMwGlobals( [
+                       "wgPopupsBetaFeature" => $betaOn
+               ] );
+               $returnValues = [ $textExtracts, $pageImages, $betaFeatures ];
+
+               $mock = $this->getMock( ExtensionRegistry::class, [ 'isLoaded' 
] );
+               $mock->expects( $this->any() )
+                       ->method( 'isLoaded' )
+                       ->will( new 
PHPUnit_Framework_MockObject_Stub_ConsecutiveCalls( $returnValues ) );
+               $context = new PopupsContextTestWrapper( $mock );
+               $this->assertEquals( $expected, $context->areDependenciesMet() 
);
+       }
+
+       /**
+        * @return array/
+        */
+       public function provideTestDataForTestAreDependenciesMet() {
+               return [
+                       [ // Beta is off, dependencies are met even 
BetaFeatures ext is not available
+                               "betaOn" => false,
+                               "textExtracts" => true,
+                               "pageImages" => true,
+                               "betaFeatures" => false,
+                               "expected" => true
+                       ], [ // textExtracts dep is missing
+                               "betaOn" => false,
+                               "textExtracts" => false,
+                               "pageImages" => true,
+                               "betaFeatures" => false,
+                               "expected" => false
+                       ], [ // PageImages dep is missing
+                               "betaOn" => false,
+                               "textExtracts" => true,
+                               "pageImages" => false,
+                               "betaFeatures" => false,
+                               "expected" => false
+                       ], [ // Beta is on but BetaFeatures dep is missing
+                               "betaOn" => true,
+                               "textExtracts" => true,
+                               "pageImages" => true,
+                               "betaFeatures" => false,
+                               "expected" => false
+                       ], [ // beta is on and all deps are available
+                               "betaOn" => true,
+                               "textExtracts" => true,
+                               "pageImages" => true,
+                               "betaFeatures" => true,
+                               "expected" => true
+                       ]
+               ];
+       }
+       /**
+        * @covers ::getLogger
         */
        public function testGetLogger() {
                $loggerMock = $this->getMock( \Psr\Log\LoggerInterface::class );
 
                $this->setLogger( PopupsContext::LOGGER_CHANNEL, $loggerMock );
-               $context = new PopupsContext();
+               $context = PopupsContext::getInstance();
                $this->assertSame( $loggerMock, $context->getLogger() );
        }
 
+       /**
+        * @covers ::getInstance
+        */
+       public function testGetInstanceReturnsSameObjectEveryTime() {
+               $first = PopupsContext::getInstance();
+               $second = PopupsContext::getInstance();
+
+               $this->assertSame( $first, $second );
+               $this->assertInstanceOf( PopupsContext::class, $first );
+       }
+
 }
diff --git a/tests/phpunit/PopupsContextTestWrapper.php 
b/tests/phpunit/PopupsContextTestWrapper.php
new file mode 100644
index 0000000..9384529
--- /dev/null
+++ b/tests/phpunit/PopupsContextTestWrapper.php
@@ -0,0 +1,60 @@
+<?php
+/*
+ * This file is part of the MediaWiki extension Popups.
+ *
+ * Popups is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Popups is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Popups.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @file
+ * @ingroup extensions
+ */
+use \Popups\PopupsContext;
+
+/**
+ * Create an initializable Popups context.
+ * It's bit dirty approach but we do not have a 
DependencyInject/ServiceLocator for extension
+ * modules and we don't want to put tests-related logic inside real classes.
+ *
+ * This wrapper extends PopupsContext and allows to Initialize context by 
creating new instance
+ * and allows to override/reset cached static PopupsContext instance.
+ *
+ * Used for testing only
+ * @codeCoverageIgnore
+ */
+class PopupsContextTestWrapper extends PopupsContext {
+
+       /**
+        * Override constructor so we can create new instances for testing
+        * .
+        * @param ExtensionRegistry $extensionRegistry
+        */
+       public function __construct( ExtensionRegistry $extensionRegistry ) {
+               parent::__construct( $extensionRegistry );
+       }
+
+       /**
+        * Allow to reset cached instance
+        */
+       public static function resetTestInstance() {
+               self::$instance = null;
+       }
+
+       /**
+        * Override cached instance
+        *
+        * @param PopupsContext $testInstance
+        */
+       public static function injectTestInstance( PopupsContext $testInstance 
) {
+               self::$instance = $testInstance;
+       }
+}
diff --git a/tests/phpunit/PopupsHooksTest.php 
b/tests/phpunit/PopupsHooksTest.php
index f6108ac..a2dd1e3 100644
--- a/tests/phpunit/PopupsHooksTest.php
+++ b/tests/phpunit/PopupsHooksTest.php
@@ -1,19 +1,40 @@
 <?php
+/*
+ * This file is part of the MediaWiki extension Popups.
+ *
+ * Popups is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Popups is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Popups.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * @file
+ * @ingroup extensions
+ */
+require_once ( 'PopupsContextTestWrapper.php' );
 
 /**
  * Integration tests for Page Preview hooks
  *
  * @group Popups
+ * @coversDefaultClass  PopupsHooks
  */
 class PopupsHooksTest extends MediaWikiTestCase {
 
        protected function tearDown() {
-               PopupsHooks::resetContext();
+               PopupsContextTestWrapper::resetTestInstance();
                parent::tearDown();
        }
 
        /**
-        * @covers PopupsHooks::onGetBetaPreferences
+        * @covers ::onGetBetaPreferences
         */
        public function testOnGetBetaPreferencesBetaDisabled() {
                $prefs = [ 'someNotEmptyValue' => 'notEmpty' ];
@@ -25,7 +46,7 @@
        }
 
        /**
-        * @covers PopupsHooks::onGetBetaPreferences
+        * @covers ::onGetBetaPreferences
         */
        public function testOnGetBetaPreferencesBetaEnabled() {
                $prefs = [ 'someNotEmptyValue' => 'notEmpty' ];
@@ -37,17 +58,16 @@
        }
 
        /**
-        * @covers PopupsHooks::onGetPreferences
-        * @covers PopupsHooks::injectContext
+        * @covers ::onGetPreferences
         */
        public function testOnGetPreferencesPreviewsDisabled() {
-               $contextMock = $this->getMock( \Popups\PopupsContext::class,
-                       [ 'showPreviewsOptInOnPreferencesPage' ] );
+               $contextMock = $this->getMock( PopupsContextTestWrapper::class,
+                       [ 'showPreviewsOptInOnPreferencesPage' ], [ 
ExtensionRegistry::getInstance() ] );
                $contextMock->expects( $this->once() )
                        ->method( 'showPreviewsOptInOnPreferencesPage' )
                        ->will( $this->returnValue( false ) );
 
-               PopupsHooks::injectContext( $contextMock );
+               PopupsContextTestWrapper::injectTestInstance( $contextMock );
                $prefs = [ 'someNotEmptyValue' => 'notEmpty' ];
 
                PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), 
$prefs );
@@ -56,17 +76,16 @@
        }
 
        /**
-        * @covers PopupsHooks::onGetPreferences
-        * @covers PopupsHooks::injectContext
+        * @covers ::onGetPreferences
         */
        public function testOnGetPreferencesPreviewsEnabled() {
-               $contextMock = $this->getMock( \Popups\PopupsContext::class,
-                       [ 'showPreviewsOptInOnPreferencesPage' ] );
+               $contextMock = $this->getMock( PopupsContextTestWrapper::class,
+                       [ 'showPreviewsOptInOnPreferencesPage' ], [ 
ExtensionRegistry::getInstance() ] );
                $contextMock->expects( $this->once() )
                        ->method( 'showPreviewsOptInOnPreferencesPage' )
                        ->will( $this->returnValue( true ) );
 
-               PopupsHooks::injectContext( $contextMock );
+               PopupsContextTestWrapper::injectTestInstance( $contextMock );
                $prefs = [ 'someNotEmptyValue' => 'notEmpty' ];
 
                PopupsHooks::onGetPreferences( $this->getTestUser()->getUser(), 
$prefs );
@@ -76,7 +95,7 @@
        }
 
        /**
-        * @covers PopupsHooks::onResourceLoaderTestModules
+        * @covers ::onResourceLoaderTestModules
         */
        public function testOnResourceLoaderTestModules() {
                $testModules = [ 'someNotEmptyValue' => 'notEmpty' ];
@@ -90,7 +109,7 @@
        }
 
        /**
-        * @covers PopupsHooks::onResourceLoaderGetConfigVars
+        * @covers ::onResourceLoaderGetConfigVars
         */
        public function testOnResourceLoaderGetConfigVars() {
                $vars = [ 'something' => 'notEmpty' ];
@@ -114,4 +133,85 @@
                        $wgDefaultUserOptions[ 
\Popups\PopupsContext::PREVIEWS_OPTIN_PREFERENCE_NAME ] );
        }
 
+       /**
+        * @covers ::onBeforePageDisplay
+        */
+       public function testOnBeforePageDisplayWhenDependenciesAreNotMet() {
+               $skinMock = $this->getMock( Skin::class );
+               $outPageMock = $this->getMock( OutputPage::class, [ 
'addModules' ], [], '', false );
+               $outPageMock->expects( $this->never() )
+                       ->method( 'addModules' );
+               $loggerMock = $this->getMock( \Psr\Log\LoggerInterface::class );
+               $loggerMock->expects( $this->once() )
+                       ->method( 'error' );
+
+               $contextMock = $this->getMock( PopupsContextTestWrapper::class,
+                       [ 'areDependenciesMet', 'getLogger' ], [ 
ExtensionRegistry::getInstance() ] );
+               $contextMock->expects( $this->once() )
+                       ->method( 'areDependenciesMet' )
+                       ->will( $this->returnValue( false ) );
+               $contextMock->expects( $this->once() )
+                       ->method( 'getLogger' )
+                       ->will( $this->returnValue( $loggerMock ) );
+
+               PopupsContextTestWrapper::injectTestInstance( $contextMock );
+               PopupsHooks::onBeforePageDisplay( $outPageMock, $skinMock );
+       }
+
+       /**
+        * @covers ::onBeforePageDisplay
+        */
+       public function testOnBeforePageDisplayWhenPagePreviewsAreDisabled() {
+               $userMock = $this->getTestUser()->getUser();
+               $skinMock = $this->getMock( Skin::class );
+               $skinMock->expects( $this->once() )
+                       ->method( 'getUser' )
+                       ->will( $this->returnValue( $userMock ) );
+
+               $outPageMock = $this->getMock( OutputPage::class, [ 
'addModules' ], [], '', false );
+               $outPageMock->expects( $this->never() )
+                       ->method( 'addModules' );
+
+               $contextMock = $this->getMock( PopupsContextTestWrapper::class,
+                       [ 'areDependenciesMet', 'isEnabledByUser' ], [ 
ExtensionRegistry::getInstance() ] );
+               $contextMock->expects( $this->once() )
+                       ->method( 'areDependenciesMet' )
+                       ->will( $this->returnValue( true ) );
+               $contextMock->expects( $this->once() )
+                       ->method( 'isEnabledByUser' )
+                       ->with( $userMock )
+                       ->will( $this->returnValue( false ) );
+
+               PopupsContextTestWrapper::injectTestInstance( $contextMock );
+               PopupsHooks::onBeforePageDisplay( $outPageMock, $skinMock );
+       }
+
+       /**
+        * @covers ::onBeforePageDisplay
+        */
+       public function testOnBeforePageDisplayWhenPagePreviewsAreEnabled() {
+               $userMock = $this->getTestUser()->getUser();
+               $skinMock = $this->getMock( Skin::class );
+               $skinMock->expects( $this->once() )
+                       ->method( 'getUser' )
+                       ->will( $this->returnValue( $userMock ) );
+
+               $outPageMock = $this->getMock( OutputPage::class, [ 
'addModules' ], [], '', false );
+               $outPageMock->expects( $this->once() )
+                       ->method( 'addModules' )
+                       ->with( [ 'ext.popups' ] );
+
+               $contextMock = $this->getMock( PopupsContextTestWrapper::class,
+                       [ 'areDependenciesMet', 'isEnabledByUser' ], [ 
ExtensionRegistry::getInstance() ] );
+               $contextMock->expects( $this->once() )
+                       ->method( 'areDependenciesMet' )
+                       ->will( $this->returnValue( true ) );
+               $contextMock->expects( $this->once() )
+                       ->method( 'isEnabledByUser' )
+                       ->with( $userMock )
+                       ->will( $this->returnValue( true ) );
+
+               PopupsContextTestWrapper::injectTestInstance( $contextMock );
+               PopupsHooks::onBeforePageDisplay( $outPageMock, $skinMock );
+       }
 }

-- 
To view, visit https://gerrit.wikimedia.org/r/328426
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id014efe72edf24628539c76968e53eb3e06709f4
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Pmiazga <pmia...@wikimedia.org>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Phuedx <samsm...@wikimedia.org>
Gerrit-Reviewer: Pmiazga <pmia...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to