From 65adc3734846c685e511e22a2846f53e05e9d80c Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Mon, 3 Sep 2018 17:02:44 +1000 Subject: [PATCH] Make phpunit.php less hackish, and install the listener unconditionally * Instead of rewriting $argv, add a Command subclass called MediaWikiPHPUnitCommand which overrides the configuration using the stub functions in Command which were provided for that purpose. * Revert c804a0b5b9be8be9, which added redundant debug output to tests, and instead install the MediaWikiPHPUnitTestListener listener unconditionally. Deprecate and make non-functional the --debug-tests option. If you don't want tests to produce debug output, you can always turn the channel off. * Because I added our listener to the listener array instead of making it override the printer, it's no longer necessary to derive from PHPUnit\TextUI\ResultPrinter. Instead we derive from BaseTestListener. So we don't need to call the (empty) parent methods. * Remove the --with-phpunitclass feature since it doesn't work with this scheme. * Instead of passing CLI args to MediaWikiTestCase via a public static variable, inject it non-statically by overriding the TestRunner and TestResult. * Remove --file, which has been non-functional since my 2016 refactor. Change-Id: Ibcaf9ca81c8dc63cce6dc6f6fb1fffee19f8804e --- tests/common/TestsAutoLoader.php | 21 ++-- tests/phpunit/MediaWikiPHPUnitCommand.php | 30 ++++++ .../phpunit/MediaWikiPHPUnitTestListener.php | 11 +-- tests/phpunit/MediaWikiTestCase.php | 27 ++---- tests/phpunit/MediaWikiTestResult.php | 17 ++++ tests/phpunit/MediaWikiTestRunner.php | 13 +++ tests/phpunit/phpunit.php | 96 ++++++------------- 7 files changed, 108 insertions(+), 107 deletions(-) create mode 100644 tests/phpunit/MediaWikiPHPUnitCommand.php create mode 100644 tests/phpunit/MediaWikiTestResult.php create mode 100644 tests/phpunit/MediaWikiTestRunner.php diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php index 9626312a30..f9b84d7b9b 100644 --- a/tests/common/TestsAutoLoader.php +++ b/tests/common/TestsAutoLoader.php @@ -51,20 +51,23 @@ $wgAutoloadClasses += [ 'TidySupport' => "$testDir/parser/TidySupport.php", # tests/phpunit - 'MediaWikiTestCase' => "$testDir/phpunit/MediaWikiTestCase.php", - 'MediaWikiPHPUnitTestListener' => "$testDir/phpunit/MediaWikiPHPUnitTestListener.php", + 'EmptyResourceLoader' => "$testDir/phpunit/ResourceLoaderTestCase.php", + 'HamcrestPHPUnitIntegration' => "$testDir/phpunit/HamcrestPHPUnitIntegration.php", + 'LessFileCompilationTest' => "$testDir/phpunit/LessFileCompilationTest.php", + 'MediaWikiCoversValidator' => "$testDir/phpunit/MediaWikiCoversValidator.php", 'MediaWikiLangTestCase' => "$testDir/phpunit/MediaWikiLangTestCase.php", + 'MediaWikiPHPUnitCommand' => "$testDir/phpunit/MediaWikiPHPUnitCommand.php", + 'MediaWikiPHPUnitTestListener' => "$testDir/phpunit/MediaWikiPHPUnitTestListener.php", + 'MediaWikiTestCase' => "$testDir/phpunit/MediaWikiTestCase.php", + 'MediaWikiTestResult' => "$testDir/phpunit/MediaWikiTestResult.php", + 'MediaWikiTestRunner' => "$testDir/phpunit/MediaWikiTestRunner.php", + 'PHPUnit4And6Compat' => "$testDir/phpunit/PHPUnit4And6Compat.php", + 'ResourceLoaderFileModuleTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php", + 'ResourceLoaderFileTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php", 'ResourceLoaderTestCase' => "$testDir/phpunit/ResourceLoaderTestCase.php", 'ResourceLoaderTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php", - 'ResourceLoaderFileTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php", - 'ResourceLoaderFileModuleTestModule' => "$testDir/phpunit/ResourceLoaderTestCase.php", - 'EmptyResourceLoader' => "$testDir/phpunit/ResourceLoaderTestCase.php", 'TestUser' => "$testDir/phpunit/includes/TestUser.php", 'TestUserRegistry' => "$testDir/phpunit/includes/TestUserRegistry.php", - 'LessFileCompilationTest' => "$testDir/phpunit/LessFileCompilationTest.php", - 'MediaWikiCoversValidator' => "$testDir/phpunit/MediaWikiCoversValidator.php", - 'PHPUnit4And6Compat' => "$testDir/phpunit/PHPUnit4And6Compat.php", - 'HamcrestPHPUnitIntegration' => "$testDir/phpunit/HamcrestPHPUnitIntegration.php", # tests/phpunit/includes 'PageArchiveTestBase' => "$testDir/phpunit/includes/page/PageArchiveTestBase.php", diff --git a/tests/phpunit/MediaWikiPHPUnitCommand.php b/tests/phpunit/MediaWikiPHPUnitCommand.php new file mode 100644 index 0000000000..a506dcb377 --- /dev/null +++ b/tests/phpunit/MediaWikiPHPUnitCommand.php @@ -0,0 +1,30 @@ +longOptions[$option] = $ignore; + } + $this->cliArgs = $cliArgs; + } + + protected function handleCustomTestSuite() { + // Use our suite.xml + if ( !isset( $this->arguments['configuration'] ) ) { + $this->arguments['configuration'] = __DIR__ . '/suite.xml'; + } + + // Add our own listener + $this->arguments['listeners'][] = new MediaWikiPHPUnitTestListener; + } + + protected function createRunner() { + $runner = new MediaWikiTestRunner; + $runner->setMwCliArgs( $this->cliArgs ); + return $runner; + } +} diff --git a/tests/phpunit/MediaWikiPHPUnitTestListener.php b/tests/phpunit/MediaWikiPHPUnitTestListener.php index 0a162a283d..ef0bc07a1f 100644 --- a/tests/phpunit/MediaWikiPHPUnitTestListener.php +++ b/tests/phpunit/MediaWikiPHPUnitTestListener.php @@ -1,7 +1,6 @@ logChannel, 'ERROR in ' . $this->getTestName( $test ) . ': ' . $this->getErrorName( $e ) @@ -50,7 +48,6 @@ class MediaWikiPHPUnitTestListener public function addFailure( PHPUnit_Framework_Test $test, PHPUnit_Framework_AssertionFailedError $e, $time ) { - parent::addFailure( $test, $e, $time ); wfDebugLog( $this->logChannel, 'FAILURE in ' . $this->getTestName( $test ) . ': ' . $this->getErrorName( $e ) @@ -65,7 +62,6 @@ class MediaWikiPHPUnitTestListener * @param float $time */ public function addIncompleteTest( PHPUnit_Framework_Test $test, Exception $e, $time ) { - parent::addIncompleteTest( $test, $e, $time ); wfDebugLog( $this->logChannel, 'Incomplete test ' . $this->getTestName( $test ) . ': ' . $this->getErrorName( $e ) @@ -80,7 +76,6 @@ class MediaWikiPHPUnitTestListener * @param float $time */ public function addSkippedTest( PHPUnit_Framework_Test $test, Exception $e, $time ) { - parent::addSkippedTest( $test, $e, $time ); wfDebugLog( $this->logChannel, 'Skipped test ' . $this->getTestName( $test ) . ': ' . $this->getErrorName( $e ) @@ -93,7 +88,6 @@ class MediaWikiPHPUnitTestListener * @param PHPUnit_Framework_TestSuite $suite */ public function startTestSuite( PHPUnit_Framework_TestSuite $suite ) { - parent::startTestSuite( $suite ); wfDebugLog( $this->logChannel, 'START suite ' . $suite->getName() ); } @@ -103,7 +97,6 @@ class MediaWikiPHPUnitTestListener * @param PHPUnit_Framework_TestSuite $suite */ public function endTestSuite( PHPUnit_Framework_TestSuite $suite ) { - parent::endTestSuite( $suite ); wfDebugLog( $this->logChannel, 'END suite ' . $suite->getName() ); } @@ -113,7 +106,6 @@ class MediaWikiPHPUnitTestListener * @param PHPUnit_Framework_Test $test */ public function startTest( PHPUnit_Framework_Test $test ) { - parent::startTest( $test ); wfDebugLog( $this->logChannel, 'Start test ' . $this->getTestName( $test ) ); } @@ -124,7 +116,6 @@ class MediaWikiPHPUnitTestListener * @param float $time */ public function endTest( PHPUnit_Framework_Test $test, $time ) { - parent::endTest( $test, $time ); wfDebugLog( $this->logChannel, 'End test ' . $this->getTestName( $test ) ); } } diff --git a/tests/phpunit/MediaWikiTestCase.php b/tests/phpunit/MediaWikiTestCase.php index 3917ca6152..383d430f81 100644 --- a/tests/phpunit/MediaWikiTestCase.php +++ b/tests/phpunit/MediaWikiTestCase.php @@ -107,9 +107,10 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { private $loggers = []; /** - * @var LoggerInterface + * The CLI arguments passed through from phpunit.php + * @var array */ - private $testLogger; + private $cliArgs = []; /** * Table name prefixes. Oracle likes it shorter. @@ -133,11 +134,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { $this->backupGlobals = false; $this->backupStaticAttributes = false; - $this->testLogger = self::getTestLogger(); - } - - private static function getTestLogger() { - return LoggerFactory::getInstance( 'tests-phpunit' ); } public function __destruct() { @@ -386,13 +382,14 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { } public function run( PHPUnit_Framework_TestResult $result = null ) { + if ( $result instanceof MediaWikiTestResult ) { + $this->cliArgs = $result->getMediaWikiCliArgs(); + } $this->overrideMwServices(); $needsResetDB = false; - if ( !self::$dbSetup || $this->needsDB() ) { // set up a DB connection for this test to use - $this->testLogger->info( "Setting up DB for " . $this->toString() ); self::$useTemporaryTables = !$this->getCliArg( 'use-normal-tables' ); self::$reuseDB = $this->getCliArg( 'reuse-db' ); @@ -419,12 +416,9 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { $needsResetDB = true; } - $this->testLogger->info( "Starting test " . $this->toString() ); parent::run( $result ); - $this->testLogger->info( "Finished test " . $this->toString() ); if ( $needsResetDB ) { - $this->testLogger->info( "Resetting DB" ); $this->resetDB( $this->db, $this->tablesUsed ); } @@ -1154,7 +1148,6 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { * @since 1.32 */ protected function addCoreDBData() { - $this->testLogger->info( __METHOD__ ); if ( $this->db->getType() == 'oracle' ) { # Insert 0 user to prevent FK violations # Anonymous user @@ -1835,11 +1828,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { * @return mixed */ public function getCliArg( $offset ) { - if ( isset( PHPUnitMaintClass::$additionalOptions[$offset] ) ) { - return PHPUnitMaintClass::$additionalOptions[$offset]; - } - - return null; + return $this->cliArgs[$offset] ?? null; } /** @@ -1848,7 +1837,7 @@ abstract class MediaWikiTestCase extends PHPUnit\Framework\TestCase { * @param mixed $value */ public function setCliArg( $offset, $value ) { - PHPUnitMaintClass::$additionalOptions[$offset] = $value; + $this->cliArgs[$offset] = $value; } /** diff --git a/tests/phpunit/MediaWikiTestResult.php b/tests/phpunit/MediaWikiTestResult.php new file mode 100644 index 0000000000..40684e8afd --- /dev/null +++ b/tests/phpunit/MediaWikiTestResult.php @@ -0,0 +1,17 @@ +cliArgs = $cliArgs; + } + + /** + * Get the command-line arguments from phpunit.php + * @return array + */ + public function getMediaWikiCliArgs() { + return $this->cliArgs; + } +} diff --git a/tests/phpunit/MediaWikiTestRunner.php b/tests/phpunit/MediaWikiTestRunner.php new file mode 100644 index 0000000000..96f6edddd3 --- /dev/null +++ b/tests/phpunit/MediaWikiTestRunner.php @@ -0,0 +1,13 @@ +cliArgs = $cliArgs; + } + + protected function createTestResult() { + return new MediaWikiTestResult( $this->cliArgs ); + } +} diff --git a/tests/phpunit/phpunit.php b/tests/phpunit/phpunit.php index b1cca4ac2e..d5a19c287d 100755 --- a/tests/phpunit/phpunit.php +++ b/tests/phpunit/phpunit.php @@ -14,35 +14,15 @@ define( 'MW_PHPUNIT_TEST', true ); require_once dirname( dirname( __DIR__ ) ) . "/maintenance/Maintenance.php"; class PHPUnitMaintClass extends Maintenance { - - public static $additionalOptions = [ - 'file' => false, - 'use-filebackend' => false, - 'use-bagostuff' => false, - 'use-jobqueue' => false, - 'use-normal-tables' => false, - 'mwdebug' => false, - 'reuse-db' => false, - 'wiki' => false, - 'profiler' => false, - ]; - public function __construct() { parent::__construct(); $this->setAllowUnregisteredOptions( true ); - $this->addOption( - 'with-phpunitclass', - 'Class name of the PHPUnit entry point to use', - false, - true - ); $this->addOption( 'debug-tests', - 'Log testing activity to the PHPUnitCommand log channel.', + 'Log testing activity to the PHPUnitCommand log channel (deprecated, always on).', false, # not required false # no arg needed ); - $this->addOption( 'file', 'File describing parser tests.', false, true ); $this->addOption( 'use-filebackend', 'Use filebackend', false, true ); $this->addOption( 'use-bagostuff', 'Use bagostuff', false, true ); $this->addOption( 'use-jobqueue', 'Use jobqueue', false, true ); @@ -64,8 +44,6 @@ class PHPUnitMaintClass extends Maintenance { } public function execute() { - global $IP; - // Deregister handler from MWExceptionHandler::installHandle so that PHPUnit's own handler // stays in tact. // Has to in execute() instead of finalSetup(), because finalSetup() runs before @@ -74,62 +52,42 @@ class PHPUnitMaintClass extends Maintenance { $this->forceFormatServerArgv(); - # Make sure we have --configuration or PHPUnit might complain - if ( !in_array( '--configuration', $_SERVER['argv'] ) ) { - // Hack to eliminate the need to use the Makefile (which sucks ATM) - array_splice( $_SERVER['argv'], 1, 0, - [ '--configuration', $IP . '/tests/phpunit/suite.xml' ] ); - } - - $phpUnitClass = PHPUnit_TextUI_Command::class; - - if ( $this->hasOption( 'with-phpunitclass' ) ) { - $phpUnitClass = $this->getOption( 'with-phpunitclass' ); - - # Cleanup $args array so the option and its value do not - # pollute PHPUnit - $key = array_search( '--with-phpunitclass', $_SERVER['argv'] ); - unset( $_SERVER['argv'][$key] ); // the option - unset( $_SERVER['argv'][$key + 1] ); // its value - $_SERVER['argv'] = array_values( $_SERVER['argv'] ); - } - - $key = array_search( '--debug-tests', $_SERVER['argv'] ); - if ( $key !== false && array_search( '--printer', $_SERVER['argv'] ) === false ) { - unset( $_SERVER['argv'][$key] ); - array_splice( $_SERVER['argv'], 1, 0, 'MediaWikiPHPUnitTestListener' ); - array_splice( $_SERVER['argv'], 1, 0, '--printer' ); - } - - foreach ( self::$additionalOptions as $option => $default ) { - $key = array_search( '--' . $option, $_SERVER['argv'] ); - if ( $key !== false ) { - unset( $_SERVER['argv'][$key] ); - if ( $this->mParams[$option]['withArg'] ) { - self::$additionalOptions[$option] = $_SERVER['argv'][$key + 1]; - unset( $_SERVER['argv'][$key + 1] ); - } else { - self::$additionalOptions[$option] = true; - } - } - } - if ( !class_exists( 'PHPUnit\\Framework\\TestCase' ) ) { echo "PHPUnit not found. Please install it and other dev dependencies by running `composer install` in MediaWiki root directory.\n"; exit( 1 ); } - if ( !class_exists( $phpUnitClass ) ) { - echo "PHPUnit entry point '" . $phpUnitClass . "' not found. Please make sure you installed - the containing component and check the spelling of the class name.\n"; - exit( 1 ); - } echo defined( 'HHVM_VERSION' ) ? 'Using HHVM ' . HHVM_VERSION . ' (' . PHP_VERSION . ")\n" : 'Using PHP ' . PHP_VERSION . "\n"; - $phpUnitClass::main(); + // Tell PHPUnit to ignore options meant for MediaWiki + $ignore = []; + foreach ( $this->mParams as $name => $param ) { + if ( empty( $param['withArg'] ) ) { + $ignore[] = $name; + } else { + $ignore[] = "$name="; + } + } + + // Pass through certain options to MediaWikiTestCase + $cliArgs = []; + foreach ( + [ + 'use-filebackend', + 'use-bagostuff', + 'use-jobqueue', + 'use-normal-tables', + 'reuse-db' + ] as $name + ) { + $cliArgs[$name] = $this->getOption( $name ); + } + + $command = new MediaWikiPHPUnitCommand( $ignore, $cliArgs ); + $command->run( $_SERVER['argv'], true ); } public function getDbType() { -- 2.20.1