jenkins-bot has submitted this change and it was merged.
Change subject: (bug 51363) Add more tests to for bad diffs.
......................................................................
(bug 51363) Add more tests to for bad diffs.
This adds high level tests for the fixes for bad diffs in change objects.
REQUIRES: I8ddc15d9 in Diff.
REQUIRES: I3835156f in WikibaseDataModel
Change-Id: I415a8b5c6aa3ff8894647d9ecf126181a3515ea7
---
M lib/includes/changes/ItemChange.php
M lib/tests/phpunit/changes/EntityChangeTest.php
A lib/tests/phpunit/changes/ItemChangeTest.php
M lib/tests/phpunit/changes/TestChanges.php
4 files changed, 241 insertions(+), 12 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/changes/ItemChange.php
b/lib/includes/changes/ItemChange.php
index dc6c499..cb4bf6c 100644
--- a/lib/includes/changes/ItemChange.php
+++ b/lib/includes/changes/ItemChange.php
@@ -2,6 +2,8 @@
namespace Wikibase;
+use Diff\Diff;
+
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -32,21 +34,22 @@
/**
* @since 0.3
*
- * @return \Diff\MapDiff|bool
+ * @return Diff
*/
public function getSiteLinkDiff() {
$diff = $this->getDiff();
if ( !$diff instanceof ItemDiff ) {
// This shouldn't happen, but we should be robust
against corrupt, incomplete
- // obsolete instances in the database, etc.
+ // or obsolete instances in the database, etc.
$cls = $diff === null ? 'null' : get_class( $diff );
- trigger_error(
- 'Cannot get sitelink diff from ' . $cls . '.
Change # ' . $this->getId()
- . ", type " . $this->getType(), E_USER_WARNING
);
- return new \Diff\Diff();
+ wfLogWarning(
+ 'Cannot get sitelink diff from ' . $cls . '.
Change #' . $this->getId()
+ . ", type " . $this->getType() );
+
+ return new Diff();
} else {
return $diff->getSiteLinkDiff();
}
diff --git a/lib/tests/phpunit/changes/EntityChangeTest.php
b/lib/tests/phpunit/changes/EntityChangeTest.php
index 855b736..5080866 100644
--- a/lib/tests/phpunit/changes/EntityChangeTest.php
+++ b/lib/tests/phpunit/changes/EntityChangeTest.php
@@ -76,13 +76,54 @@
return 'Wikibase\EntityChange';
}
+ /**
+ * Returns the name of the class of the entities under test.
+ *
+ * @since 0.4
+ * @return string
+ */
+ protected function getEntityClass() {
+ return 'Wikibase\Entity';
+ }
+
+
public function entityProvider() {
- return array_map(
+ $entityClass = $this->getEntityClass(); // PHP fail
+
+ $entities = array_filter(
+ TestChanges::getEntities(),
+ function( Entity $entity ) use ( $entityClass ) {
+ return is_a( $entity, $entityClass );
+ }
+ );
+
+ $cases = array_map(
function( Entity $entity ) {
return array( $entity );
},
- TestChanges::getEntities()
+ $entities
);
+
+ return $cases;
+ }
+
+ public function changeProvider() {
+ $rowClass = $this->getRowClass(); // PHP fail
+
+ $changes = array_filter(
+ TestChanges::getChanges(),
+ function( EntityChange $change ) use ( $rowClass ) {
+ return is_a( $change, $rowClass );
+ }
+ );
+
+ $cases = array_map(
+ function( EntityChange $change ) {
+ return array( $change );
+ },
+ $changes );
+
+ return $cases;
}
/**
@@ -101,7 +142,7 @@
}
/**
- * @dataProvider instanceProvider
+ * @dataProvider changeProvider
*
* @param \Wikibase\EntityChange $entityChange
*/
@@ -122,7 +163,7 @@
}
/**
- * @dataProvider instanceProvider
+ * @dataProvider changeProvider
* @since 0.3
*/
public function testMetadata( EntityChange $entityChange ) {
@@ -142,7 +183,7 @@
}
/**
- * @dataProvider instanceProvider
+ * @dataProvider changeProvider
* @since 0.3
*/
public function testGetEmptyMetadata( EntityChange $entityChange ) {
@@ -154,7 +195,7 @@
}
/**
- * @dataProvider instanceProvider
+ * @dataProvider changeProvider
* @since 0.4
*/
public function testToString( EntityChange $entityChange ) {
diff --git a/lib/tests/phpunit/changes/ItemChangeTest.php
b/lib/tests/phpunit/changes/ItemChangeTest.php
new file mode 100644
index 0000000..79c2eb9
--- /dev/null
+++ b/lib/tests/phpunit/changes/ItemChangeTest.php
@@ -0,0 +1,172 @@
+<?php
+
+namespace Wikibase\Test;
+use Diff\Diff;
+use Diff\DiffOpChange;
+use Exception;
+use Wikibase\EntityChange;
+use Wikibase\Item;
+use Wikibase\ItemChange;
+use Wikibase\Entity;
+use Wikibase\ItemDiff;
+
+/**
+ * Tests for the Wikibase\ItemChange class.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @since 0.3
+*
+ * @ingroup WikibaseLib
+ * @ingroup Test
+ *
+ * @group Database
+ * @group Wikibase
+ * @group WikibaseLib
+ * @group WikibaseChange
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Kinzler
+ */
+class ItemChangeTest extends EntityChangeTest {
+
+ /**
+ * @see ORMRowTest::getRowClass
+ * @since 0.4
+ * @return string
+ */
+ protected function getRowClass() {
+ return 'Wikibase\ItemChange';
+ }
+
+ public function entityProvider() {
+ $entities = array_filter(
+ TestChanges::getEntities(),
+ function( Entity $entity ) {
+ return ( $entity instanceof Item );
+ }
+ );
+
+ $cases = array_map(
+ function( Entity $entity ) {
+ return array( $entity );
+ },
+ $entities
+ );
+
+ return $cases;
+ }
+
+ public function itemChangeProvider() {
+ $changes = array_filter(
+ TestChanges::getChanges(),
+ function( EntityChange $change ) {
+ return ( $change instanceof ItemChange );
+ }
+ );
+
+ $cases = array_map( function( ItemChange $change ) {
+ return array( $change );
+ },
+ $changes );
+
+ return $cases;
+ }
+
+ /**
+ * @dataProvider changeProvider
+ *
+ * @param \Wikibase\ItemChange $change
+ */
+ public function testGetSiteLinkDiff( ItemChange $change ) {
+ $siteLinkDiff = $change->getSiteLinkDiff();
+ $this->assertInstanceOf( 'Diff\Diff', $siteLinkDiff,
+ "getSiteLinkDiff must return a Diff" );
+ }
+
+ public function changeBackwardsCompatProvider() {
+ //NOTE: Disable developer warnings that may get triggered by
+ // the B/C code path.
+ $this->setMwGlobals( 'wgDevelopmentWarnings', false );
+ wfSuppressWarnings();
+
+ $cases = array();
+
+ // --------
+ // We may hit a plain diff generated by old code.
+ // Make sure we can deal with that.
+
+ $diff = new Diff();
+
+ $change = ItemChange::newFromDiff( $diff, array( 'type' =>
'test' ) );
+ $cases['plain-diff'] = array( $change );
+
+ // --------
+ // Bug 51363: As of commit ff65735a125e, MapDiffer may generate
atomic diffs for
+ // substructures even in recursive mode. Make sure we can
handle them
+ // if we happen to load them from the database or such.
+
+ $diff = new ItemDiff( array(
+ 'links' => new DiffOpChange(
+ array( 'foowiki' => 'X', 'barwiki' => 'Y' ),
+ array( 'barwiki' => 'Y', 'foowiki' => 'X' )
+ )
+ ) );
+
+ // make sure we got the right key for sitelinks
+ assert( $diff->getSiteLinkDiff() !== null );
+
+ //NOTE: ItemChange's constructor may or may not already fix the
bad diff.
+
+ $change = ItemChange::newFromDiff( $diff, array( 'type' =>
'test' ) );
+ $cases['atomic-sitelink-diff'] = array( $change );
+
+ wfRestoreWarnings();
+ return $cases;
+ }
+
+ /**
+ * @dataProvider changeBackwardsCompatProvider
+ *
+ * @param \Wikibase\ItemChange $change
+ */
+ public function testGetSiteLinkDiffBackwardsCompat( ItemChange $change
) {
+ //NOTE: Disable developer warnings that may get triggered by
+ // the B/C code path.
+ $this->setMwGlobals( 'wgDevelopmentWarnings', false );
+
+ // Also suppress notices that may be triggered by wfLogWarning
+ wfSuppressWarnings();
+ $exception = null;
+
+ try {
+ $siteLinkDiff = $change->getSiteLinkDiff();
+ $this->assertInstanceOf( 'Diff\Diff', $siteLinkDiff,
+ "getSiteLinkDiff must return a Diff" );
+ } catch ( Exception $ex ) {
+ // PHP 5.3 doesn't have `finally`, so we use a hacky
emulation
+ $exception = $ex;
+ }
+
+ // this is our make-shift `finally` section.
+ wfRestoreWarnings();
+
+ if ( $exception ) {
+ throw $exception;
+ }
+ }
+}
diff --git a/lib/tests/phpunit/changes/TestChanges.php
b/lib/tests/phpunit/changes/TestChanges.php
index 741f1e8..d1ce6d1 100644
--- a/lib/tests/phpunit/changes/TestChanges.php
+++ b/lib/tests/phpunit/changes/TestChanges.php
@@ -108,6 +108,19 @@
$old = $new->copy();
// -----
+ $new->removeSiteLink( 'enwiki' );
+ $new->removeSiteLink( 'dewiki' );
+
+ $link = new SimpleSiteLink( 'enwiki', "Emmy" );
+ $new->addSimpleSiteLink( $link, 'add' );
+
+ $link = new SimpleSiteLink( 'dewiki', "Dummy" );
+ $new->addSimpleSiteLink( $link, 'add' );
+
+ $changes['change-sitelink-order'] =
EntityChange::newFromUpdate( EntityChange::UPDATE, $old, $new );
+ $old = $new->copy();
+
+ // -----
$link = new SimpleSiteLink( 'dewiki', "Dummy2" );
$new->addSimpleSiteLink( $link, 'set' );
$changes['change-dewiki-sitelink'] =
EntityChange::newFromUpdate( EntityChange::UPDATE, $old, $new );
--
To view, visit https://gerrit.wikimedia.org/r/73772
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I415a8b5c6aa3ff8894647d9ecf126181a3515ea7
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Denny Vrandecic <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
