Thiemo Mättig (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/330229 )
Change subject: Cleanup EntityDiffVisualizerTest
......................................................................
Cleanup EntityDiffVisualizerTest
This is a direct follow up that picks up the changes done in
I2f53b0e. Main changes in here are:
* There was only a single case using assertRegExp(). I replaced this
with a dedicated test method.
* Replaced all assertTag() with assertContains().
* The Context, Language and Message stuff is now entirely mocked and
does not use real translations any more.
Change-Id: Ie06b7cdfcee9ba1264c23a989b1ec5c8c7865711
---
M repo/includes/Diff/DiffView.php
M repo/includes/Diff/EntityDiffVisualizer.php
M repo/tests/phpunit/includes/Diff/EntityDiffVisualizerTest.php
3 files changed, 63 insertions(+), 58 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/29/330229/1
diff --git a/repo/includes/Diff/DiffView.php b/repo/includes/Diff/DiffView.php
index e45463a..9eff5c6 100644
--- a/repo/includes/Diff/DiffView.php
+++ b/repo/includes/Diff/DiffView.php
@@ -32,11 +32,6 @@
class DiffView extends ContextSource {
/**
- * @var SiteLookup
- */
- private $siteLookup;
-
- /**
* @var string[]
*/
private $path;
@@ -45,6 +40,11 @@
* @var Diff
*/
private $diff;
+
+ /**
+ * @var SiteLookup
+ */
+ private $siteLookup;
/**
* @var EntityIdFormatter
@@ -74,12 +74,13 @@
) {
$this->path = $path;
$this->diff = $diff;
- $this->entityIdFormatter = $entityIdFormatter;
$this->siteLookup = $siteLookup;
+ $this->entityIdFormatter = $entityIdFormatter;
if ( !is_null( $contextSource ) ) {
$this->setContext( $contextSource );
}
+
$this->siteLinkPath = $this->msg( 'wikibase-diffview-link'
)->text();
}
@@ -105,16 +106,17 @@
*/
private function generateOpHtml( array $path, DiffOp $op ) {
if ( $op->isAtomic() ) {
- $translatedPath = $path;
+ $localizedPath = $path;
- if ( $this->isSiteLinkPath( $path ) ) {
+ if ( $this->isSiteLinkPath( $path ) && isset( $path[2]
) ) {
$translatedLinkSubPath = $this->msg(
'wikibase-diffview-link-' . $path[2] );
+
if ( !$translatedLinkSubPath->isDisabled() ) {
- $translatedPath[2] =
$translatedLinkSubPath->text();
+ $localizedPath[2] =
$translatedLinkSubPath->text();
}
}
- $html = $this->generateDiffHeaderHtml( implode( ' / ',
$translatedPath ) );
+ $html = $this->generateDiffHeaderHtml( implode( ' / ',
$localizedPath ) );
//TODO: no path, but localized section title
@@ -209,9 +211,20 @@
} else {
$value = $this->getSiteLinkElement( $path[1],
$value );
}
+
return Html::rawElement( $tag, array( 'class' =>
'diffchange diffchange-inline' ), $value );
}
+
return Html::element( $tag, array( 'class' => 'diffchange
diffchange-inline' ), $value );
+ }
+
+ /**
+ * @param string[] $path
+ *
+ * @return bool
+ */
+ private function isSiteLinkPath( array $path ) {
+ return $path[0] === $this->siteLinkPath && isset( $path[1] );
}
/**
@@ -267,14 +280,6 @@
$html .= Html::closeElement( 'tr' );
return $html;
- }
-
- /**
- * @param string[] $path
- * @return boolean
- */
- private function isSiteLinkPath( array $path ) {
- return count( $path ) === 3 && $path[0] === $this->siteLinkPath;
}
}
diff --git a/repo/includes/Diff/EntityDiffVisualizer.php
b/repo/includes/Diff/EntityDiffVisualizer.php
index 4776733..10a3e74 100644
--- a/repo/includes/Diff/EntityDiffVisualizer.php
+++ b/repo/includes/Diff/EntityDiffVisualizer.php
@@ -61,7 +61,8 @@
* @param SiteLookup $siteLookup
* @param EntityIdFormatter $entityIdFormatter
*/
- public function __construct( IContextSource $contextSource,
+ public function __construct(
+ IContextSource $contextSource,
ClaimDiffer $claimDiffer,
ClaimDifferenceVisualizer $claimDiffView,
SiteLookup $siteLookup,
@@ -107,13 +108,13 @@
$html = '';
$termDiffVisualizer = new DiffView(
- array(),
+ [],
new Diff(
- array(
+ [
$this->context->msg(
'wikibase-diffview-label' )->text() => $diff->getLabelsDiff(),
$this->context->msg(
'wikibase-diffview-alias' )->text() => $diff->getAliasesDiff(),
$this->context->msg(
'wikibase-diffview-description' )->text() => $diff->getDescriptionsDiff(),
- ),
+ ],
true
),
$this->siteLookup,
@@ -130,11 +131,11 @@
// FIXME: this does not belong here as it is specific to items
if ( $diff instanceof ItemDiff ) {
$linkDiffVisualizer = new DiffView(
- array(),
+ [],
new Diff(
- array(
+ [
$this->context->msg(
'wikibase-diffview-link' )->text() => $diff->getSiteLinkDiff(),
- ),
+ ],
true
),
$this->siteLookup,
diff --git a/repo/tests/phpunit/includes/Diff/EntityDiffVisualizerTest.php
b/repo/tests/phpunit/includes/Diff/EntityDiffVisualizerTest.php
index 0776a09..9cd87f3 100644
--- a/repo/tests/phpunit/includes/Diff/EntityDiffVisualizerTest.php
+++ b/repo/tests/phpunit/includes/Diff/EntityDiffVisualizerTest.php
@@ -7,8 +7,8 @@
use Diff\DiffOp\DiffOpRemove;
use HashSiteStore;
use IContextSource;
-use Language;
use MediaWikiTestCase;
+use RawMessage;
use Site;
use Wikibase\DataModel\Services\Diff\EntityDiff;
use Wikibase\DataModel\Services\EntityId\EntityIdFormatter;
@@ -28,9 +28,14 @@
*/
class EntityDiffVisualizerTest extends MediaWikiTestCase {
- public function diffProvider() {
- $emptyDiff = new EntityContentDiff( new EntityDiff(), new
Diff() );
+ public function testEmptyDiff() {
+ $diff = new EntityContentDiff( new EntityDiff(), new Diff() );
+ $html = $this->getVisualizer()->visualizeEntityContentDiff(
$diff );
+ $this->assertSame( '', $html );
+ }
+
+ public function diffProvider() {
$fingerprintDiff = new EntityContentDiff(
new EntityDiff( array(
'label' => new Diff( array(
@@ -46,23 +51,23 @@
new DiffOpAdd( 'daaaah'
),
new DiffOpRemove( 'foo'
),
new DiffOpRemove( 'bar'
),
- ) )
+ ) )
), true ),
) ),
new Diff()
);
$fingerprintTags = array(
- 'has <td>label / en</td>' => array( 'tag' => 'td',
'content' => 'label / en' ),
- 'has <ins>O_o</ins>' => array( 'tag' => 'ins',
'content' => 'O_o' ),
- 'has <td>aliases / nl / 0</td>' => array( 'tag' =>
'td', 'content' => 'aliases / nl / 0' ),
- 'has <ins>daaaah</ins>' => array( 'tag' => 'ins',
'content' => 'daaaah' ),
- 'has <td>aliases / nl / 1</td>' => array( 'tag' =>
'td', 'content' => 'aliases / nl / 1' ),
- 'has <del>foo</del>' => array( 'tag' => 'del',
'content' => 'foo' ),
- 'has <td>aliases / nl / 2</td>' => array( 'tag' =>
'td', 'content' => 'aliases / nl / 2' ),
- 'has <del>bar</del>' => array( 'tag' => 'del',
'content' => 'bar' ),
- 'has <td>description / en</td>' => array( 'tag' =>
'td', 'content' => 'description / en' ),
- 'has <del>ohi there</del>' => array( 'tag' => 'del',
'content' => 'ohi there' ),
+ 'has <td>label / en</td>' =>
'>(wikibase-diffview-label) / en</td>',
+ 'has <ins>O_o</ins>' => '>O_o</ins>',
+ 'has <td>aliases / nl / 0</td>' =>
'>(wikibase-diffview-alias) / nl / 0</td>',
+ 'has <ins>daaaah</ins>' => '>daaaah</ins>',
+ 'has <td>aliases / nl / 1</td>' =>
'>(wikibase-diffview-alias) / nl / 1</td>',
+ 'has <del>foo</del>' => '>foo</del>',
+ 'has <td>aliases / nl / 2</td>' =>
'>(wikibase-diffview-alias) / nl / 2</td>',
+ 'has <del>bar</del>' => '>bar</del>',
+ 'has <td>description / en</td>' =>
'>(wikibase-diffview-description) / en</td>',
+ 'has <del>ohi there</del>' => '>ohi there</del>',
);
$redirectDiff = new EntityContentDiff( new EntityDiff(), new
Diff( array(
@@ -70,12 +75,11 @@
), true ) );
$redirectTags = array(
- 'has <td>redirect</td>' => array( 'tag' => 'td',
'content' => 'redirect' ),
- 'has <ins>Q1234</ins>' => array( 'tag' => 'ins',
'content' => 'Q1234' ),
+ 'has <td>redirect</td>' => '>redirect</td>',
+ 'has <ins>Q1234</ins>' => '>Q1234</ins>',
);
return array(
- 'empty' => array( $emptyDiff, array( 'empty' => '/^$/',
) ),
'fingerprint changed' => array( $fingerprintDiff,
$fingerprintTags ),
'redirect changed' => array( $redirectDiff,
$redirectTags ),
);
@@ -84,13 +88,14 @@
/**
* @return IContextSource
*/
- protected function getMockContext() {
- $en = Language::factory( 'en' );
-
+ private function getMockContext() {
$mock = $this->getMock( IContextSource::class );
+
$mock->expects( $this->any() )
- ->method( 'getLanguage' )
- ->will( $this->returnValue( $en ) );
+ ->method( 'msg' )
+ ->will( $this->returnCallback( function ( $key ) {
+ return new RawMessage( "($key)" );
+ } ) );
return $mock;
}
@@ -98,7 +103,7 @@
/**
* @return ClaimDiffer
*/
- protected function getMockClaimDiffer() {
+ private function getMockClaimDiffer() {
$mock = $this->getMockBuilder( ClaimDiffer::class )
->disableOriginalConstructor()
->getMock();
@@ -108,7 +113,7 @@
/**
* @return ClaimDifferenceVisualizer
*/
- protected function getMockClaimDiffVisualizer() {
+ private function getMockClaimDiffVisualizer() {
$mock = $this->getMockBuilder( ClaimDifferenceVisualizer::class
)
->disableOriginalConstructor()
->getMock();
@@ -118,7 +123,7 @@
/**
* @return EntityDiffVisualizer
*/
- protected function getVisualizer() {
+ private function getVisualizer() {
$enwiki = new Site();
$enwiki->setGlobalId( 'enwiki' );
@@ -135,18 +140,12 @@
* @dataProvider diffProvider
*/
public function testGenerateEntityContentDiffBody( EntityContentDiff
$diff, array $matchers ) {
- $visualizer = $this->getVisualizer();
-
- $html = $visualizer->visualizeEntityContentDiff( $diff );
+ $html = $this->getVisualizer()->visualizeEntityContentDiff(
$diff );
$this->assertInternalType( 'string', $html );
foreach ( $matchers as $name => $matcher ) {
- if ( is_string( $matcher ) ) {
- $this->assertRegExp( $matcher, $html );
- } else {
- $this->assertTag( $matcher, $html, $name );
- }
+ $this->assertContains( $matcher, $html, $name );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/330229
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie06b7cdfcee9ba1264c23a989b1ec5c8c7865711
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
