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

[mediawiki/core@master] Avoid mentioning tests/phpunit.php in documentation

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

daimona opened https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-evaluator/-/merge_requests/24

Update references to tests/phpunit/phpunit.php

daimona opened https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-orchestrator/-/merge_requests/37

Update references to tests/phpunit/phpunit.php

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

[mediawiki/extensions/DiscussionTools@master] Update reference to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/Math@master] Update reference to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/MobileFrontend@master] Update reference to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/TitleBlacklist@master] Remove redundant documentation from test

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

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

[mediawiki/extensions/WikiLambda@master] Update reference to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/Wikibase@master] Update PHPUnit command from old script to composer

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

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

[mediawiki/extensions/WikibaseMediaInfo@master] Update reference to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/WikibaseQualityConstraints@master] Update references to tests/phpunit/phpunit.php

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

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

[mediawiki/core@master] Deprecate the tests/phpunit/phpunit.php entrypoint

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

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

[mediawiki/core@master] Fix requireOnceInGlobalScope for PHP < 8.1

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

Change 934706 merged by jenkins-bot:

[mediawiki/extensions/WikibaseQualityConstraints@master] Update references to tests/phpunit/phpunit.php

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

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

[mediawiki/extensions/Wikibase@master] Add explicit `global` in WikibaseClient.ci.php

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

Change 935034 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Add explicit `global` in WikibaseClient.ci.php

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

Change 934615 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Update reference to tests/phpunit/phpunit.php

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

Change 934702 merged by jenkins-bot:

[mediawiki/extensions/TitleBlacklist@master] Remove redundant documentation from test

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

jforrester merged https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-orchestrator/-/merge_requests/37

Update references to tests/phpunit/phpunit.php

jforrester merged https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-evaluator/-/merge_requests/24

Update references to tests/phpunit/phpunit.php

Change 934703 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Update reference to tests/phpunit/phpunit.php

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

Change 934700 merged by jenkins-bot:

[mediawiki/extensions/Math@master] Update reference to tests/phpunit/phpunit.php

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

Change 934701 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Update reference to tests/phpunit/phpunit.php

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

Change 934705 merged by jenkins-bot:

[mediawiki/extensions/WikibaseMediaInfo@master] Update reference to tests/phpunit/phpunit.php

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

Change 934717 merged by jenkins-bot:

[mediawiki/core@master] Fix requireOnceInGlobalScope for PHP < 8.1

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

I've drafted an email for the wikitech-l announcement: https://etherpad.wikimedia.org/p/xwyjuyWchr6kC_31OjsN Please feel free to edit.

@Daimona It looks like the old script is still advertised at https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Running_the_tests.

My first attempt was as follows:

I have no name!@20c0dc97a8fe:/var/www/html/w$ cd tests/phpunit/
I have no name!@20c0dc97a8fe:/var/www/html/w/tests/phpunit$ composer phpunit:entrypoint includes/ResourceLoader/
No composer.json in current directory, do you want to use the one at /var/www/html/w? [Y,n]?

It seems unlike npm, composer does not search for a composer.json in a parent directory. This means, I think, that it must (only) be called from the root, and then to enjoy beginner-friendly tab completion (i.e. not having to remember and type the full path exactly right), one must include the tests/phpunit/ prefix when passing arguments, which seems to work indeed

nobody@20c0dc97a8fe:/var/www/html/w$ composer phpunit:entrypoint tests/phpunit/includes/ResourceLoader/
> phpunit '-c' 'tests/phpunit/suite.xml' 'tests/phpunit/includes/ResourceLoader/'
Using PHP 8.1.13
PHPUnit 9.5.28 by Sebastian Bergmann and contributors.

...............................................................  352 / 352 (100%)

Time: 00:12.340, Memory: 62.50 MB

OK (352 tests, 554 assertions)

Is this the recommended way?

One benefit I see right away is that this removes the confusion that some people experience, when it comes to running individual extension tests. It's now a lot more intuitive to find your way to the extension directory without having to be consious of the fact that it is ../../ up from where the test runner resides.

In T90875#9003703, @Krinkle wrote:

@Daimona It looks like the old script is still advertised at https://www.mediawiki.org/wiki/Manual:PHP_unit_testing/Running_the_tests.

Thanks, I'll update it shortly.

My first attempt was as follows:

[...]

It seems unlike npm, composer does not search for a composer.json in a parent directory. This means, I think, that it must (only) be called from the root, and then to enjoy beginner-friendly tab completion (i.e. not having to remember and type the full path exactly right), one must include the tests/phpunit/ prefix when passing arguments, which seems to work indeed

[...]
Is this the recommended way?

Pretty much, yes. Although I think the true eventual goal is to simply use vendor/bin/phpunit as the entry point, without the need to use a composer script or to specify a config file. And once we're in that position, you can also use

$ cd tests/phpunit
$ ../../vendor/bin/phpunit includes/ResourceLoader

if you wish.

Remaining active use of deprecated entrypoint in CI config and MW code:

• https://codesearch.wmcloud.org/devtools/?q=phpunit\.php
• https://codesearch.wmcloud.org/deployed/?q=phpunit\.php

Change 934707 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Deprecate tests/phpunit/phpunit.php script

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

In T90875#9010115, @Krinkle wrote:

Remaining active use of deprecated entrypoint in CI config and MW code:

In MW code, it looks like there are lots of makefiles using this command. We could clearly fix them, but when I last looked at them, I was actually wondering if they're useful (i.e., if people are actually using them). For instance, the CirrusSearch one still has references to PHP5 and HHVM. I personally think we should reduce, not increase, the possible ways of running tests. But that's just my opinion.

As for CI: I guess some places could be updated. Others, especially the ones where we need to pass CLI arguments to the PHP interpreter, should keep using phpunit.php until T227900 is done, IMHO. T227900#5985015 suggests that adding CLI arguments to an explicit composer invocation might not work (that was for using a different interpreter though, so not exactly the same thing). And replacing phpunit.php with vendor/bin/phpunit -c tests/phpunit/suite.xml only means making it harder to phase out suite.xml. Since the deprecation warning causes no harm, I'd rather proceed with phasing out suite.xml, and then just using vendor/bin/phpunit directly when needed.

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

[mediawiki/core@master] phpunit: Special-case $wgWikimediaJenkinsCI in requireOnceInGlobalScope

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

Change 937533 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Special-case $wgWikimediaJenkinsCI in requireOnceInGlobalScope

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

This broke Parsoid CI -- https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/ /937975. The failures only make sense if the tests are being run with a version of Parsoid from the vendor repo instead of from the patch (which is what you would expect).

@Arlolra investigated and identified this as the source and specifically noticed that the change in invocation is breaking this.

1. vendor/phpunit/phpunit/phpunit '-c' 'tests/phpunit/suite.xml' '--testsuite' 'parsertests'
2. php tests/phpunit/phpunit.php '-c' 'tests/phpunit/suite.xml' '--testsuite' 'parsertests'

Locally, I can confirm that 2 succeeds but 1 fails. As far as I can tell, this is because Parsoid's integration tests on a test file in Parsoid's repository is not being run with version of Parsoid in the patch (which you would expect given the test file is from that repo), but is being run with a version of Parsoid from the vendor/ repo. I confirmed this by overwriting my vendor/wikimedia/parsoid (installed via composer install/update) with my copy of Parsoid master which now passes tests.

@ssastry If I understand correctly, Parsoid CI is interacting with a full MediaWiki install but adding itself to the autoloader (how?) before the rest of composer and vendor are defined, so it gets to effectively shadow its own classes, and doing so without informing Composer or otherwise preventing the Parsoid release from being installed.

Indeed, MediaWiki's autoloader makes a point of registering itself before vendor. But, when running from the phpunit entrypoint, this inherently can't happen as vendor needs to be in the PHP autoloader first for PHPUnit to be able to execute and start up.

Having said that, it should be possible to fix this by telling the place where Parsoid registers itself in the autoloader (where?) to use spl_autoload_register(,,prepend=true). Assuming nothing else references Parsoid classes before this point in time, that would preserve the load priority.

I'd also be interested in learning more about Parsoid's interactions with the autoloader. Something like what @Krinkle suggested should work, or we might explore alternatives depending on what exactly the relevant code does.

Arlo and I were chatting about this earlier. At least in our local setup, we do indeed intervene with the autoloader via LocalSettings.php.

// Parsoid/PHP 
$parsoidDir = '/home/subbu/work/wmf/parsoid';
if ( $parsoidDir !== 'vendor/wikimedia/parsoid' ) {
    AutoLoader::registerNamespaces( [
        // Keep this in sync with the "autoload" clause in 
        // $parsoidDir/composer.json
        'Wikimedia\\Parsoid\\Tools' => "$parsoidDir/tools",
        'Wikimedia\\Parsoid\\' => "$parsoidDir/src",
    ] );
}

And, since LocalSettings.php loading changed with this (as per the wikitech-l message), I suppose this is what breaks this in local testing.

This is how auoloader interception happens for round trip testing on scandium. This is loaded via the config file..

All that said, I haven't paid attention to how our CI has been set up. Scott (who is on vacation) is the one who knows this best. So, I don't quite know how / where to intervene for Parsoid CI without looking further. We could investigate further, but, you may have some idea based on this info?

Found it, in Quibble's LocalSettings.php (as used by CI)

https://gerrit.wikimedia.org/g/integration/quibble/ /4d4aa1ff6bfce8282691bb1baebe7cdc1300956d/quibble/mediawiki/local_settings.php.tpl#73

// Hack to support testing Parsoid as an extension, while overriding
// the composer library included with core. (T227352)
$parsoidDir = $IP . '/services/parsoid';
if ( is_dir( $parsoidDir ) ) {
    // Keep this in sync with the "autoload" clause in
    // $PARSOID_INSTALL_DIR/composer.json
    $parsoidNamespace = [ 'Wikimedia\\Parsoid\\' => "$parsoidDir/src" ];
    if ( method_exists( 'AutoLoader', 'registerNamespaces' ) ) {
        AutoLoader::registerNamespaces( $parsoidNamespace );
    } else {
        AutoLoader::$psr4Namespaces  = $parsoidNamespace;
    }
    wfLoadExtension( 'Parsoid', "$parsoidDir/extension.json" );

Locally, this fixes it for me (code in LocalSettings.php)

function wfInterceptParsoidLoading( $className ) {
    $fileName = Autoloader::find( $className );
    if ( $fileName !== null ) {
        require $fileName;
    }
}

$parsoidDir = '/home/subbu/work/wmf/parsoid';
spl_autoload_register( 'wfInterceptParsoidLoading', true, true );
AutoLoader::registerNamespaces( [
    // Keep this in sync with the "autoload" clause in
    // $parsoidDir/composer.json
    'Wikimedia\\Parsoid\\Tools' => "$parsoidDir/tools",
    'Wikimedia\\Parsoid\\' => "$parsoidDir/src",
] );

But, since this is somewhat new territory for me, I am not sure if that is the best approach, but something like that should also do the trick for CI.

Change 938010 had a related patch set uploaded (by Subramanya Sastry; author: Subramanya Sastry):

[integration/quibble@master] Fix Parsoid CI after changes to use phpunit directly

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

Would someone be able to review my patch today? I am keen to have Parsoid CI functional again.

In T90875#9015252, @ssastry wrote:

Would someone be able to review my patch today? I am keen to have Parsoid CI functional again.

I'd also like to have r938012 included in the next quibble release. But both are currently unmergeable due to T341840, and I'm unsure what the best course of action should be...

Can we temporarily disable that test in CentralAuth to unblock Quibble and re-enable once these patches are merged? I can submit that patch if needed. There are number of Parsoid patches for next week's train that are blocked now. It is of course not the end of the world if Parsoid doesn't have any updates on next week's train, but if there is a way we can avoid that, I would like to pursue that.

Change 938010 merged by jenkins-bot:

[integration/quibble@master] Fix Parsoid CI after changes to use phpunit directly

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

Change 938289 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/quibble@master] release: Quibble 1.5.5

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

Change 938289 merged by jenkins-bot:

[integration/quibble@master] release: Quibble 1.5.5

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

Mentioned in SAL (#wikimedia-releng) [2023-07-14T18:45:00Z] <James_F> Tag Quibble 1.5.5 @ 85009d62de # T90875 T227900 T341840

Change 938296 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] Docker [quibble] Upgrade Quibble to 1.5.5

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

Change 938296 merged by jenkins-bot:

[integration/config@master] Docker [quibble] Upgrade Quibble to 1.5.5

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

Mentioned in SAL (#wikimedia-releng) [2023-07-14T18:56:23Z] <James_F> Docker: Building and publishing quibble images # T90875 T227900 T341840

Change 938298 had a related patch set uploaded (by Jforrester; author: Jforrester):

[integration/config@master] jjb: Update quibble jobs to images with Quibble 1.5.5

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

Change 938298 merged by jenkins-bot:

[integration/config@master] jjb: Update quibble jobs to images with Quibble 1.5.5

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

Change 934699 merged by jenkins-bot:

[mediawiki/core@master] Avoid mentioning tests/phpunit.php in documentation

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

Change 934704 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Update PHPUnit command from old script to composer

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

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

[integration/config@master] jjb: use composer phpunit:entrypoint in wikibase-{repo,client}

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

Change 949160 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] phpunit: Remove $wgWikimediaJenkinsCI hack

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

Change 947369 merged by jenkins-bot:

[integration/config@master] jjb: use composer phpunit:entrypoint in wikibase-{repo,client}

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

Change 803487 abandoned by Hashar:

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

Reason:

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

Change 813327 abandoned by Kosta Harlan:

[mediawiki/core@master] phpunit: Default to vendor/bin/phpunit and phpunit.xml.dist

Reason:

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

Change 804526 abandoned by Kosta Harlan:

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

Reason:

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

Change 803525 abandoned by Hashar:

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

Reason:

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

Change 803525 restored by Jforrester:

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

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

Change 949160 merged by jenkins-bot:

[mediawiki/core@master] phpunit: Remove $wgWikimediaJenkinsCI hack

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