Page MenuHomePhabricator

Use vendor/bin/phpunit instead of tests/phpunit/phpunit.php
Open, LowPublic

Description

Currently, core tests cannot be run by running the phpunit command directly. Instead, we have a custom entrypoint that wraps PHPUnit and calls it programmatically from tests/phpunit/phpunit.php.

We can convert all our logic in the wrapper to a PHPUnit bootstrap file (done already in tests/phpunit/bootstrap.php), so that we (and other automated tools) can start PHPUnit through the regular entry point.

Next steps:

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+144 -165
mediawiki/coremaster+50 -0
mediawiki/coremaster+274 -260
mediawiki/coremaster+2 -2
integration/configmaster+24 -18
mediawiki/coremaster+237 -222
mediawiki/coremaster+222 -237
integration/configmaster+2 -2
mediawiki/extensions/Wikibasemaster+24 -10
mediawiki/extensions/Wikibasemaster+1 -0
integration/quibblemaster+11 -3
mediawiki/extensions/Flowmaster+9 -0
mediawiki/coremaster+25 -165
mediawiki/coremaster+40 -35
mediawiki/coremaster+13 -33
mediawiki/coremaster+23 -55
mediawiki/coremaster+16 -36
mediawiki/coremaster+146 -25
mediawiki/coreREL1_36+2 -1
mediawiki/coreREL1_35+2 -1
mediawiki/coremaster+1 -0
mediawiki/coreREL1_31+2 -1
integration/quibblemaster+41 -3
integration/quibblemaster+19 -3
mediawiki/coremaster+2 -1
mediawiki/vagrantmaster+3 -1
mediawiki/coremaster+44 -242
mediawiki/coremaster+1 -36
mediawiki/coremaster+23 -93
mediawiki/coremaster+60 -401
mediawiki/coremaster+760 -27
mediawiki/coremaster+6 -1
mediawiki/coremaster+86 -0
mediawiki/coremaster+385 -167
Show related patches Customize query in gerrit

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

So, merging config files is not trivial; the main differences I could find:

  • Different bootstrap. I think this is also not trivial on its own, as unit and integration tests have different setups
  • failOnRisky is unset in the unit tests config (probably unintentional, can be added)
  • Unit tests have a memory limit of 512 MB. I think this can be set in the bootstrap file instead
  • Different suites, of course; I think merging all of them in a single file shouldn't cause too many issues
  • The configuration of addUncoveredFilesFromWhitelist is different, I haven't checked how we could merge them
  • The integration tests config uses two custom extensions, MediaWikiLoggerPHPUnitExtension and MediaWikiHooksPHPUnitExtension. The latter is going to be removed anyway (r673705); the former might be added to the unit xml file, but I don't know if it would be helpful.

So perhaps we might start by having different composer commands and keep two separate files until we can improve this.

Change 693561 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/693561

Change 693566 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] [WIP] Delete the custom phpunit.php entry point

https://gerrit.wikimedia.org/r/693566

The point of this task is to remove tests/phpunit/phpunit.php entirely. Can you try with vendor/bin/phpunit?

$ PHPUNIT_WIKI=wiki php /vagrant/mediawiki/vendor/bin/phpunit /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/
PHP Fatal error:  Class 'ApiTestCase' not found in /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/integration/ApiHelpPanelQuestionPosterTest.php on line 15

Also

$ PHPUNIT_WIKI=wiki php /vagrant/mediawiki/tests/phpunit/phpunit.php /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/
Using PHP 7.2.31-1+0~20200514.41+debian9~1.gbpe2a56b+wmf1
PHPUnit 8.5.15 by Sebastian Bergmann and contributors.

[863175c0668e15457765b528] [no req]   Wikimedia\Rdbms\DBConnectionError: Cannot access the database: Unknown database '/vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/' (127.0.0.1) (127.0.0.1)
In T90875#7105008, @Tgr wrote:
$ PHPUNIT_WIKI=wiki php /vagrant/mediawiki/tests/phpunit/phpunit.php /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/
Using PHP 7.2.31-1+0~20200514.41+debian9~1.gbpe2a56b+wmf1
PHPUnit 8.5.15 by Sebastian Bergmann and contributors.

[863175c0668e15457765b528] [no req]   Wikimedia\Rdbms\DBConnectionError: Cannot access the database: Unknown database '/vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/' (127.0.0.1) (127.0.0.1)

This one might be caused by the change to the DB-related options, see r673069.

The immediate cause is that it tries to use the first parameter as the wiki name. Not sure why, I skimmed the code and nothing seemed to do that.

That is because of MWMultiVersion. Vagrant's CommonSettings sets $wgDBname to what was determined by MWMultiVersion, which for PHPUnit tests sets this to either --wiki, or the first argument given to the script ($argv). It should be updated to use the environment variable.

