Merge "Add phpunit:parallel:extensions composer command"

autoload.php

@@ -970,6 +970,7 @@ $wgAutoloadLocalClasses = [
 	'MediaWiki\\CommentFormatter\\StringCommentIterator' => __DIR__ . '/includes/CommentFormatter/StringCommentIterator.php',
 	'MediaWiki\\CommentStore\\CommentStore' => __DIR__ . '/includes/CommentStore/CommentStore.php',
 	'MediaWiki\\CommentStore\\CommentStoreComment' => __DIR__ . '/includes/CommentStore/CommentStoreComment.php',
+	'MediaWiki\\Composer\\ComposerLaunchParallel' => __DIR__ . '/includes/composer/ComposerLaunchParallel.php',
 	'MediaWiki\\Composer\\ComposerPhpunitXmlCoverageEdit' => __DIR__ . '/includes/composer/ComposerPhpunitXmlCoverageEdit.php',
 	'MediaWiki\\Composer\\ComposerVendorHtaccessCreator' => __DIR__ . '/includes/composer/ComposerVendorHtaccessCreator.php',
 	'MediaWiki\\Composer\\LockFileChecker' => __DIR__ . '/includes/composer/LockFileChecker.php',

composer.json

@@ -181,10 +181,18 @@
 		"phpunit:entrypoint": "@phpunit",
 		"phpunit:prepare-parallel:extensions": [
 			"MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::listTestsNotice",
-			"@phpunit --list-tests-xml=tests-list.xml --testsuite=extensions",
-			"MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::splitTestsList"
+			"@phpunit --list-tests-xml=tests-list-extensions.xml --testsuite=extensions",
+			"MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::splitTestsListExtensions"
+		],
+		"phpunit:prepare-parallel:default": [
+			"MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::listTestsNotice",
+			"@phpunit --list-tests-xml=tests-list-default.xml",
+			"MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::splitTestsListDefault"
 		],
 		"phpunit:prepare-parallel:split-file": "MediaWiki\\Composer\\PhpUnitSplitter\\PhpUnitXmlManager::splitTestsCustom",
+		"phpunit:parallel:custom-groups": "MediaWiki\\Composer\\ComposerLaunchParallel::launchTestsCustomGroups",
+		"phpunit:parallel:database": "MediaWiki\\Composer\\ComposerLaunchParallel::launchTestsDatabase",
+		"phpunit:parallel:databaseless": "MediaWiki\\Composer\\ComposerLaunchParallel::launchTestsDatabaseless",
 		"maintenance": "@php maintenance/run.php"
 	},
 	"config": {

includes/Maintenance/ForkController.php

@@ -52,6 +52,9 @@ class ForkController {
 
 	protected static $RESTARTABLE_SIGNALS = [];
 
+	/** @var int[] */
+	protected $exitStatuses = [];
+
 	/**
 	 * Pass this flag to __construct() to cause the class to automatically restart
 	 * workers that exit with non-zero exit status or a signal such as SIGSEGV.
@@ -93,6 +96,7 @@ class ForkController {
 	 * This will return 'child' in the child processes. In the parent process,
 	 * it will run until all the child processes exit or a TERM signal is
 	 * received. It will then return 'done'.
+	 *
 	 * @return string
 	 */
 	public function start() {
@@ -135,6 +139,13 @@ class ForkController {
 						}
 					}
 				}
+
+				if ( pcntl_wifexited( $status ) ) {
+					$exitStatus = pcntl_wexitstatus( $status );
+					echo "Worker exited with status $exitStatus\n";
+					$this->exitStatuses[] = $exitStatus;
+				}
+
 				// Throttle restarts
 				if ( $this->procsToStart ) {
 					usleep( 500_000 );
@@ -162,6 +173,20 @@ class ForkController {
 		return 'done';
 	}
 
+	/**
+	 * Return true if all completed child processes exited with an exit
+	 * status / return code of 0.
+	 *
+	 * @return bool
+	 */
+	public function allSuccessful(): bool {
+		return array_reduce(
+			$this->exitStatuses,
+			static fn ( $acc, $status ) => $acc && ( $status === 0 ),
+			true
+		);
+	}
+
 	/**
 	 * Get the number of the child currently running. Note, this
 	 * is not the pid, but rather which of the total number of children

includes/composer/ComposerLaunchParallel.php

@@ -0,0 +1,170 @@
+<?php
+
+declare( strict_types = 1 );
+
+namespace MediaWiki\Composer;
+
+use Composer\Script\Event;
+use MediaWiki\Composer\PhpUnitSplitter\PhpUnitXml;
+use MediaWiki\Maintenance\ForkController;
+use Shellbox\Shellbox;
+
+$basePath = getenv( 'MW_INSTALL_PATH' ) !== false ? getenv( 'MW_INSTALL_PATH' ) : __DIR__ . '/../..';
+
+require_once $basePath . '/includes/BootstrapHelperFunctions.php';
+require_once $basePath . '/includes/Maintenance/ForkController.php';
+
+/**
+ * @license GPL-2.0-or-later
+ */
+class ComposerLaunchParallel extends ForkController {
+
+	private const ALWAYS_EXCLUDE = [ 'Broken', 'ParserFuzz', 'Stub' ];
+	private array $groups = [];
+	private array $excludeGroups = [];
+
+	public function __construct(
+		array $groups,
+		array $excludeGroups
+	) {
+		$this->groups = $groups;
+		$this->excludeGroups = $excludeGroups;
+		/**
+		 * By default, the splitting process splits the tests into 8 groups. 7 of the groups are composed
+		 * of evenly distributed test classes extracted from the `--list-tests-xml` phpunit function. The
+		 * 8th group contains just the ExtensionsParserTestSuite.
+		 */
+		$splitGroupCount = 7;
+		if ( $this->isDatabaseRun() ) {
+			/**
+			 * In the splitting, we put ExtensionsParserTestSuite in `split_group_7` on its own. We only
+			 * need to run `split_group_7` when we run Database tests, since all Parser tests use the
+			 * database. Running `split_group_7` when no matches tests get executed results in a phpunit
+			 * error code.
+			 */
+			$splitGroupCount = 8;
+		}
+		parent::__construct( $splitGroupCount );
+	}
+
+	private function isDatabaseRun(): bool {
+		return in_array( 'Database', $this->groups ) &&
+			!in_array( 'Database', $this->excludeGroups );
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function start(): string {
+		$status = parent::start();
+		if ( $status === 'child' ) {
+			$this->runTestSuite( $this->getChildNumber() );
+		}
+		return $status;
+	}
+
+	protected function prepareEnvironment() {
+		// Skip parent class method to avoid errors:
+		// this script does not run inside MediaWiki, so there is no environment to prepare
+	}
+
+	private function runTestSuite( int $groupId ) {
+		$command = "composer run --timeout=0 phpunit:entrypoint -- --testsuite split_group_"
+			. $groupId . " --exclude-group " . Shellbox::escape(
+				implode( ",", array_diff( $this->excludeGroups, $this->groups ) )
+			);
+		if ( count( $this->groups ) ) {
+			$command .= " --group " . Shellbox::escape( implode( ",", $this->groups ) );
+		}
+		$groupName = $this->isDatabaseRun() ? "database" : "databaseless";
+		$command .= " --cache-result-file=.phpunit_group_" . $groupId . "_" . $groupName . ".result.cache";
+		$command .= " 2>&1";
+		print( "Running command '" . $command . "' ..." . PHP_EOL );
+		// Not sure about the best way to actually launch the tests here. The 'exec' here just
+		// re-launches composer at the same binary location with a different set of arguments.
+		// I wonder if there is a smarter way to launch a composer task from inside a composer
+		// task and also supply arbitrary arguments to the subtask.
+		$output = [];
+		$resultCode = 0;
+		// TODO: Consider using Shell::command() here instead of exec
+		// phpcs:ignore MediaWiki.Usage.ForbiddenFunctions.exec
+		exec( $command, $output, $resultCode );
+		foreach ( $output as $line ) {
+			print( $line . PHP_EOL );
+		}
+		exit( $resultCode );
+	}
+
+	private static function extractArgs(): array {
+		$options = [];
+		foreach ( [ "group", "exclude-group" ] as $argument ) {
+			$groupIndex = array_search( "--" . $argument, $_SERVER['argv'] );
+			if ( $groupIndex > 0 ) {
+				if ( count( $_SERVER['argv'] ) > $groupIndex + 1 ) {
+					$nextArg = $_SERVER['argv'][$groupIndex + 1];
+					if ( strpos( $nextArg, "--" ) === 0 ) {
+						throw new \InvalidArgumentException(
+							"parameter " . $argument . " takes a variable - none supplied"
+						);
+					}
+					$options[$argument] = $nextArg;
+				} else {
+					throw new \InvalidArgumentException(
+						"parameter " . $argument . " takes a variable - not enough arguments supplied"
+					);
+				}
+			}
+		}
+		return $options;
+	}
+
+	public static function launchTests( Event $event, array $groups, array $excludeGroups ): void {
+		$phpUnitConfig = getcwd() . DIRECTORY_SEPARATOR . 'phpunit.xml';
+		if ( !PhpUnitXml::isPhpUnitXmlPrepared( $phpUnitConfig ) ) {
+			$event->getIO()->error( "phpunit.xml is not present or does not contain split test suites" );
+			$event->getIO()->error( "run `composer phpunit:prepare-parallel:...` to generate the split suites" );
+			exit( 1 );
+		}
+		$event->getIO()->info( "Running 'split_group_X' suites in parallel..." );
+		$launcher = new ComposerLaunchParallel( $groups, $excludeGroups );
+		$launcher->start();
+		if ( $launcher->allSuccessful() ) {
+			$event->getIO()->info( "All split_groups succeeded!" );
+			exit( 0 );
+		} else {
+			$event->getIO()->warning( "Some split_groups failed - returning failure status" );
+			exit( 1 );
+		}
+	}
+
+	public static function launchTestsCustomGroups( Event $event ) {
+		$options = self::extractArgs();
+		if ( array_key_exists( 'exclude-group', $options ) ) {
+			$excludeGroups = explode( ',', $options['exclude-group'] );
+		} else {
+			$excludeGroups = [ 'Broken', 'ParserFuzz', 'Stub', 'Standalone', 'Database' ];
+		}
+		if ( array_key_exists( 'group', $options ) ) {
+			$groups = explode( ',', $options['group'] );
+		} else {
+			$groups = [];
+		}
+		self::launchTests( $event, $groups, $excludeGroups );
+	}
+
+	public static function launchTestsDatabase( Event $event ) {
+		self::launchTests(
+			$event,
+			[ 'Database' ],
+			array_merge( self::ALWAYS_EXCLUDE, [ 'Standalone' ] )
+		);
+	}
+
+	public static function launchTestsDatabaseless( Event $event ) {
+		self::launchTests(
+			$event,
+			[],
+			array_merge( self::ALWAYS_EXCLUDE, [ 'Standalone', 'Database' ] )
+		);
+	}
+}

includes/composer/PhpUnitSplitter/PhpUnitXml.php

@@ -20,6 +20,14 @@ class PhpUnitXml {
 		$this->xml = new SimpleXMLElement( file_get_contents( $phpUnitXmlFile ) );
 	}
 
+	public static function isPhpUnitXmlPrepared( string $targetFile ): bool {
+		if ( !file_exists( $targetFile ) ) {
+			return false;
+		}
+		$unitFile = new PhpUnitXml( $targetFile );
+		return $unitFile->containsSplitGroups();
+	}
+
 	public function containsSplitGroups(): bool {
 		if ( !property_exists( $this->xml, "testsuites" ) ||
 			!property_exists( $this->xml->testsuites, "testsuite" ) ) {

includes/composer/PhpUnitSplitter/PhpUnitXmlManager.php

@@ -11,6 +11,8 @@ class PhpUnitXmlManager {
 
 	private string $rootDir;
 
+	private string $testsListFile;
+
 	/**
 	 * The `SkippedTestCase` is generated dynamically by PHPUnit for tests
 	 * that are marked as skipped. We don't need to find a matching filesystem
@@ -26,8 +28,9 @@ class PhpUnitXmlManager {
 		"\\ParserIntegrationTest",
 	];
 
-	public function __construct( string $rootDir ) {
+	public function __construct( string $rootDir, string $testsListFile ) {
 		$this->rootDir = $rootDir;
+		$this->testsListFile = $testsListFile;
 	}
 
 	private function getPhpUnitXmlTarget(): string {
@@ -39,15 +42,7 @@ class PhpUnitXmlManager {
 	}
 
 	private function getTestsList(): string {
-		return $this->rootDir . DIRECTORY_SEPARATOR . "tests-list.xml";
-	}
-
-	public function isPhpUnitXmlPrepared(): bool {
-		if ( !file_exists( $this->getPhpUnitXmlTarget() ) ) {
-			return false;
-		}
-		$unitFile = $this->loadPhpUnitXml( $this->getPhpUnitXmlTarget() );
-		return $unitFile->containsSplitGroups();
+		return $this->rootDir . DIRECTORY_SEPARATOR . $this->testsListFile;
 	}
 
 	private function loadPhpUnitXmlDist(): PhpUnitXml {
@@ -112,6 +107,10 @@ class PhpUnitXmlManager {
 		return ( new TestSuiteBuilder() )->buildSuites( $testClasses, $groups );
 	}
 
+	public function isPhpUnitXmlPrepared(): bool {
+		return PhpUnitXml::isPhpUnitXmlPrepared( $this->rootDir . DIRECTORY_SEPARATOR . "phpunit.xml" );
+	}
+
 	/**
 	 * @return void
 	 * @throws MissingNamespaceMatchForTestException
@@ -149,16 +148,36 @@ class PhpUnitXmlManager {
 	 * @throws MissingNamespaceMatchForTestException
 	 * @throws SuiteGenerationException
 	 */
-	public static function splitTestsList() {
+	public static function splitTestsList( string $testListFile ) {
 		/**
 		 * We split into 8 groups here, because experimentally that generates 100% CPU load
 		 * on developer machines and results in groups that are similar in size to the
 		 * Parser tests (which we have to run in a group on their own - see T345481)
 		 */
-		( new PhpUnitXmlManager( getcwd() ) )->createPhpUnitXml( 8 );
+		( new PhpUnitXmlManager( getcwd(), $testListFile ) )->createPhpUnitXml( 8 );
 		print( PHP_EOL . 'Created modified `phpunit.xml` with test suite groups' . PHP_EOL );
 	}
 
+	/**
+	 * @throws TestListMissingException
+	 * @throws UnlocatedTestException
+	 * @throws MissingNamespaceMatchForTestException
+	 * @throws SuiteGenerationException
+	 */
+	public static function splitTestsListExtensions() {
+		self::splitTestsList( 'tests-list-extensions.xml' );
+	}
+
+	/**
+	 * @throws TestListMissingException
+	 * @throws UnlocatedTestException
+	 * @throws MissingNamespaceMatchForTestException
+	 * @throws SuiteGenerationException
+	 */
+	public static function splitTestsListDefault() {
+		self::splitTestsList( 'tests-list-default.xml' );
+	}
+
 	/**
 	 * @throws TestListMissingException
 	 * @throws UnlocatedTestException

tests/phpunit/unit/includes/composer/PhpUnitSplitter/PhpUnitXmlManagerTest.php

@@ -21,7 +21,7 @@ class PhpUnitXmlManagerTest extends TestCase {
 		parent::setUp();
 		$this->testDir = implode( DIRECTORY_SEPARATOR, [ sys_get_temp_dir(), uniqid( 'PhpUnitTest' ) ] );
 		mkdir( $this->testDir );
-		$this->manager = new PhpUnitXmlManager( $this->testDir );
+		$this->manager = new PhpUnitXmlManager( $this->testDir, 'tests-list.xml' );
 		$this->setupTestFolder();
 	}