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

diff --git a/tests/common/TestsAutoLoader.php b/tests/common/TestsAutoLoader.php
index 9626312..f9b84d7 100644 (file)
--- 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 (file)
index 0000000..a506dcb
--- /dev/null
+++ b/tests/phpunit/MediaWikiPHPUnitCommand.php
@@ -0,0 +1,30 @@
+<?php
+
+class MediaWikiPHPUnitCommand extends PHPUnit_TextUI_Command {
+       private $cliArgs;
+
+       public function __construct( $ignorableOptions, $cliArgs ) {
+               $ignore = function ( $arg ) {
+               };
+               foreach ( $ignorableOptions as $option ) {
+                       $this->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 0a162a2..ef0bc07 100644 (file)
--- a/tests/phpunit/MediaWikiPHPUnitTestListener.php
+++ b/tests/phpunit/MediaWikiPHPUnitTestListener.php
@@ -1,7 +1,6 @@
 <?php
 
-class MediaWikiPHPUnitTestListener
-       extends PHPUnit_TextUI_ResultPrinter implements PHPUnit_Framework_TestListener {
+class MediaWikiPHPUnitTestListener extends PHPUnit_Framework_BaseTestListener {
 
        /**
         * @var string
@@ -33,7 +32,6 @@ class MediaWikiPHPUnitTestListener
         * @param float $time
         */
        public function addError( PHPUnit_Framework_Test $test, Exception $e, $time ) {
-               parent::addError( $test, $e, $time );
                wfDebugLog(
                        $this->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 3917ca6..383d430 100644 (file)
--- 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 (file)
index 0000000..40684e8
--- /dev/null
+++ b/tests/phpunit/MediaWikiTestResult.php
@@ -0,0 +1,17 @@
+<?php
+
+class MediaWikiTestResult extends PHPUnit_Framework_TestResult {
+       private $cliArgs;
+
+       public function __construct( array $cliArgs ) {
+               $this->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 (file)
index 0000000..96f6edd
--- /dev/null
+++ b/tests/phpunit/MediaWikiTestRunner.php
@@ -0,0 +1,13 @@
+<?php
+
+class MediaWikiTestRunner extends PHPUnit_TextUI_TestRunner {
+       private $cliArgs;
+
+       public function setMwCliArgs( array $cliArgs ) {
+               $this->cliArgs = $cliArgs;
+       }
+
+       protected function createTestResult() {
+               return new MediaWikiTestResult( $this->cliArgs );
+       }
+}

diff --git a/tests/phpunit/phpunit.php b/tests/phpunit/phpunit.php
index b1cca4a..d5a19c2 100755 (executable)
--- 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() {