A quick update. I tried to migrate our custom script to a bootstrap file. This is currently very hard because Setup.php, LocalSettings etc. all use global state a lot. They define global variables, alter them, require more files that alter these variables etc. PHPUnit's bootstrap system, however, includes the bootstrap file from function scope (FileLoader::load), so these variables are not available globally during the bootstrap process. Then PHPUnit will build a list of variables that were created within the bootstrap file and import them into the global scope, but this happens too late for us. Some of the issues can be fixed by manually importing some variables into the global scope if we already know that a file is going to use them, but it's not always possible (e.g. Setup.php declares wgAutoloadClasses and then TestsAutoloader alters it); also, this would break very easily if the process of setting any global is changed.

I think it would be better if we could first improve this global mess in the various setup files, which is probably a good idea regardless of PHPUnit.

Change 693614 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/vagrant@master] Allow wiki ID to be set as an env var for maintenance scripts

https://gerrit.wikimedia.org/r/693614

The patch fixes the DB selection issue. Class autoloading for extensions still seems broken.

In T90875#7105008, @Tgr wrote:
$ PHPUNIT_WIKI=wiki php /vagrant/mediawiki/vendor/bin/phpunit /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/
PHP Fatal error:  Class 'ApiTestCase' not found in /vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/integration/ApiHelpPanelQuestionPosterTest.php on line 15

Weird, seems like the test autoloader is not invoked? The bootstrap file (in this case, tests/phpunit/bootstrap.php) loads it though, so I'm not sure why you're seeing this failure. Could you please verify that the bootstrap file is being executed, and whether something is preventing the test autoloader from being executed, too?

(BTW, apologies if I can't be of much help, but I've never used vagrant)

It turned out to be mostly a PEBKAC issue: phpunit.php could be invoked from anywhere but plain phpunit looks for the suite in the current directory, so you need to cd /vagrant/mediawiki first.

Change 693614 merged by jenkins-bot:

[mediawiki/vagrant@master] Allow wiki ID to be set as an env var for maintenance scripts

https://gerrit.wikimedia.org/r/693614

Change 693561 merged by jenkins-bot:

[mediawiki/core@master] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/693561

Change 702821 had a related patch set uploaded (by Jforrester; author: Daimona Eaytoy):

[mediawiki/core@REL1_36] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/702821

Change 702822 had a related patch set uploaded (by Jforrester; author: Daimona Eaytoy):

[mediawiki/core@REL1_35] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/702822

Change 702823 had a related patch set uploaded (by Jforrester; author: Daimona Eaytoy):

[mediawiki/core@REL1_31] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/702823

Change 692372 merged by jenkins-bot:

[integration/quibble@master] phpunit: Use composer phpunit:entrypoint

https://gerrit.wikimedia.org/r/692372

Change 704936 had a related patch set uploaded (by Hashar; author: Hashar):

[integration/quibble@master] Release Quibble 1.0.0

https://gerrit.wikimedia.org/r/704936

Change 704936 merged by jenkins-bot:

[integration/quibble@master] Release Quibble 1.0.0

https://gerrit.wikimedia.org/r/704936

Change 702823 abandoned by Umherirrender:

[mediawiki/core@REL1_31] Add a composer command for the custom PHPUnit entry point

Reason:

This release is EOL

https://gerrit.wikimedia.org/r/702823

kostajh renamed this task from Convert tests/phpunit/phpunit.php entrypoint to plain PHPUnit with bootstrap file to Use vendor/bin/phpunit instead of tests/phpunit/phpunit.php.Dec 8 2021, 2:19 PM
kostajh updated the task description. (Show Details)
kostajh updated the task description. (Show Details)

Change 745264 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] [WIP] phpunit: Remove phpunit.php wrapper script

https://gerrit.wikimedia.org/r/745264

Change 745467 had a related patch set uploaded (by Krinkle; author: Kosta Harlan):

[mediawiki/core@master] phpunit: Initialize CLI options for vendor/bin/phpunit

https://gerrit.wikimedia.org/r/745467

Change 745467 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Initialize CLI options for vendor/bin/phpunit

https://gerrit.wikimedia.org/r/745467

Change 702822 merged by jenkins-bot:

[mediawiki/core@REL1_35] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/702822

Change 702821 merged by jenkins-bot:

[mediawiki/core@REL1_36] Add a composer command for the custom PHPUnit entry point

https://gerrit.wikimedia.org/r/702821

Change 760301 had a related patch set uploaded (by Krinkle; author: Tim Starling):

[mediawiki/core@master] Improve ObjectCache integration tests

