wiki.techinc.nl/tests/phpunit/includes/jobqueue/JobTest.php

217 lines
7.3 KiB
PHP
Raw Normal View History

<?php
use MediaWiki\MainConfigNames;
use MediaWiki\Request\WebRequest;
use MediaWiki\Title\Title;
/**
* @author Addshore
objectcache,resourceloader,rdbms,jobqueue: Widen @covers annotations Follows-up I4c7d826c7ec654b, I1287f3979aba1bf1. We lose useful coverage and spend valuable time keeping these accurate through refactors (or worse, forget to do so). The theoretically "bad" accidental coverage is almost never actually bad. Having said that, I'm not removing them wholesale (yet). I've audited each of these specific files to confirm it is a general test of the specified subject class, and also kept it limited to those specified classes. That's imho more than 100% of the benefit for less than 1% of the cost (more because `@covers` is more valuable than the fragile and corrosive individual private method tracking in tests that inevitably get out of date with no local incentive to keep them up to date). Cases like structure tests keep `@coversNothing` etc and we still don't count coverage of other classes. There may be a handful of large legacy classes where some methods are effectively class-like in complexity and that's why it's good for PHPUnit to offer the precision instrument but that doesn't meant we have to use that by-default for everything. I think best practice is to write good narrow unit tests, that reflect how the code should be used in practice. Not to write bad tests and hide part of its coverage within the same class or even namespace. Fortunately, that's generally what we do already it's just that we also kept these annotations still in many cases. This wastes time to keep methods in sync, time to realize (and fix) when other people inevitably didn't keep them in sync, time to find uncovered code only to realize it is already covered, time for a less experienced engineer to feel obligate to and do write a low quality test to cover the "missing" branch in an unrealistic way, time wasted in on-boarding by using such "bad" tests as example for how to use the code and then having to unlearn it months/years later, loss of telemetry in knowing what code actually isn't propertly tested due to being masked by a bad test, and lost oppertunities to find actually ununused/unreachable code and to think about how to instead structure the code such that maybe that code can be removed. ------ Especially cases like LBFactoryTest.php were getting out of hand, and in GlobalIdGeneratorTest.php we even resorted to reminding people with inline comments to keep tags in sync. Change-Id: I69b5385868cc6b451e5f2ebec9539694968bf58c
2023-04-10 21:25:34 +00:00
* @covers Job
*/
class JobTest extends MediaWikiIntegrationTestCase {
/**
* @dataProvider provideTestToString
*
* @param Job $job
* @param string $expected
*/
public function testToString( $job, $expected ) {
$this->overrideConfigValue( MainConfigNames::LanguageCode, 'en' );
$this->assertEquals( $expected, $job->toString() );
}
public function provideTestToString() {
$mockToStringObj = $this->getMockBuilder( stdClass::class )
->addMethods( [ '__toString' ] )->getMock();
$mockToStringObj->method( '__toString' )
->willReturn( '{STRING_OBJ_VAL}' );
$requestId = 'requestId=' . WebRequest::getRequestId();
return [
[
$this->getMockJob( [ 'key' => 'val' ] ),
'someCommand Special: key=val ' . $requestId
],
[
$this->getMockJob( [ 'key' => [ 'inkey' => 'inval' ] ] ),
'someCommand Special: key={"inkey":"inval"} ' . $requestId
],
[
$this->getMockJob( [ 'val1' ] ),
'someCommand Special: 0=val1 ' . $requestId
],
[
$this->getMockJob( [ 'val1', 'val2' ] ),
'someCommand Special: 0=val1 1=val2 ' . $requestId
],
[
$this->getMockJob( [ (object)[] ] ),
'someCommand Special: 0=object(stdClass) ' . $requestId
],
[
$this->getMockJob( [ $mockToStringObj ] ),
'someCommand Special: 0={STRING_OBJ_VAL} ' . $requestId
],
[
$this->getMockJob( [
"pages" => [
"932737" => [
0,
"Robert_James_Waller"
]
],
"rootJobSignature" => "45868e99bba89064e4483743ebb9b682ef95c1a7",
"rootJobTimestamp" => "20160309110158",
"masterPos" => [
"file" => "db1023-bin.001288",
"pos" => "308257743",
"asOfTime" => 1457521464.3814
],
"triggeredRecursive" => true
] ),
'someCommand Special: pages={"932737":[0,"Robert_James_Waller"]} ' .
'rootJobSignature=45868e99bba89064e4483743ebb9b682ef95c1a7 ' .
'rootJobTimestamp=20160309110158 masterPos=' .
'{"file":"db1023-bin.001288","pos":"308257743",' .
'"asOfTime":1457521464.3814} triggeredRecursive=1 ' .
$requestId
],
];
}
public function getMockJob( $params ) {
$mock = $this->getMockForAbstractClass(
Job::class,
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
[ 'someCommand', $params ],
'SomeJob'
);
return $mock;
}
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
public function testInvalidParamsArgument() {
$params = false;
$this->expectException( InvalidArgumentException::class );
$this->expectExceptionMessage( '$params must be an array' );
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
$job = $this->getMockJob( $params );
}
/**
* @dataProvider provideTestJobFactory
*/
public function testJobFactory( $handler, $expectedClass ) {
$this->overrideConfigValue( 'JobClasses', [ 'testdummy' => $handler ] );
$job = Job::factory( 'testdummy', Title::newMainPage(), [] );
$this->assertInstanceOf( $expectedClass, $job );
$job2 = Job::factory( 'testdummy', [] );
$this->assertInstanceOf( $expectedClass, $job2 );
$this->assertNotSame( $job, $job2, 'should not reuse instance' );
$job3 = Job::factory( 'testdummy', [ 'namespace' => NS_MAIN, 'title' => 'JobTestTitle' ] );
$this->assertInstanceOf( $expectedClass, $job3 );
$this->assertNotSame( $job, $job3, 'should not reuse instance' );
}
public function provideTestJobFactory() {
return [
'class name, no title' => [ 'NullJob', NullJob::class ],
'class name with title' => [ DeleteLinksJob::class, DeleteLinksJob::class ],
'closure' => [ static function ( Title $title, array $params ) {
return new NullJob( $params );
}, NullJob::class ],
'function' => [ [ $this, 'newNullJob' ], NullJob::class ],
'object spec, no title' => [ [ 'class' => 'NullJob' ], NullJob::class ],
'object spec with title' => [ [ 'class' => DeleteLinksJob::class ], DeleteLinksJob::class ],
'object spec with no title and not subclass of GenericParameterJob' => [
[
'class' => ParsoidCachePrewarmJob::class,
'services' => [
'ParsoidOutputAccess',
'PageStore',
'RevisionLookup'
],
'needsPage' => false
],
ParsoidCachePrewarmJob::class
]
];
}
public function newNullJob( Title $title, array $params ) {
return new NullJob( $params );
}
public function testJobSignatureGeneric() {
$testPage = Title::makeTitle( NS_PROJECT, 'x' );
$blankTitle = Title::makeTitle( NS_SPECIAL, '' );
$params = [ 'z' => 1, 'lives' => 1, 'usleep' => 0 ];
$paramsWithTitle = $params + [ 'namespace' => NS_PROJECT, 'title' => 'x' ];
$job = new NullJob( [ 'namespace' => NS_PROJECT, 'title' => 'x' ] + $params );
$this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $paramsWithTitle );
$job = Job::factory( 'null', $testPage, $params );
$this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $params );
$job = Job::factory( 'null', $paramsWithTitle );
$this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $paramsWithTitle );
$job = Job::factory( 'null', $params );
$this->assertTrue( $blankTitle->equals( $job->getTitle() ) );
$this->assertJobParamsMatch( $job, $params );
}
public function testJobSignatureTitleBased() {
$testPage = Title::makeTitle( NS_PROJECT, 'X' );
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
$blankPage = Title::makeTitle( NS_SPECIAL, 'Blankpage' );
$params = [ 'z' => 1, 'causeAction' => 'unknown', 'causeAgent' => 'unknown' ];
$paramsWithTitle = $params + [ 'namespace' => NS_PROJECT, 'title' => 'X' ];
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
$paramsWithBlankpage = $params + [ 'namespace' => NS_SPECIAL, 'title' => 'Blankpage' ];
$job = new RefreshLinksJob( $testPage, $params );
$this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
$this->assertTrue( $testPage->equals( $job->getTitle() ) );
$this->assertJobParamsMatch( $job, $paramsWithTitle );
$job = Job::factory( 'htmlCacheUpdate', $testPage, $params );
$this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $paramsWithTitle );
$job = Job::factory( 'htmlCacheUpdate', $paramsWithTitle );
$this->assertEquals( $testPage->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $paramsWithTitle );
$job = Job::factory( 'htmlCacheUpdate', $params );
jobqueue: Follow-up for fc5d51f12936ed (added GenericParameterJob) * Remove duplicate $params check from Job::factory done in Job::__construct. * In Job::factory(), restore use of a valid title as default for passing as constructor arg to old job classes. Their constructor may expect it to be valid. Keep the invalid dummy in Job::__construct, and document why. * tests: Update test case for failure mode when using Job::factory with a class that requires a title. It asserted getting an invalid title. This now restores the behaviour prior to fc5d51f12936ed, which is that job classes that require a title, get a valid one. * tests: Remove test case for testToString that used an explicitly passed but invalid params value. I've converted that to expect the exception we now throw instead. * tests: Update getMockJob(), also used by testToString, which was relying on undocumented behaviour that 'new Title' is public and gets namespace=0 and title=''. Before fc5d51f12936ed, title params weren't in toString() and it asserted outputting three spaces (delimiter, empty string from formatted title, delimiter). In fc5d51f12936ed, this changed to asserting "Special:" which seems unintentional as we didn't pass it the internally reserved NS_SPECIAL/'' value, and yet was caught by the dbkey=='' check. Given this test case doesn't deal with titles, omit it for now. A job can either have a $title and title/namespace in params, or neither. This test was asserting an in-memory scenario where $title can be an object, but title/namespace absent from params. Bug: T221368 Depends-On: I89f6ad6967d6f82d87a62c15c0dded901c51b714 Change-Id: I2ec99a12ecc627359a2aae5153d5d7c54156ff46
2019-04-18 15:29:41 +00:00
$this->assertTrue( $blankPage->equals( $job->getTitle() ) );
$this->assertJobParamsMatch( $job, $paramsWithBlankpage );
}
public function testJobSignatureTitleBasedIncomplete() {
$testPage = Title::makeTitle( NS_PROJECT, 'X' );
$blankTitle = Title::makeTitle( NS_SPECIAL, '' );
$params = [ 'z' => 1, 'causeAction' => 'unknown', 'causeAgent' => 'unknown' ];
$job = new RefreshLinksJob( $testPage, $params + [ 'namespace' => 0 ] );
$this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $params + [ 'namespace' => 0 ] );
$job = new RefreshLinksJob( $testPage, $params + [ 'title' => 'x' ] );
$this->assertEquals( $blankTitle->getPrefixedText(), $job->getTitle()->getPrefixedText() );
$this->assertJobParamsMatch( $job, $params + [ 'title' => 'x' ] );
}
private function assertJobParamsMatch( IJobSpecification $job, array $params ) {
$actual = $job->getParams();
unset( $actual['requestId'] );
$this->assertEquals( $actual, $params );
}
}