Improve ApiQueryTestBase::assertResult
authorBrad Jorsch <bjorsch@wikimedia.org>
Wed, 4 Dec 2013 16:52:32 +0000 (11:52 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 4 Dec 2013 17:00:12 +0000 (12:00 -0500)
It really shouldn't be printing output in the middle of the phpunit
progress indicator; this output should be included in the final output
instead.

This rewrites the function to make use of PHPUnit's data structure
diffing support.

Bug: 57974
Change-Id: Ia4b66182922005da8e2bc173794d712492ad13fa

tests/phpunit/includes/api/query/ApiQueryContinueTestBase.php
tests/phpunit/includes/api/query/ApiQueryTestBase.php

index e7203c9..1b5a05e 100644 (file)
@@ -204,7 +204,7 @@ abstract class ApiQueryContinueTestBase extends ApiQueryTestBase {
                        if ( $numericIds ) {
                                ksort( $results, SORT_NUMERIC );
                        } elseif ( $sort !== null && $sort !== false ) {
-                               uasort( $results, $sort );
+                               usort( $results, $sort );
                        }
                }
        }
index 9cfab41..a657459 100644 (file)
@@ -102,47 +102,43 @@ STR;
 
        protected function assertResult( $exp, $result, $message = '' ) {
                try {
-                       $this->assertResultRecursive( $exp, $result );
-               } catch ( Exception $e ) {
+                       $exp = self::sanitizeResultArray( $exp );
+                       $result = self::sanitizeResultArray( $result );
+                       $this->assertEquals( $exp, $result );
+               } catch ( PHPUnit_Framework_ExpectationFailedException $e ) {
                        if ( is_array( $message ) ) {
                                $message = http_build_query( $message );
                        }
-                       print "\nRequest: $message\n";
-                       print "\nExpected:\n";
-                       print_r( $exp );
-                       print "\nResult:\n";
-                       print_r( $result );
-                       throw $e; // rethrow it
+                       throw new PHPUnit_Framework_ExpectationFailedException(
+                               $e->getMessage() . "\nRequest: $message",
+                               new PHPUnit_Framework_ComparisonFailure(
+                                       $exp,
+                                       $result,
+                                       print_r( $exp, true ),
+                                       print_r( $result, true ),
+                                       false,
+                                       $e->getComparisonFailure()->getMessage() . "\nRequest: $message"
+                               )
+                       );
                }
        }
 
        /**
-        * Recursively compare arrays, ignoring mismatches in numeric key and pageids.
-        * @param $expected array expected values
-        * @param $result array returned values
+        * Recursively ksorts a result array and removes any 'pageid' keys.
+        * @param array $result
+        * @return array
         */
-       private function assertResultRecursive( $expected, $result ) {
-               reset( $expected );
-               reset( $result );
-               while ( true ) {
-                       $e = each( $expected );
-                       $r = each( $result );
-                       // If either of the arrays is shorter, abort. If both are done, success.
-                       $this->assertEquals( (bool)$e, (bool)$r );
-                       if ( !$e ) {
-                               break; // done
-                       }
-                       // continue only if keys are identical or both keys are numeric
-                       $this->assertTrue( $e['key'] === $r['key'] || ( is_numeric( $e['key'] ) && is_numeric( $r['key'] ) ) );
-                       // don't compare pageids
-                       if ( $e['key'] !== 'pageid' ) {
-                               // If values are arrays, compare recursively, otherwise compare with ===
-                               if ( is_array( $e['value'] ) && is_array( $r['value'] ) ) {
-                                       $this->assertResultRecursive( $e['value'], $r['value'] );
-                               } else {
-                                       $this->assertEquals( $e['value'], $r['value'] );
-                               }
+       private static function sanitizeResultArray( $result ) {
+               unset( $result['pageid'] );
+               foreach ( $result as $key => $value ) {
+                       if ( is_array( $value ) ) {
+                               $result[$key] = self::sanitizeResultArray( $value );
                        }
                }
+
+               // Sort the result by keys, then take advantage of how array_merge will
+               // renumber numeric keys while leaving others alone.
+               ksort( $result );
+               return array_merge( $result );
        }
 }