https://gerrit.wikimedia.org/r/760301

Change 760301 merged by jenkins-bot:

[mediawiki/core@master] Improve ObjectCache integration tests

https://gerrit.wikimedia.org/r/760301

Change 775834 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Kill MediaWikiPHPUnitCommand

https://gerrit.wikimedia.org/r/775834

Change 775854 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Remove some unneeded stuff from phpunit.php and clean up

https://gerrit.wikimedia.org/r/775854

Change 775834 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Remove custom MediaWikiPHPUnitCommand and use default directly

https://gerrit.wikimedia.org/r/775834

Change 775854 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Remove or inline phpunit.php loadSettings() and fatalError()

https://gerrit.wikimedia.org/r/775854

Change 785367 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Remove some unnecessary stuff from phpunit.php

https://gerrit.wikimedia.org/r/785367

Change 785367 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Remove some unnecessary code from phpunit.php

https://gerrit.wikimedia.org/r/785367

Change 785383 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Move wfRequireOnceInGlobalScope to TestSetup

https://gerrit.wikimedia.org/r/785383

Change 785383 merged by jenkins-bot:

[mediawiki/core@master] Move wfRequireOnceInGlobalScope to TestSetup

https://gerrit.wikimedia.org/r/785383

Change 741970 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] [WIP] phpunit: Use vendor/bin/phpunit and memove suite.xml

https://gerrit.wikimedia.org/r/741970

Change 745264 abandoned by Kosta Harlan:

[mediawiki/core@master] phpunit: Remove phpunit.php wrapper script

Reason:

squashed

https://gerrit.wikimedia.org/r/745264

Change 802153 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/Wikibase@master] Wikibase.example.php: Use global keyword

https://gerrit.wikimedia.org/r/802153

Change 802154 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/quibble@master] composer: Set default memory limit to unlimited

https://gerrit.wikimedia.org/r/802154

Change 802154 abandoned by Kosta Harlan:

[integration/quibble@master] composer: Set default memory limit to unlimited

Reason:

https://gerrit.wikimedia.org/r/802154

Change 802184 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/Flow@master] ConfirmEditTest: Override Parser service

https://gerrit.wikimedia.org/r/802184

Change 802201 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/Wikibase@master] SetClaimTest: Mock API request to siteinfo

https://gerrit.wikimedia.org/r/802201

Change 802184 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] ConfirmEditTest: Override Parser service

https://gerrit.wikimedia.org/r/802184

Change 802153 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Wikibase.example.php: Use global keyword

https://gerrit.wikimedia.org/r/802153

Change 803487 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] Remove hardcoded references to suite.xml and phpunit.php

https://gerrit.wikimedia.org/r/803487

Change 803525 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] jjb: Use composer phpunit:entrypoint

https://gerrit.wikimedia.org/r/803525

Change 802201 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] SetClaimTest: Mock API request to siteinfo

https://gerrit.wikimedia.org/r/802201

Change 741970 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Default to vendor/bin/phpunit, remove suites.xml

https://gerrit.wikimedia.org/r/741970

For the record, this task is not yet done as we still need to update CI jobs. Some of which are currently failing (e.g. the periodic coverage job).

Still pending:

Change 803487 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[integration/config@master] Remove hardcoded references to suite.xml and phpunit.php

https://gerrit.wikimedia.org/r/803487

