Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/65259
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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Diff
refs/changes/59/65259/1
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 < [email protected] >
@@ -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: newchange
Gerrit-Change-Id: I25639436313879d145354fadaa053f900690116d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Diff
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
