Refactor MediaWikiServices::forceGlobalInstance to return void instead of old instance.
Motivation:
Right now code contains a lot of legacy calls without Dependency Inversion to singleton MediaWikiService instance like:
MediaWikiService::getInstance()->getService() ->doSomething();
Example:
interface IColor { function getName(): string } interfact IColorer { function getColor(string $color): ?IColor } class ItemHandler { function coloredTo(string $color): void { $colorer = MediaWikiService::getInstance()->getColorer(); $this->color = $colorer->getColor($color) } function getColor(): IColor { } }
To cover ItemHandler::coloredTo by pure unit test we have to mock MediaWikiService somehow.
class ItemHandlerTest ... { function testColoredTo() { $item = new SomeClass; $this->assertNull($item->getColor()) $item->coloredTo('red') $this->assertSame( 'Red' $item->getColor()->getName() ) } }
We can't do that now, because MediaWikiServices::forceGlobalInstance has
$old = self::getInstance();
Line which on first call initialises filly-working service with valid config.
Note. This line violated the Single Responsibility Principle and causes side-effects.
To fix this problem we
- can re-engineer
MediaWikiServices::forceGlobalInstance($): MediaWikiServices
to
MediaWikiServices::forceGlobalInstance($): void
remove the line:
$old = self::getInstance();
And change return void;
- reengineer each call to it.
from
$oldInstance = MediaWikiServices::forceGlobalInstance($newInstance);
to
$oldInstance = MediaWikiServices::getInstance(); MediaWikiServices::forceGlobalInstance($newInstance);
Below is the draft of implementation:
class MockMediaWikiServicesRegistration { public static function register( TestCase $testInstance ) { $mock = new MockBuilder( $testInstance, MediaWikiServices::class ); $service = $mock ->disableOriginalConstructor() ->getMock(); MediaWikiServices::forceGlobalInstance( $service ); } } class MediaWikiServicesTest extends MediaWikiUnitTestCase { public function setUp() { parent::setUp(); MockMediaWikiServicesRegistration::register($this); } public function tearDown() { parent::tearDown(); MockMediaWikiServicesRegistration::destroy($); } public function testGetInstance() { MediaWikiServices::getInstance(); } }
Then tests get into:
class ItemTest extends MediaWikiUnitTestCase { public function setUp() { parent::setUp(); MockMediaWikiServicesRegistration::register( $this ); } public function tearDown() { parent::tearDown(); MockMediaWikiServices::destroy(); } function testColoredTo() { $colorer = $this->createMock( IColorer::class ) $colorer->method( 'getName' )->willReturn( 'mock color' ); $this->registerColorer($colorer); $item = new SomeClass; $this->assertNull($item->getColor()) $item->coloredTo('red') $this->assertSame( 'mock color' $item->getColor()->getName() ) } private function registerColorer(IColorer $colorer) { /** * @var MediaWikiServices|MockObject $service */ $service = MediaWikiServices::getInstance(); $service ->method( 'getColorer' ) ->willReturn( $colorer ); } }