Daniel Werner has submitted this change and it was merged. Change subject: Added and fixed patcher tests and two uncovered bugs ......................................................................
Added and fixed patcher tests and two uncovered bugs Change-Id: I25639436313879d145354fadaa053f900690116d --- M RELEASE-NOTES M includes/patcher/MapPatcher.php M tests/phpunit/patcher/ListPatcherTest.php M tests/phpunit/patcher/MapPatcherTest.php 4 files changed, 125 insertions(+), 41 deletions(-) Approvals: Daniel Werner: Looks good to me, approved diff --git a/RELEASE-NOTES b/RELEASE-NOTES index a9b39a3..c5995a4 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -7,9 +7,21 @@ === Version 0.7 === (dev) +; Improvements + +* Added extra tests for MapPatcher and ListPatcher +* Added extra tests for Diff + ; Removals * Removed static methods from ListDiff and MapDiff (all deprecated since 0.4) +* Removed DiffOpTestDummy + +; Bug fixes + +* MapPatcher will now no longer stop patching after the first remove operation it encounters +* MapPatcher now always treats its top level input diff as a map diff +* Fixed several issues in ListPatcherTest === Version 0.6 === 2013-05-08 diff --git a/includes/patcher/MapPatcher.php b/includes/patcher/MapPatcher.php index 2ebd70f..fc6e82a 100644 --- a/includes/patcher/MapPatcher.php +++ b/includes/patcher/MapPatcher.php @@ -65,44 +65,14 @@ } /** - * Sets the value comparer that should be used to determine if values are equal. - * - * @since 0.6 - * - * @param ValueComparer $comparer - */ - public function setValueComparer( ValueComparer $comparer ) { - $this->comparer = $comparer; - } - - /** * @see Patcher::patch * - * @since 0.4 - * - * @param array $base - * @param Diff $diff - * - * @return array - */ - public function patch( array $base, Diff $diff ) { - if ( $diff->looksAssociative() ) { - $base = $this->getPatchedMap( $base, $diff ); - } - else { - $base = $this->listPatcher->patch( $base, $diff ); - } - - return $base; - } - - /** * Applies the provided diff to the provided array and returns the result. * The array is treated as a map, ie keys are held into account. * * It is possible to pass in non-associative diffs (those for which isAssociative) * returns false, however the likely intended behavior can be obtained via - * @see getPatchedList + * a list patcher. * * @since 0.4 * @@ -112,7 +82,7 @@ * @return array * @throws RuntimeException */ - protected function getPatchedMap( array $base, Diff $diff ) { + public function patch( array $base, Diff $diff ) { /** * @var DiffOp $diffOp */ @@ -135,7 +105,7 @@ $base[$key] = array(); } - $base[$key] = $this->patch( $base[$key], $diffOp ); + $base[$key] = $this->patchMapOrList( $base[$key], $diffOp ); } else if ( $diffOp instanceof DiffOpRemove ) { if ( !array_key_exists( $key, $base ) ) { @@ -144,7 +114,6 @@ } unset( $base[$key] ); - break; } else if ( $diffOp instanceof DiffOpChange ) { if ( !array_key_exists( $key, $base ) ) { @@ -167,6 +136,17 @@ return $base; } + protected function patchMapOrList( array $base, Diff $diff ) { + if ( $diff->looksAssociative() ) { + $base = $this->patch( $base, $diff ); + } + else { + $base = $this->listPatcher->patch( $base, $diff ); + } + + return $base; + } + protected function valuesAreEqual( $firstValue, $secondValue ) { if ( $this->comparer === null ) { $this->comparer = new StrictComparer(); @@ -175,4 +155,15 @@ return $this->comparer->valuesAreEqual( $firstValue, $secondValue ); } + /** + * Sets the value comparer that should be used to determine if values are equal. + * + * @since 0.6 + * + * @param ValueComparer $comparer + */ + public function setValueComparer( ValueComparer $comparer ) { + $this->comparer = $comparer; + } + } diff --git a/tests/phpunit/patcher/ListPatcherTest.php b/tests/phpunit/patcher/ListPatcherTest.php index 8234c01..9ec89a1 100644 --- a/tests/phpunit/patcher/ListPatcherTest.php +++ b/tests/phpunit/patcher/ListPatcherTest.php @@ -141,7 +141,7 @@ * @param string|null $message */ public function testGetApplicableDiff( Diff $diff, array $currentObject, Diff $expected, $message = null ) { - $patcher = new MapPatcher(); + $patcher = new ListPatcher(); $actual = $patcher->getApplicableDiff( $currentObject, $diff ); $this->assertEquals( $expected->getOperations(), $actual->getOperations(), $message ); @@ -208,8 +208,8 @@ $diff = new Diff( array( - 'en' => new Diff( array( new DiffOpAdd( 42 ) ), false ), - ), true ); + new DiffOpAdd( 42 ), + ), false ); $currentObject = array(); @@ -219,11 +219,14 @@ $diff = new Diff( array( - 'en' => new Diff( array( new DiffOpRemove( 42 ) ), false ), - ), true ); + new DiffOpRemove( 42 ), + new DiffOpRemove( 9001 ), + ), false ); $currentObject = array( - 'en' => array( 42 ), + 42, + 72010, + 9001, ); $expected = clone $diff; diff --git a/tests/phpunit/patcher/MapPatcherTest.php b/tests/phpunit/patcher/MapPatcherTest.php index e8d2c3e..f0f171f 100644 --- a/tests/phpunit/patcher/MapPatcherTest.php +++ b/tests/phpunit/patcher/MapPatcherTest.php @@ -36,6 +36,7 @@ * * @group Diff * @group DiffPatcher + * @group MapPatcherTest * * @licence GNU GPL v2+ * @author Jeroen De Dauw < jeroended...@gmail.com > @@ -49,6 +50,83 @@ $base = array(); $diff = new Diff(); $expected = array(); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array( 'foo', 'bar' => array( 'baz' ) ); + $diff = new Diff(); + $expected = array( 'foo', 'bar' => array( 'baz' ) ); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array( 'foo', 'bar' => array( 'baz' ) ); + $diff = new Diff( array( 'bah' => new DiffOpAdd( 'blah' ) ) ); + $expected = array( 'foo', 'bar' => array( 'baz' ), 'bah' => 'blah' ); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array( 'foo', 'bar' => array( 'baz' ) ); + $diff = new Diff( array( 'bah' => new DiffOpAdd( 'blah' ) ) ); + $expected = array( 'foo', 'bar' => array( 'baz' ), 'bah' => 'blah' ); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array(); + $diff = new Diff( array( + 'foo' => new DiffOpAdd( 'bar' ), + 'bah' => new DiffOpAdd( 'blah' ) + ) ); + $expected = array( + 'foo' => 'bar', + 'bah' => 'blah' + ); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array( + 'foo' => 'bar', + 'nyan' => 'cat', + 'bah' => 'blah', + ); + $diff = new Diff( array( + 'foo' => new DiffOpRemove( 'bar' ), + 'bah' => new DiffOpRemove( 'blah' ), + ) ); + $expected = array( + 'nyan' => 'cat' + ); + + $argLists[] = array( $patcher, $base, $diff, $expected ); + + + $patcher = new MapPatcher(); + $base = array( + 'foo' => 'bar', + 'nyan' => 'cat', + 'spam' => 'blah', + 'bah' => 'blah', + ); + $diff = new Diff( array( + 'foo' => new DiffOpChange( 'bar', 'baz' ), + 'bah' => new DiffOpRemove( 'blah' ), + 'oh' => new DiffOpAdd( 'noez' ), + ) ); + $expected = array( + 'foo' => 'baz', + 'nyan' => 'cat', + 'spam' => 'blah', + 'oh' => 'noez', + ); $argLists[] = array( $patcher, $base, $diff, $expected ); @@ -68,7 +146,7 @@ public function testPatch( Patcher $patcher, array $base, Diff $diff, array $expected ) { $actual = $patcher->patch( $base, $diff ); - $this->assertArrayEquals( $actual, $expected ); + $this->assertArrayEquals( $expected, $actual, true, true ); } public function getApplicableDiffProvider() { -- To view, visit https://gerrit.wikimedia.org/r/65259 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I25639436313879d145354fadaa053f900690116d Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Diff Gerrit-Branch: master Gerrit-Owner: Jeroen De Dauw <jeroended...@gmail.com> Gerrit-Reviewer: Anja Jentzsch <a...@anjeve.de> Gerrit-Reviewer: Ataherivand <abraham.taheriv...@wikimedia.de> Gerrit-Reviewer: Aude <aude.w...@gmail.com> Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de> Gerrit-Reviewer: Daniel Werner <daniel.wer...@wikimedia.de> Gerrit-Reviewer: Denny Vrandecic <denny.vrande...@wikimedia.de> Gerrit-Reviewer: Henning Snater <henning.sna...@wikimedia.de> Gerrit-Reviewer: Jens Ohlig <jens.oh...@wikimedia.de> Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com> Gerrit-Reviewer: John Erling Blad <jeb...@gmail.com> Gerrit-Reviewer: Lydia Pintscher <lydia.pintsc...@wikimedia.de> Gerrit-Reviewer: Markus Kroetzsch <mar...@semantic-mediawiki.org> Gerrit-Reviewer: Nikola Smolenski <smole...@eunet.rs> Gerrit-Reviewer: Silke Meyer <silke.me...@wikimedia.de> Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits