Test deleteOldRevisions.php

Why:
* Maintenance scripts in core are mostly untested and testing them
  will help to avoid regressions.
** For example, in testing the script a bug is found where the
   Maintenance::purgeRedundantText method will throw an
   unexpected exception if no valid rows exist in the
   content_address table.

What:
* Create DeleteOldRevisionsTest that fully tests the associated
  maintenance script.
* Update Maintenance::purgeRedundantText to not attempt to
  use an empty list as the value provided to
  IReadableDatabase::getExpr.
** Add a regression test for this.

Bug: T371167
Change-Id: Id0c3c5d6fe56bc9fe5d5347c82dab9ab61137f58
This commit is contained in:
Dreamy Jazz 2024-08-19 10:43:35 +01:00
parent 57777e1c08
commit 06ccac8c49
4 changed files with 127 additions and 6 deletions

View file

@ -36,12 +36,12 @@ class DeleteOldRevisions extends Maintenance {
parent::__construct();
$this->addDescription( 'Delete old (non-current) revisions from the database' );
$this->addOption( 'delete', 'Actually perform the deletion' );
$this->addOption( 'page_id', 'List of page ids to work on', false );
$this->addArg( 'page_id', 'List of page ids to work on', false, true );
}
public function execute() {
$this->output( "Delete old revisions\n\n" );
$this->doDelete( $this->hasOption( 'delete' ), $this->getArgs() );
$this->doDelete( $this->hasOption( 'delete' ), $this->getArgs( 'page_id' ) );
}
private function doDelete( $delete = false, $pageIds = [] ) {

View file

@ -1034,12 +1034,16 @@ abstract class Maintenance {
# Get the IDs of all text records not in these sets
$this->output( 'Searching for inactive text records...' );
$res = $dbw->newSelectQueryBuilder()
$textTableQueryBuilder = $dbw->newSelectQueryBuilder()
->select( 'old_id' )
->distinct()
->from( 'text' )
->where( $dbw->expr( 'old_id', '!=', $cur ) )
->caller( __METHOD__ )->fetchResultSet();
->from( 'text' );
if ( count( $cur ) ) {
$textTableQueryBuilder->where( $dbw->expr( 'old_id', '!=', $cur ) );
}
$res = $textTableQueryBuilder
->caller( __METHOD__ )
->fetchResultSet();
$old = [];
foreach ( $res as $row ) {
$old[] = $row->old_id;

View file

@ -0,0 +1,110 @@
<?php
namespace MediaWiki\Tests\Maintenance;
use DeleteOldRevisions;
use WikiPage;
/**
* @covers \DeleteOldRevisions
* @group Database
* @author Dreamy Jazz
*/
class DeleteOldRevisionsTest extends MaintenanceBaseTestCase {
public function getMaintenanceClass() {
return DeleteOldRevisions::class;
}
private function expectTheExpectedOutputRegex(
bool $shouldDelete, int $specifiedPageId, int $expectedOldRevisionsCount
) {
$expectedOutputRegex = '/Delete old revisions';
// Only expect the page IDs to be outputted if we specified some.
if ( $specifiedPageId ) {
$expectedOutputRegex .= '[\s\S]*Limiting to page IDs.*' . $specifiedPageId;
} else {
$expectedOutputRegex .= '[\s\S]*(?!Limiting to page IDs)';
}
$expectedOutputRegex .= '[\s\S]*Searching for active revisions.*done[\s\S]*' .
'Searching for inactive revisions.*done[\s\S]*' .
"$expectedOldRevisionsCount old revisions found";
if ( $shouldDelete ) {
$expectedOutputRegex .= '[\s\S]*Deleting/';
} else {
$expectedOutputRegex .= '[\s\S]*(?!Deleting)/';
}
$this->expectOutputRegex( $expectedOutputRegex );
}
/** @dataProvider provideExecuteForNoPages */
public function testExecuteForNoPages( $deleteOptionProvided ) {
if ( $deleteOptionProvided ) {
$this->maintenance->setOption( 'delete', 1 );
}
$this->maintenance->execute();
$this->expectTheExpectedOutputRegex( false, 0, 0 );
}
public static function provideExecuteForNoPages() {
return [
'Delete option provided' => [ true ],
'Delete option not provided' => [ false ],
];
}
private function verifyRevisionCountForPage( WikiPage $page, int $expectedCount ) {
$this->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'revision' )
->where( [ 'rev_page' => $page->getId() ] )
->assertFieldValue( $expectedCount );
}
/** @dataProvider provideExecute */
public function testExecute( $shouldDelete, $shouldSpecifyPageId, $expectedOldRevisionsCount ) {
$firstExistingTestPage = $this->getExistingTestPage();
$secondExistingTestPage = $this->getExistingTestPage();
$thirdExistingTestPage = $this->getExistingTestPage();
// Make an edit to both the first and second existing test pages, so that they have old revisions
$this->editPage( $firstExistingTestPage, 'abcdef' );
$this->editPage( $secondExistingTestPage, 'abcef' );
// Check that the revision count for each page is correct for the test.
$this->verifyRevisionCountForPage( $thirdExistingTestPage, 1 );
$this->verifyRevisionCountForPage( $secondExistingTestPage, 2 );
$this->verifyRevisionCountForPage( $firstExistingTestPage, 2 );
// Set the options for the maintenance script
if ( $shouldDelete ) {
$this->maintenance->setOption( 'delete', 1 );
}
if ( $shouldSpecifyPageId ) {
$this->maintenance->setArg( 'page_id', $firstExistingTestPage->getId() );
}
// Run the maintenance script and then verify the number of revisions for each test page is as expected
$this->maintenance->execute();
$this->verifyRevisionCountForPage( $thirdExistingTestPage, 1 );
$this->verifyRevisionCountForPage(
$secondExistingTestPage,
// Should not have been touched, unless --delete was specified and --page_id was not specified
$shouldDelete && !$shouldSpecifyPageId ? 1 : 2
);
$this->verifyRevisionCountForPage(
$firstExistingTestPage,
// Should not be touched, unless --delete is specified.
$shouldDelete ? 1 : 2
);
$this->expectTheExpectedOutputRegex(
$shouldDelete,
$shouldSpecifyPageId ? $firstExistingTestPage->getId() : 0,
$expectedOldRevisionsCount
);
}
public static function provideExecute() {
return [
'No options provided' => [ false, false, 2 ],
'Not deleting, but specified page ID' => [ false, true, 1 ],
'Deleting without specifying page IDs' => [ true, false, 2 ],
'Deleting while specifying page ID' => [ true, true, 1 ],
];
}
}

View file

@ -11,6 +11,7 @@ use Wikimedia\TestingAccessWrapper;
/**
* @covers \Maintenance
* @covers \MediaWiki\Maintenance\MaintenanceFatalError
* @group Database
*/
class MaintenanceTest extends MaintenanceBaseTestCase {
@ -673,4 +674,10 @@ class MaintenanceTest extends MaintenanceBaseTestCase {
'Batch size as 150' => [ 150, true ],
];
}
public function testPurgeRedundantTextWhenNoPagesExist() {
// Regression test for the method breaking if no rows exist in the content_address table.
$this->maintenance->purgeRedundantText();
$this->expectOutputRegex( '/0 inactive items found[\s\S]*(?!Deleting)/' );
}
}