From 214750d8d224fc7e79d63536915d0dbba9fe969a Mon Sep 17 00:00:00 2001 From: Kosta Harlan Date: Tue, 25 Jun 2019 22:33:14 -0400 Subject: [PATCH 1/1] Define unit and integration test suites Following discussion in Ibb8175981092d7f41864e641cc3c118af70a5c76, this patch proposes to further reduce the scope of what unit tests may access, by removing the loading of DefaultSettings and GlobalFunctions.php. This also has the implied effect of disabling the storage backend, as well as the global service locator. MediaWikiTestCase is renamed to MediaWikiIntegrationTestCase so it's scope and purpose is more clear. Whether we still need to keep `@group Database` annotation around is debatable, as it's unclear to me what the performance costs are of implying database access for all tests which extend IntegrationTestCase. As far as I can tell, `@group Database` is primarily used in CI to run faster tests before slower ones, and with the new UnitTestCase the annotation seems redundant. To run all testsuites, use `composer phpunit`. Other composer scripts: - `composer phpunit:unit` to run unit tests - `composer phpunit:integration` to run integration tests - `composer phpunit:coverage` to generate code coverage reports from unit tests (requires XDebug). Note that you can pass arguments to composer scripts with `--`, e.g. `composer phpunit:integration --exclude-group Dump`. Other changes: - Rename bootstrap.php to bootstrap.maintenance.php so it's clear it's part of the legacy PHPUnit-as-maintenance-class setup - Create new bootstrap.php which loads the minimal configuration necessary for the tests, and do additional setup in the run() method of the unit/integration test case classes - Move the unit-tests.xml file to phpunit.xml.dist in preparation for this being the default test configuration For a follow-up patch: - Find unit/integration tests for extensions/skins - Migrate other test suites from suite.xml - Support running all tests via vendor/bin/phpunit Bug: T84948 Bug: T89432 Bug: T87781 Change-Id: Ie717b0ecf4fcfd089d46248f14853c80b7ef4a76 --- .gitattributes | 2 +- .gitignore | 1 + .phpcs.xml | 1 + composer.json | 6 +- includes/DefaultSettings.php | 2 +- includes/Setup.php | 4 +- maintenance/findHooks.php | 2 +- phpunit.xml.dist | 62 ++++++++++++++ tests/common/TestsAutoLoader.php | 3 +- ...e.php => MediaWikiIntegrationTestCase.php} | 24 +++++- tests/phpunit/MediaWikiUnitTestCase.php | 8 +- tests/phpunit/bootstrap.maintenance.php | 34 ++++++++ tests/phpunit/bootstrap.php | 85 +++++++++++++------ .../includes/db/DatabaseSqliteTest.php | 2 +- tests/phpunit/suite.xml | 2 +- tests/phpunit/unit-tests.xml | 42 --------- tests/phpunit/unit/initUnitTests.php | 69 --------------- 17 files changed, 201 insertions(+), 148 deletions(-) create mode 100644 phpunit.xml.dist rename tests/phpunit/{MediaWikiTestCase.php => MediaWikiIntegrationTestCase.php} (98%) create mode 100644 tests/phpunit/bootstrap.maintenance.php rename tests/phpunit/{ => integration}/includes/db/DatabaseSqliteTest.php (99%) delete mode 100644 tests/phpunit/unit-tests.xml delete mode 100644 tests/phpunit/unit/initUnitTests.php diff --git a/.gitattributes b/.gitattributes index 81b7a33173..145caeb3b8 100644 --- a/.gitattributes +++ b/.gitattributes @@ -10,4 +10,4 @@ package.json export-ignore README.mediawiki export-ignore Gemfile* export-ignore vendor/pear/net_smtp/README.rst export-ignore - +phpunit.xml.dist export-ignore diff --git a/.gitignore b/.gitignore index 8cacb1ee30..a76270e184 100644 --- a/.gitignore +++ b/.gitignore @@ -52,6 +52,7 @@ npm-debug.log node_modules/ /resources/lib/.foreign /tests/phpunit/phpunit.phar +phpunit.xml /tests/selenium/log .eslintcache diff --git a/.phpcs.xml b/.phpcs.xml index 9ccf5657b7..76234a2b56 100644 --- a/.phpcs.xml +++ b/.phpcs.xml @@ -180,6 +180,7 @@ */tests/phpunit/includes/GlobalFunctions/*\.php */tests/phpunit/maintenance/*\.php + */tests/phpunit/integration/includes/GlobalFunctions/*\.php diff --git a/composer.json b/composer.json index 428c1a8033..df516215e6 100644 --- a/composer.json +++ b/composer.json @@ -116,7 +116,11 @@ "test": [ "composer lint", "composer phpcs" - ] + ], + "phpunit": "vendor/bin/phpunit", + "phpunit:unit": "vendor/bin/phpunit --colors=always --testsuite=unit", + "phpunit:integration": "vendor/bin/phpunit --colors=always --testsuite=integration", + "phpunit:coverage": "php -d zend_extensions=xdebug.so vendor/bin/phpunit --testsuite=unit --exclude-group Dump,Broken,ParserFuzz,Stub" }, "config": { "optimize-autoloader": true, diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 10155f6f45..a32af365d0 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -7425,7 +7425,7 @@ $wgSpecialPages = []; /** * Array mapping class names to filenames, for autoloading. */ -$wgAutoloadClasses = []; +$wgAutoloadClasses = $wgAutoloadClasses ?? []; /** * Switch controlling legacy case-insensitive classloading. diff --git a/includes/Setup.php b/includes/Setup.php index 54e6795414..641f1f9030 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -807,7 +807,9 @@ if ( $wgRequest->getCookie( 'UseDC', '' ) === 'master' ) { // Useful debug output if ( $wgCommandLineMode ) { - wfDebug( "\n\nStart command line script $self\n" ); + if ( isset( $self ) ) { + wfDebug( "\n\nStart command line script $self\n" ); + } } else { $debug = "\n\nStart request {$wgRequest->getMethod()} {$wgRequest->getRequestURL()}\n"; diff --git a/maintenance/findHooks.php b/maintenance/findHooks.php index a902397391..6d5dda1bfc 100644 --- a/maintenance/findHooks.php +++ b/maintenance/findHooks.php @@ -82,7 +82,7 @@ class FindHooks extends Maintenance { "$IP/", ]; $extraFiles = [ - "$IP/tests/phpunit/MediaWikiTestCase.php", + "$IP/tests/phpunit/MediaWikiIntegrationTestCase.php", ]; foreach ( $recurseDirs as $dir ) { diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000000..e160f3b2ac --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,62 @@ + + + + + + + + tests/phpunit/unit + + + tests/phpunit/integration + + + + + Broken + + + + + includes + languages + maintenance + + languages/messages + languages/data/normalize-ar.php + languages/data/normalize-ml.php + + + + + + + + + 50 + + + 50 + + + + + + diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 8b6c6d5da3..e1dde22436 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -59,8 +59,9 @@ $wgAutoloadClasses += [ 'MediaWikiPHPUnitCommand' => "$testDir/phpunit/MediaWikiPHPUnitCommand.php", 'MediaWikiPHPUnitResultPrinter' => "$testDir/phpunit/MediaWikiPHPUnitResultPrinter.php", 'MediaWikiPHPUnitTestListener' => "$testDir/phpunit/MediaWikiPHPUnitTestListener.php", - 'MediaWikiTestCase' => "$testDir/phpunit/MediaWikiTestCase.php", + 'MediaWikiTestCase' => "$testDir/phpunit/MediaWikiIntegrationTestCase.php", 'MediaWikiUnitTestCase' => "$testDir/phpunit/MediaWikiUnitTestCase.php", + 'MediaWikiIntegrationTestCase' => "$testDir/phpunit/MediaWikiIntegrationTestCase.php", 'MediaWikiTestResult' => "$testDir/phpunit/MediaWikiTestResult.php", 'MediaWikiTestRunner' => "$testDir/phpunit/MediaWikiTestRunner.php", 'PHPUnit4And6Compat' => "$testDir/phpunit/PHPUnit4And6Compat.php", diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiIntegrationTestCase.php similarity index 98% rename from tests/phpunit/MediaWikiTestCase.php rename to tests/phpunit/MediaWikiIntegrationTestCase.php index d46a4da01d..999ba47a42 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiIntegrationTestCase.php @@ -13,8 +13,14 @@ use Wikimedia\TestingAccessWrapper; /** * @since 1.18 + * + * Extend this class if you are testing classes which access global variables, methods, services + * or a storage backend. + * + * Consider using MediaWikiUnitTestCase and mocking dependencies if your code uses dependency + * injection and does not access any globals. */ -abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { +abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase { use MediaWikiCoversValidator; use PHPUnit4And6Compat; @@ -160,8 +166,22 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { } } + private static function initializeForStandardPhpunitEntrypointIfNeeded() { + if ( function_exists( 'wfRequireOnceInGlobalScope' ) ) { + $IP = realpath( __DIR__ . '/../..' ); + wfRequireOnceInGlobalScope( "$IP/includes/Defines.php" ); + wfRequireOnceInGlobalScope( "$IP/includes/DefaultSettings.php" ); + wfRequireOnceInGlobalScope( "$IP/includes/GlobalFunctions.php" ); + wfRequireOnceInGlobalScope( "$IP/includes/Setup.php" ); + wfRequireOnceInGlobalScope( "$IP/tests/common/TestsAutoLoader.php" ); + TestSetup::applyInitialConfig(); + } + } + public static function setUpBeforeClass() { parent::setUpBeforeClass(); + \PHPUnit\Framework\Assert::assertFileExists( 'LocalSettings.php' ); + self::initializeForStandardPhpunitEntrypointIfNeeded(); // Get the original service locator if ( !self::$originalServices ) { @@ -2522,3 +2542,5 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { ) ); } } + +class_alias( 'MediaWikiIntegrationTestCase', 'MediaWikiTestCase' ); diff --git a/tests/phpunit/MediaWikiUnitTestCase.php b/tests/phpunit/MediaWikiUnitTestCase.php index 407be208b6..06f0c9cd01 100644 --- a/tests/phpunit/MediaWikiUnitTestCase.php +++ b/tests/phpunit/MediaWikiUnitTestCase.php @@ -1,7 +1,5 @@ $value ) { + $GLOBALS[$varName] = $value; + } } -// The PHPUnit_TextUI_TestRunner class will run each test suite and may call -// exit() with an exit status code. As such, we cannot run code "after the last test" -// by adding statements to PHPUnitMaintClass::execute or MediaWikiPHPUnitCommand::run. -// Instead, we work around it by registering a shutdown callback from the bootstrap -// file, which runs before PHPUnit starts. -// @todo Once we use PHPUnit 8 or higher, use the 'AfterLastTestHook' feature. -// https://phpunit.readthedocs.io/en/8.0/extending-phpunit.html#available-hook-interfaces -register_shutdown_function( function () { - // This will: - // - clear the temporary job queue. - // - allow extensions to delete any temporary tables they created. - // - restore ability to connect to the real database, - // (for logging profiling data). - MediaWikiTestCase::teardownTestDB(); - - // Log profiling data, e.g. in the database or UDP - wfLogProfilingData(); -} ); +define( 'MEDIAWIKI', true ); +define( 'MW_PHPUNIT_TEST', true ); + +// We don't use a settings file here but some code still assumes that one exists +define( 'MW_CONFIG_FILE', 'LocalSettings.php' ); + +$IP = realpath( __DIR__ . '/../../' ); + +// these variables must be defined before setup runs +$GLOBALS['IP'] = $IP; +// Faking for Setup.php +$GLOBALS['wgScopeTest'] = 'MediaWiki Setup.php scope test'; +$GLOBALS['wgCommandLineMode'] = true; +$GLOBALS['wgAutoloadClasses'] = []; + +require_once "$IP/tests/common/TestSetup.php"; + +wfRequireOnceInGlobalScope( "$IP/includes/AutoLoader.php" ); +wfRequireOnceInGlobalScope( "$IP/tests/common/TestsAutoLoader.php" ); diff --git a/tests/phpunit/includes/db/DatabaseSqliteTest.php b/tests/phpunit/integration/includes/db/DatabaseSqliteTest.php similarity index 99% rename from tests/phpunit/includes/db/DatabaseSqliteTest.php rename to tests/phpunit/integration/includes/db/DatabaseSqliteTest.php index 0f5c1f2f7f..6fa911b67e 100644 --- a/tests/phpunit/includes/db/DatabaseSqliteTest.php +++ b/tests/phpunit/integration/includes/db/DatabaseSqliteTest.php @@ -13,7 +13,7 @@ use Wikimedia\TestingAccessWrapper; * @group Database * @group medium */ -class DatabaseSqliteTest extends MediaWikiTestCase { +class DatabaseSqliteTest extends \MediaWikiIntegrationTestCase { /** @var DatabaseSqlite */ protected $db; diff --git a/tests/phpunit/suite.xml b/tests/phpunit/suite.xml index cc6ac31f70..d7d3c6130b 100644 --- a/tests/phpunit/suite.xml +++ b/tests/phpunit/suite.xml @@ -1,5 +1,5 @@ - - - - - unit - - - - - Broken - - - - - ../../includes - ../../languages - ../../maintenance - - ../../languages/messages - ../../languages/data/normalize-ar.php - ../../languages/data/normalize-ml.php - - - - diff --git a/tests/phpunit/unit/initUnitTests.php b/tests/phpunit/unit/initUnitTests.php deleted file mode 100644 index 212187711a..0000000000 --- a/tests/phpunit/unit/initUnitTests.php +++ /dev/null @@ -1,69 +0,0 @@ - $value ) { - $GLOBALS[$varName] = $value; - } -} - -define( 'MEDIAWIKI', true ); -define( 'MW_PHPUNIT_TEST', true ); - -// We don't use a settings file here but some code still assumes that one exists -define( 'MW_CONFIG_FILE', 'LocalSettings.php' ); - -$IP = realpath( __DIR__ . '/../../..' ); - -// these variables must be defined before setup runs -$GLOBALS['IP'] = $IP; -$GLOBALS['wgCommandLineMode'] = true; - -require_once "$IP/tests/common/TestSetup.php"; - -wfRequireOnceInGlobalScope( "$IP/includes/AutoLoader.php" ); -wfRequireOnceInGlobalScope( "$IP/includes/Defines.php" ); -wfRequireOnceInGlobalScope( "$IP/includes/DefaultSettings.php" ); -wfRequireOnceInGlobalScope( "$IP/includes/GlobalFunctions.php" ); - -require_once "$IP/tests/common/TestsAutoLoader.php"; - -TestSetup::applyInitialConfig(); -- 2.20.1