ChangesListSpecialPageTest: Use Database::makeList() instead of makeshift DIY code
authorRoan Kattouw <roan.kattouw@gmail.com>
Thu, 5 Apr 2018 18:10:58 +0000 (11:10 -0700)
committerRoan Kattouw <roan.kattouw@gmail.com>
Fri, 6 Apr 2018 17:20:32 +0000 (10:20 -0700)
Otherwise these tests break completely when you add conditions like
$conds['rc_patrolled'] = [ 1, 2 ];
That maps to 'rc_patrolled IN (1,2)', but the DIY code in
normalizeCondition() was too simplistic to generate that.

Change-Id: I449317185f98e20b3e17f1b13610d872ae828171

includes/specialpage/ChangesListSpecialPage.php
tests/phpunit/includes/specialpage/ChangesListSpecialPageTest.php

index b9d20be..14b824f 100644 (file)
@@ -276,7 +276,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                                                'queryCallable' => function ( $specialClassName, $ctx, $dbr, &$tables, &$fields, &$conds,
                                                        &$query_options, &$join_conds
                                                ) {
-                                                       $conds[] = 'rc_bot = 0';
+                                                       $conds['rc_bot'] = 0;
                                                },
                                                'cssClassSuffix' => 'bot',
                                                'isRowApplicableCallable' => function ( $ctx, $rc ) {
@@ -291,7 +291,7 @@ abstract class ChangesListSpecialPage extends SpecialPage {
                                                'queryCallable' => function ( $specialClassName, $ctx, $dbr, &$tables, &$fields, &$conds,
                                                        &$query_options, &$join_conds
                                                ) {
-                                                       $conds[] = 'rc_bot = 1';
+                                                       $conds['rc_bot'] = 1;
                                                },
                                                'cssClassSuffix' => 'human',
                                                'isRowApplicableCallable' => function ( $ctx, $rc ) {
index d612b53..5118218 100644 (file)
@@ -105,9 +105,14 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
        }
 
        private static function normalizeCondition( $conds ) {
+               $dbr = wfGetDB( DB_REPLICA );
                $normalized = array_map(
-                       function ( $k, $v ) {
-                               return is_numeric( $k ) ? $v : "$k = $v";
+                       function ( $k, $v ) use ( $dbr ) {
+                               if ( is_array( $v ) ) {
+                                       sort( $v );
+                               }
+                               // (Ab)use makeList() to format only this entry
+                               return $dbr->makeList( [ $k => $v ], Database::LIST_AND );
                        },
                        array_keys( $conds ),
                        $conds
@@ -116,9 +121,9 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                return $normalized;
        }
 
-       /** return false if condition begin with 'rc_timestamp ' */
+       /** return false if condition begins with 'rc_timestamp ' */
        private static function filterOutRcTimestampCondition( $var ) {
-               return ( false === strpos( $var, 'rc_timestamp ' ) );
+               return ( is_array( $var ) || false === strpos( $var, 'rc_timestamp ' ) );
        }
 
        public function testRcNsFilter() {
@@ -342,7 +347,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                $user = $this->getTestSysop()->getUser();
                $this->assertConditions(
                        [ # expected
-                               "rc_patrolled = 0",
+                               'rc_patrolled = 0',
                        ],
                        [
                                'hidepatrolled' => 1,
@@ -356,7 +361,7 @@ class ChangesListSpecialPageTest extends AbstractChangesListSpecialPageTestCase
                $user = $this->getTestSysop()->getUser();
                $this->assertConditions(
                        [ # expected
-                               "rc_patrolled != 0",
+                               'rc_patrolled != 0',
                        ],
                        [
                                'hideunpatrolled' => 1,