One thing I noticed with the new system, in particular the new bootstrap, is that it loads every extension and skin in the respective directories. This is not ideal for me, because I have some duplicated extension folders (e.g. uncommitted WIPs), some extensions are not updated (because I don't use them), and in general, I fear that loading all extensions could slow everything down. I'm not sure how this can be fixed though, because I assume we don't want to load LocalSettings.php...

@Daimona I'm not aware of this intentionally having changed. If you use composer phpunit, that should use the same maintenance (now "integration") bootstrap for PHPUnit as before, including in fact the loading of LocalSettings.php.

It's only the pure-unit tests composer phpunit:unit which runs core-only tests without LocalSettings and (with DB/servicewiring disallowed, naturally), and I believe thee's a composer phpunit:extensions variant as well which allows the same thing in CI for extensions' pure unit tests. Afaik we don't have to use these locally though.

@Daimona I'm not aware of this intentionally having changed. If you use composer phpunit, that should use the same maintenance (now "integration") bootstrap for PHPUnit as before, including in fact the loading of LocalSettings.php.
It's only the pure-unit tests composer phpunit:unit which runs core-only tests without LocalSettings and (with DB/servicewiring disallowed, naturally), and I believe thee's a composer phpunit:extensions variant as well which allows the same thing in CI for extensions' pure unit tests. Afaik we don't have to use these locally though.

I run tests from PHPStorm (using the phpunit executable), and faced this problem when running a unit test. I was already using the same setup before and never run into this issue.

Ack. Okay. Can you confirm if you have an old phpunit.xml file in the root? The one we have there for the binary to pick up and is checked into Git contains <phpunit bootstrap="tests/phpunit/bootstrap.integration.php">, which should behave exactly as before for the IDE use case. In fact, it should behave better than before, since previously the top-level phpunit.xml file (which we've only had since a short while) was configured for pure unit tests. Before last year, we didn't have a top-level file, which meant phpunit binary didn't work at all out of the box unless config params are set to where our suite.xml file was.

I'm guessing you have something in your IDE, that made it use tests/phpunit/suite.xml, which no longer exists now?

One thing I noticed with the new system, in particular the new bootstrap, is that it loads every extension and skin in the respective directories. This is not ideal for me, because I have some duplicated extension folders (e.g. uncommitted WIPs), some extensions are not updated (because I don't use them), and in general, I fear that loading all extensions could slow everything down. I'm not sure how this can be fixed though, because I assume we don't want to load LocalSettings.php...

I don't think that is related to patches from this task, perhaps it was from the most recent patch on T240535: Clean up ExtensionRegistry autoloading mess, https://gerrit.wikimedia.org/r/c/773514 ?

Ack. Okay. Can you confirm if you have an old phpunit.xml file in the root?

No, I've always used the default one.

The one we have there for the binary to pick up and is checked into Git contains <phpunit bootstrap="tests/phpunit/bootstrap.integration.php">, which should behave exactly as before for the IDE use case.

It does, however we have:

bootstrap.integration.php
require_once __DIR__ . "/bootstrap.php";

and

bootstrap.php
$directoryToJsonMap = [
	$GLOBALS['wgExtensionDirectory'] => 'extension*.json',
	$GLOBALS['wgStyleDirectory'] => 'skin*.json'
];

$extensionProcessor = new ExtensionProcessor();

foreach ( $directoryToJsonMap as $directory => $jsonFilePattern ) {
	foreach ( new GlobIterator( $directory . '/*/' . $jsonFilePattern ) as $iterator ) {
		$jsonPath = $iterator->getPathname();
		$extensionProcessor->extractInfoFromFile( $jsonPath );
	}
}

so it's reading extension.json for every extension in the extensions/ directory, which is what causes the issues I was seeing: for instance, if I have two copies of the same extension, I will get an error from ExtensionProcessor about that.

I'm guessing you have something in your IDE, that made it use tests/phpunit/suite.xml, which no longer exists now?

I used to use both tests/phpunit/phpunit.php + tests/phpunit/suite.xml and vendor/bin/phpunit + phpunit.xml.dist from PHPStorm, switching the configuration depending on whether I needed to run a unit or integration test, and both would work fine. Now I changed the config to always use vendor/bin/phpunit and phpunit.xml.dist, but I get the errors above.

I don't think that is related to patches from this task, perhaps it was from the most recent patch on T240535: Clean up ExtensionRegistry autoloading mess, https://gerrit.wikimedia.org/r/c/773514 ?

Maybe? One thing I can say for sure is that I was able to run tests until a couple days ago, and yesterday I immediately upgraded MW to try the new system, and it started failing.

Change 804305 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] phpunit: Temporary revert entrypoint to phpunit.php

https://gerrit.wikimedia.org/r/804305

Change 803918 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/core@master] Revert "phpunit: Default to vendor/bin/phpunit, remove suites.xml"

https://gerrit.wikimedia.org/r/803918

Change 803918 merged by jenkins-bot:

[mediawiki/core@master] Revert "phpunit: Default to vendor/bin/phpunit, remove suites.xml"

https://gerrit.wikimedia.org/r/803918

Change 804526 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] [DNM] phpunit: Default to vendor/bin/phpunit, remove suites.xml (take 2)

https://gerrit.wikimedia.org/r/804526

Change 804547 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] [WIP] phpunit: Inherit Integration/Unit group from parent test case

https://gerrit.wikimedia.org/r/804547

Change 804305 abandoned by Krinkle:

[mediawiki/core@master] phpunit: Temporarily revert entrypoint to phpunit.php

Reason:

https://gerrit.wikimedia.org/r/804305

Change 804547 abandoned by Kosta Harlan:

[mediawiki/core@master] [WIP] phpunit: Inherit Integration/Unit group from parent test case

Reason:

https://gerrit.wikimedia.org/r/804547

Change 693566 abandoned by Daimona Eaytoy:

[mediawiki/core@master] [WIP] Delete the custom phpunit.php entry point

Reason:

https://gerrit.wikimedia.org/r/693566