diff options
author | Daimona Eaytoy <daimona.wiki@gmail.com> | 2024-12-11 18:33:13 +0100 |
---|---|---|
committer | Daimona Eaytoy <daimona.wiki@gmail.com> | 2024-12-11 18:56:50 +0100 |
commit | a14b0e646443c94bd21747682612279b39bd213f (patch) | |
tree | 6154090c828213784a342fc518ad187109256533 | |
parent | 7739cbaac10a132d84a26bfeb1823739690d0c79 (diff) | |
download | mediawikicore-a14b0e646443c94bd21747682612279b39bd213f.tar.gz mediawikicore-a14b0e646443c94bd21747682612279b39bd213f.zip |
db: Drop AbstractSchemaValidator::$missingDepCallback
For starters, the property is really only used by checkDependencies(),
which in turn is only called by AbstractSchemaValidationTest. Everywhere
else, all this code was a no-op.
But for PHPUnit tests, we must not tolerate missing composer
dependencies. In fact, our PHPUnit bootstrap even runs the
CheckComposerLockUpToDate script and exits immediately if dependencies
are not up-to-date, let alone missing.
Instead, move the dependency checks to the constructor, so that they're
always executed, and throw an exception when they fail, instead of
relying on the callback. In practice:
- For PHPUnit tests there's no real difference thanks to the
composer.lock check mentioned above.
- For maintenance scripts, when dependencies are not available, we now
fail with a useful exception, instead of a generic "Class X does not
exist" message.
Bug: T381981
Change-Id: I893dc6a52262f8a63fce3ec5eecdd91dd9f339a5
4 files changed, 6 insertions, 32 deletions
diff --git a/includes/db/AbstractSchemaValidator.php b/includes/db/AbstractSchemaValidator.php index c95171a8f484..92fc5a22c345 100644 --- a/includes/db/AbstractSchemaValidator.php +++ b/includes/db/AbstractSchemaValidator.php @@ -42,38 +42,18 @@ use function is_object; * @since 1.38 */ class AbstractSchemaValidator { - /** - * @var callable(string):void - */ - private $missingDepCallback; - - /** - * @param callable(string):void $missingDepCallback - */ - public function __construct( callable $missingDepCallback ) { - $this->missingDepCallback = $missingDepCallback; - } - - /** - * @codeCoverageIgnore - * @return bool - */ - public function checkDependencies(): bool { + public function __construct() { if ( !class_exists( Validator::class ) ) { - ( $this->missingDepCallback )( + throw new AbstractSchemaValidationError( 'The JsonSchema library cannot be found, please install it through composer.' ); - return false; } if ( !class_exists( JsonParser::class ) ) { - ( $this->missingDepCallback )( + throw new AbstractSchemaValidationError( 'The JSON lint library cannot be found, please install it through composer.' ); - return false; } - - return true; } /** diff --git a/maintenance/includes/SchemaMaintenance.php b/maintenance/includes/SchemaMaintenance.php index 2ec37664b4ca..6e459235ead4 100644 --- a/maintenance/includes/SchemaMaintenance.php +++ b/maintenance/includes/SchemaMaintenance.php @@ -297,9 +297,7 @@ abstract class SchemaMaintenance extends Maintenance { ); } - $validator = new AbstractSchemaValidator( function ( string $msg ): void { - $this->fatalError( $msg ); - } ); + $validator = new AbstractSchemaValidator(); try { $validator->validate( $jsonPath ); } catch ( AbstractSchemaValidationError $e ) { diff --git a/tests/phpunit/structure/AbstractSchemaValidationTest.php b/tests/phpunit/structure/AbstractSchemaValidationTest.php index acb5dbba6213..24585e664d89 100644 --- a/tests/phpunit/structure/AbstractSchemaValidationTest.php +++ b/tests/phpunit/structure/AbstractSchemaValidationTest.php @@ -34,8 +34,7 @@ class AbstractSchemaValidationTest extends PHPUnit\Framework\TestCase { protected function setUp(): void { parent::setUp(); - $this->validator = new AbstractSchemaValidator( [ $this, 'markTestSkipped' ] ); - $this->validator->checkDependencies(); + $this->validator = new AbstractSchemaValidator(); } public static function provideSchemas(): Generator { diff --git a/tests/phpunit/unit/includes/db/AbstractSchemaValidatorTest.php b/tests/phpunit/unit/includes/db/AbstractSchemaValidatorTest.php index d4d3de5e8240..2bf7d56e419d 100644 --- a/tests/phpunit/unit/includes/db/AbstractSchemaValidatorTest.php +++ b/tests/phpunit/unit/includes/db/AbstractSchemaValidatorTest.php @@ -30,10 +30,7 @@ class AbstractSchemaValidatorTest extends MediaWikiUnitTestCase { * @param string|true $expected */ public function testValidate( string $file, $expected ): void { - // If a dependency is missing, skip this test. - $validator = new AbstractSchemaValidator( function ( $msg ) { - $this->markTestSkipped( $msg ); - } ); + $validator = new AbstractSchemaValidator(); if ( is_string( $expected ) ) { $this->expectException( AbstractSchemaValidationError::class ); |