Page MenuHomePhabricator

Call to undefined method MediaWikiIntegrationTestCase::suite() in extensions/JsonConfig/tests/phpunit/JCLuaLibraryTest.php:48
Closed, ResolvedPublic

Description

As part of T389998 I went to run the PHPUnit for JsonConfig with the following extensions loaded:

wfLoadExtension( 'FlaggedRevs' );
wfLoadExtension( 'JsonConfig' );

That results in a failure in the Phpunit parallel step:

INFO:quibble.commands:>>> Start: PHPUnit Prepare Parallel Run (Composer)
> MediaWiki\Composer\PhpUnitSplitter\PhpUnitXmlManager::fetchResultsCache

Unable to generate results cache URL - is LOG_PATH set?

> MediaWiki\Composer\PhpUnitSplitter\PhpUnitXmlManager::listTestsNotice

Running `phpunit --list-tests-xml` to get a list of expected tests ... 

Using PHP 8.3.26
Running with MediaWiki settings because there might be integration tests
PHP Fatal error:  Uncaught Error: Call to undefined method MediaWikiIntegrationTestCase::suite() in /workspace/src/extensions/JsonConfig/tests/phpunit/JCLuaLibraryTest.php:48
Stack trace:
#0 [internal function]: JsonConfig\Tests\JCLuaLibraryTest::suite()
#1 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(486): ReflectionMethod->invoke()
#2 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(537): PHPUnit\Framework\TestSuite->addTestFile()
#3 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(37): PHPUnit\Framework\TestSuite->addTestFiles()
#4 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(46): ExtensionsTestSuite->__construct()
#5 [internal function]: ExtensionsTestSuite::suite()
#6 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(486): ReflectionMethod->invoke()
#7 /workspace/src/vendor/phpunit/phpunit/src/TextUI/TestSuiteMapper.php(84): PHPUnit\Framework\TestSuite->addTestFile()
#8 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(391): PHPUnit\TextUI\TestSuiteMapper->map()
#9 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(114): PHPUnit\TextUI\Command->handleArguments()
#10 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(99): PHPUnit\TextUI\Command->run()
#11 /workspace/src/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#12 /workspace/src/vendor/bin/phpunit(122): include('...')
#13 {main}

Next PHPUnit\TextUI\RuntimeException: Call to undefined method MediaWikiIntegrationTestCase::suite() in /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php:101
Stack trace:
#0 /workspace/src/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#1 /workspace/src/vendor/bin/phpunit(122): include('...')
#2 {main}
  thrown in /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php on line 101

Event Timeline

Found it! The JsonConfig extension has LUA integration tests which require Scribunto. To avoid failing when Scribunto is not loaded, the php class has an early return:

tests/phpunit/JCLuaLibraryTest.php
<?php

namespace JsonConfig\Tests; 

use MediaWiki\Extension\Scribunto\Tests\Engines\LuaCommon\LuaEngineTestBase;

if ( !class_exists( LuaEngineTestBase::class ) ) {
    return;
}

class JCLuaLibraryTest extends LuaEngineTestBase {
...
    public static function suite( $className ) {
        self::doMock();
        try {
            return parent::suite( $className );
        } finally {
            self::doUnmock();
        }
    }

I do not understand how:

  • the JCLuaLibraryTest gets loaded given the class does not exist and the php file containing the class has a return
  • the traces refers to MediaWikiIntegrationTestCase::suite but the class extends non existing LuaEngineTestBase.

Maybe it is an oddity of the test splitter.

I gave it a try a few times and could not reproduce since the splitter could not find GlobalJsonLinksTest and expected .../GlobalJsonLinksTest.php. I suspected

  • something wrong in the autoloader
  • PHPUnit files discovery misbehaving somehow
  • the splitter mishandling the data structure somehow

I have the MediaWiki repositories cloning using the same tree as in Gerrit (ex: ~/projects/mediawiki/core and ~/projects/mediawiki/extensions/JsonConfig) with $wgExtensionDirectory = "../extensions";.

Turns out it uses RecursiveDirectoryIterator which does not follow symbolic links by default and I had JsonConfig symlinked from ../../extensions). I fixed it by cp -r inside core extensions directory, updated $wgExtensionDirectory et voilà! Fixed.

I ran PHPUnit and reproduced the issue.

Once I have managed to reproduce the issue, I went to try to reduce the scope in order to find a quick/easy reproducible. The issue occurs when using solely FlaggedRevs :

wfLoadExtension( 'FlaggedRevs' );
wfLoadExtension( 'JsonConfig' );

The reason is, and excuse my swearing, A CLASS ALIAS FOR A CLASS FROM ANOTHER EXTENSION:

tests/phpunit/integration/FlaggedRevsLibraryTest.php
<?php

use MediaWiki\Extension\Scribunto\Tests\Engines\LuaCommon\LuaEngineTestBase;
use MediaWiki\Title\Title;    

if ( !class_exists( LuaEngineTestBase::class ) ) {
    class_alias( MediaWikiIntegrationTestCase::class, LuaEngineTestBase::class );
}

That was introduced by fb4eaba9f9ab199bd91ec918a4298500188a6582 / https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/774435:

Stub base class so that test doesn't break when Scribunto is missing
    
When the Scribunto extension is absent, a base test class cannot be
loaded, which is fatal to the PHP parser.  Work around by providing a
class alias.

JsonConfig has the same requirement (skipping tests when Scribunto is not loaded) but solves it with an early return:

tests/phpunit/JCLuaLibraryTest.php
<?php

namespace JsonConfig\Tests;

use MediaWiki\Extension\Scribunto\Tests\Engines\LuaCommon\LuaEngineTestBase;

if ( !class_exists( LuaEngineTestBase::class ) ) {
    return;
}

class JCLuaLibraryTest extends LuaEngineTestBase {

And since the class alias fulfills class_exists() expectations, the JCLuaLibraryTest get defined, PHPUnit invokes its suite() and that explodes.

Why do I get FlaggedRevs? Because Kartographer depends on both JsonConfig and FlaggedRevs

I think the easiest is to make FlaggedRevs to do an early return, the same it is done by JsonConfig

🎉 🎂 🕯

ProofreadPage does the same:

if ( !class_exists( LuaEngineTestBase::class ) ) {
	return;
}

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

[mediawiki/extensions/FlaggedRevs@master] Skip test class when LuaEngineTestBase is not available

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

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

[mediawiki/extensions/FlaggedRevs@REL1_45] Skip test class when LuaEngineTestBase is not available

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

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

[mediawiki/extensions/FlaggedRevs@REL1_44] Skip test class when LuaEngineTestBase is not available

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

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

[mediawiki/extensions/FlaggedRevs@REL1_43] Skip test class when LuaEngineTestBase is not available

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

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

[mediawiki/extensions/FlaggedRevs@REL1_39] Skip test class when LuaEngineTestBase is not available

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

This will let us disable recursion on Kartographer (which depends on FlaggedRevs and will no more have Scribunto)

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

[integration/config@master] Zuul: [Kartographer] disable recursion

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

Change #1217777 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Skip test class when LuaEngineTestBase is not available

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

Change #1217781 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_39] Skip test class when LuaEngineTestBase is not available

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

Change #1217779 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_44] Skip test class when LuaEngineTestBase is not available

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

Change #1217778 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_45] Skip test class when LuaEngineTestBase is not available

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

Change #1217780 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_43] Skip test class when LuaEngineTestBase is not available

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

Change #1217784 merged by jenkins-bot:

[integration/config@master] Zuul: [Kartographer] disable recursion

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

hashar claimed this task.