From 40220100adf1c6f25c269f9bf737061983dd64dd Mon Sep 17 00:00:00 2001 From: Marius Hoch Date: Tue, 3 Apr 2018 07:15:43 +0200 Subject: [PATCH] Add further test cases to deleteAutoPatrolLogsTest Also make --batch-size work and make an if check nicer (makes no functional difference AFAICT). Bug: T189594 Change-Id: I3ce63386cb35441acfa226f313ec8aac1aa417a6 --- maintenance/deleteAutoPatrolLogs.php | 4 +- .../maintenance/deleteAutoPatrolLogsTest.php | 213 +++++++++--------- 2 files changed, 105 insertions(+), 112 deletions(-) diff --git a/maintenance/deleteAutoPatrolLogs.php b/maintenance/deleteAutoPatrolLogs.php index 3082d60a02..73e0baa461 100644 --- a/maintenance/deleteAutoPatrolLogs.php +++ b/maintenance/deleteAutoPatrolLogs.php @@ -56,6 +56,8 @@ class DeleteAutoPatrolLogs extends Maintenance { } public function execute() { + $this->setBatchSize( $this->getOption( 'batch-size', $this->getBatchSize() ) ); + $sleep = (int)$this->getOption( 'sleep', 10 ); $fromId = $this->getOption( 'from-id', null ); $this->countDown( 5 ); @@ -159,7 +161,7 @@ class DeleteAutoPatrolLogs extends Maintenance { Wikimedia\restoreWarnings(); // Skipping really old rows, before 2011 - if ( is_array( $params ) && !array_key_exists( '6::auto', $params ) ) { + if ( !is_array( $params ) || !array_key_exists( '6::auto', $params ) ) { continue; } diff --git a/tests/phpunit/maintenance/deleteAutoPatrolLogsTest.php b/tests/phpunit/maintenance/deleteAutoPatrolLogsTest.php index af29ff9ca7..c141817478 100644 --- a/tests/phpunit/maintenance/deleteAutoPatrolLogsTest.php +++ b/tests/phpunit/maintenance/deleteAutoPatrolLogsTest.php @@ -56,6 +56,15 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { 'log_timestamp' => 20061223210426 ]; + // Very old/ invalid patrol + $logs[] = [ + 'log_type' => 'patrol', + 'log_action' => 'patrol', + 'log_user' => 7253, + 'log_params' => 'nanana', + 'log_timestamp' => 20061223210426 + ]; + // Autopatrol #2 $logs[] = [ 'log_type' => 'patrol', @@ -86,58 +95,8 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { wfGetDB( DB_MASTER )->insert( 'logging', $logs ); } - public function testBasicRun() { - $this->maintenance->loadWithArgv( [ '--sleep', '0', '-q' ] ); - - $this->maintenance->execute(); - - $remainingLogs = wfGetDB( DB_REPLICA )->select( - [ 'logging' ], - [ 'log_type', 'log_action', 'log_user' ], - [], - __METHOD__, - [ 'ORDER BY' => 'log_id' ] - ); - - $expected = [ - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7251', - ], - (object)[ - 'log_type' => 'block', - 'log_action' => 'block', - 'log_user' => '7253', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7255', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7256', - ], - ]; - $this->assertEquals( $expected, iterator_to_array( $remainingLogs, false ) ); - } - - public function testDryRun() { - $this->maintenance->loadWithArgv( [ '--sleep', '0', '--dry-run', '-q' ] ); - - $this->maintenance->execute(); - - $remainingLogs = wfGetDB( DB_REPLICA )->select( - [ 'logging' ], - [ 'log_type', 'log_action', 'log_user' ], - [], - __METHOD__, - [ 'ORDER BY' => 'log_id' ] - ); - - $expected = [ + public function runProvider() { + $allRows = [ (object)[ 'log_type' => 'patrol', 'log_action' => 'patrol', @@ -153,6 +112,11 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { 'log_action' => 'block', 'log_user' => '7253', ], + (object)[ + 'log_type' => 'patrol', + 'log_action' => 'patrol', + 'log_user' => '7253', + ], (object)[ 'log_type' => 'patrol', 'log_action' => 'autopatrol', @@ -169,11 +133,67 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { 'log_user' => '7256', ], ]; - $this->assertEquals( $expected, iterator_to_array( $remainingLogs, false ) ); + + $cases = [ + 'dry run' => [ + $allRows, + [ '--sleep', '0', '--dry-run', '-q' ] + ], + 'basic run' => [ + [ + $allRows[0], + $allRows[2], + $allRows[3], + $allRows[5], + $allRows[6], + ], + [ '--sleep', '0', '-q' ] + ], + 'run with before' => [ + [ + $allRows[0], + $allRows[2], + $allRows[3], + $allRows[4], + $allRows[5], + $allRows[6], + ], + [ '--sleep', '0', '--before', '20060123210426', '-q' ] + ], + 'run with check-old' => [ + [ + $allRows[0], + $allRows[1], + $allRows[2], + $allRows[3], + $allRows[4], + $allRows[6], + ], + [ '--sleep', '0', '--check-old', '-q' ] + ], + ]; + + foreach ( $cases as $key => $case ) { + yield $key . '-batch-size-1' => [ + $case[0], + array_merge( $case[1], [ '--batch-size', '1' ] ) + ]; + yield $key . '-batch-size-5' => [ + $case[0], + array_merge( $case[1], [ '--batch-size', '5' ] ) + ]; + yield $key . '-batch-size-1000' => [ + $case[0], + array_merge( $case[1], [ '--batch-size', '1000' ] ) + ]; + } } - public function testRunWithTimestamp() { - $this->maintenance->loadWithArgv( [ '--sleep', '0', '--before', '20060123210426', '-q' ] ); + /** + * @dataProvider runProvider + */ + public function testRun( $expected, $args ) { + $this->maintenance->loadWithArgv( $args ); $this->maintenance->execute(); @@ -185,38 +205,17 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { [ 'ORDER BY' => 'log_id' ] ); - $expected = [ - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7251', - ], - (object)[ - 'log_type' => 'block', - 'log_action' => 'block', - 'log_user' => '7253', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'autopatrol', - 'log_user' => '7254', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7255', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7256', - ] - ]; $this->assertEquals( $expected, iterator_to_array( $remainingLogs, false ) ); } - public function testRunWithCheckOld() { - $this->maintenance->loadWithArgv( [ '--sleep', '0', '--check-old', '-q' ] ); + public function testFromId() { + $fromId = wfGetDB( DB_REPLICA )->selectField( + 'logging', + 'log_id', + [ 'log_params' => 'nanana' ] + ); + + $this->maintenance->loadWithArgv( [ '--sleep', '0', '--from-id', strval( $fromId ), '-q' ] ); $this->maintenance->execute(); @@ -228,34 +227,26 @@ class DeleteAutoPatrolLogsTest extends MaintenanceBaseTestCase { [ 'ORDER BY' => 'log_id' ] ); - $expected = [ - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7251', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'autopatrol', - 'log_user' => '7252', - ], - (object)[ - 'log_type' => 'block', - 'log_action' => 'block', - 'log_user' => '7253', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'autopatrol', - 'log_user' => '7254', - ], - (object)[ - 'log_type' => 'patrol', - 'log_action' => 'patrol', - 'log_user' => '7256', - ] + $deleted = [ + 'log_type' => 'patrol', + 'log_action' => 'autopatrol', + 'log_user' => '7254', ]; - $this->assertEquals( $expected, iterator_to_array( $remainingLogs, false ) ); + $notDeleted = [ + 'log_type' => 'patrol', + 'log_action' => 'autopatrol', + 'log_user' => '7252', + ]; + + $remainingLogs = array_map( + function ( $val ) { + return (array)$val; + }, + iterator_to_array( $remainingLogs, false ) + ); + + $this->assertNotContains( $deleted, $remainingLogs ); + $this->assertContains( $notDeleted, $remainingLogs ); } } -- 2.20.1