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
2024-11-23 23:24:39 +00:00 · 2016-12-20 22:54:47 +01:00 · 2016-12-20 22:54:47 +01:00 · c587a9785b
parent 147772bbd8
commit c587a9785b
5 changed files with 369 additions and 105 deletions
--- a/Popups.hooks.php
+++ b/Popups.hooks.php
@ -23,11 +23,9 @@ use Popups\PopupsContext;
 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 @@ class PopupsHooks {
 	 * @param array $prefs
 	 */
 	static function onGetPreferences( User $user, array &$prefs ) {
-		$module = self::getModuleContext();
+		$module = PopupsContext::getInstance();

 		if ( !$module->showPreviewsOptInOnPreferencesPage() ) {
 			return;
@ -70,28 +68,13 @@ class PopupsHooks {
 		];
 	}

-	/**
-	 * @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 @@ class PopupsHooks {
 	/**
 	 * @param array &$testModules
 	 * @param ResourceLoader $resourceLoader
-	 * @return bool
 	 */
 	public static function onResourceLoaderTestModules( array &$testModules,
 		ResourceLoader &$resourceLoader ) {
@ -143,8 +125,7 @@ class PopupsHooks {
 	 * @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 @@ class PopupsHooks {
 			$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;
-	}
 }
--- 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 @@ class PopupsContext {
 	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 @@ class PopupsContext {
 		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 @@ class PopupsContext {
 	public function getConfig() {
 		return $this->config;
 	}
+
 }
--- 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 @@ use Popups\PopupsContext;
 * 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 @@ class PopupsContextTest extends MediaWikiTestCase {
 			return $configFactoryMock;
 		} );

-		$module = new Popups\PopupsContext();
-		$this->assertSame( $module->getConfig(), $configMock );
+		$context = PopupsContext::getInstance();
+		$this->assertSame( $context->getConfig(), $configMock );
 	}

 	/**
@ -51,15 +85,13 @@ class PopupsContextTest extends MediaWikiTestCase {
 					"wgPopupsHideOptInOnPreferencesPage" => false
 				],
 				"expected" => true
-			],
-			[
+			], [
 				"options" => [
 					"wgPopupsBetaFeature" => true,
 					"wgPopupsHideOptInOnPreferencesPage" => false
 				],
 				"expected" => false
-			],
-			[
+			], [
 				"options" => [
 					"wgPopupsBetaFeature" => false,
 					"wgPopupsHideOptInOnPreferencesPage" => true
@ -70,17 +102,20 @@ class PopupsContextTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @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 @@ class PopupsContextTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @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 @@ class PopupsContextTest extends MediaWikiTestCase {
 		$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 @@ class PopupsContextTest extends MediaWikiTestCase {
 			[
 				"optin" => PopupsContext::PREVIEWS_ENABLED,
 				'expected' => true
-			],
-			[
+			], [
 				"optin" => PopupsContext::PREVIEWS_DISABLED,
 				'expected' => false
 			]
@ -148,14 +187,88 @@ class PopupsContextTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @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 );
+	}
+
 }
--- a/tests/phpunit/PopupsContextTestWrapper.php
+++ 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;
+	}
+}
--- 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 @@ class PopupsHooksTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @covers PopupsHooks::onGetBetaPreferences
+	 * @covers ::onGetBetaPreferences
 	 */
 	public function testOnGetBetaPreferencesBetaEnabled() {
 		$prefs = [ 'someNotEmptyValue' => 'notEmpty' ];
@ -37,17 +58,16 @@ class PopupsHooksTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @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 @@ class PopupsHooksTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @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 @@ class PopupsHooksTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @covers PopupsHooks::onResourceLoaderTestModules
+	 * @covers ::onResourceLoaderTestModules
 	 */
 	public function testOnResourceLoaderTestModules() {
 		$testModules = [ 'someNotEmptyValue' => 'notEmpty' ];
@ -90,7 +109,7 @@ class PopupsHooksTest extends MediaWikiTestCase {
 	}

 	/**
-	 * @covers PopupsHooks::onResourceLoaderGetConfigVars
+	 * @covers ::onResourceLoaderGetConfigVars
 	 */
 	public function testOnResourceLoaderGetConfigVars() {
 		$vars = [ 'something' => 'notEmpty' ];
@ -114,4 +133,85 @@ class PopupsHooksTest extends MediaWikiTestCase {
 			$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 );
+	}
 }