diff options
author | Gergő Tisza <tgr.huwiki@gmail.com> | 2018-07-11 11:24:07 +0200 |
---|---|---|
committer | Gergő Tisza <tgr.huwiki@gmail.com> | 2018-08-20 15:39:12 +0200 |
commit | d31580eeb0e9a88f46439301e27db273b7c63a7e (patch) | |
tree | 197f3dc98ba8f60db3ec34e572ee0c35774406d1 /tests/phpunit/includes/diff/TextSlotDiffRendererTest.php | |
parent | 47b5d95f12c9288dbeabaae46dd8a369b5797f5c (diff) | |
download | mediawikicore-d31580eeb0e9a88f46439301e27db273b7c63a7e.tar.gz mediawikicore-d31580eeb0e9a88f46439301e27db273b7c63a7e.zip |
[MCR] Render multi-slot diffs
Move logic for rendering a diff between two content objects out of
DifferenceEngine, into a new SlotDiffRenderer class. Make
DifferenceEngine use multiple SlotDiffRenderers, one per slot.
This separates the class tree for changing high-level diff properties
such as the header or the revision selection method (same as before:
subclass DifferenceEngine and override ContentHandler::getDiffEngineClass
or implement GetDifferenceEngine) and the one for changing the actual
diff rendering for a given content type (subclass SlotDiffRenderer and
override ContentHandler::getSlotDiffRenderer or implement
GetSlotDiffRenderer). To keep B/C, when SlotDiffRenderer is not overridden
for a given content type but DifferenceEngine is, that DifferenceEngine
will be used instead.
The weak point of the scheme is overriding the DifferenceEngine methods
passing control to the SlotDiffRenderers (the ones calling
getDifferenceEngines), without calling the parent. These are:
showDiffStyle, getDiffBody, getDiffBodyCacheKeyParams. Extensions doing
that will probably break in unpredictable ways (most likely, only
showing the main slot diff). Nothing in gerrit does it, at least.
A new GetSlotDiffRenderer hook is added to modify rendering for content
models not owned by the extension, much like how GetDifferenceEngine
works.
Also deprecates public access to mNewRev/mOldRev and creates public
getters instead. DifferenceEngine never supported external changes to
those properties, this just acknowledges it.
Bug: T194731
Change-Id: I2f8a9dbebd2290b7feafb20e2bb2a2693e18ba11
Depends-On: I04e885a33bfce5bccc807b9bcfe1eaa577a9fd47
Depends-On: I203d8895bf436b7fee53fe4718dede8a3b1768bc
Diffstat (limited to 'tests/phpunit/includes/diff/TextSlotDiffRendererTest.php')
-rw-r--r-- | tests/phpunit/includes/diff/TextSlotDiffRendererTest.php | 113 |
1 files changed, 113 insertions, 0 deletions
diff --git a/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php new file mode 100644 index 000000000000..ec45e29d672b --- /dev/null +++ b/tests/phpunit/includes/diff/TextSlotDiffRendererTest.php @@ -0,0 +1,113 @@ +<?php + +/** + * @covers TextSlotDiffRenderer + */ +class TextSlotDiffRendererTest extends MediaWikiTestCase { + + /** + * @dataProvider provideGetDiff + * @param Content|null $oldContent + * @param Content|null $newContent + * @param string|Exception $expectedResult + * @throws Exception + */ + public function testGetDiff( + Content $oldContent = null, Content $newContent = null, $expectedResult + ) { + if ( $expectedResult instanceof Exception ) { + $this->setExpectedException( get_class( $expectedResult ), $expectedResult->getMessage() ); + } + + $slotDiffRenderer = $this->getTextSlotDiffRenderer(); + $diff = $slotDiffRenderer->getDiff( $oldContent, $newContent ); + if ( $expectedResult instanceof Exception ) { + return; + } + $plainDiff = $this->getPlainDiff( $diff ); + $this->assertSame( $expectedResult, $plainDiff ); + } + + public function provideGetDiff() { + $this->mergeMwGlobalArrayValue( 'wgContentHandlers', [ + 'testing' => DummyContentHandlerForTesting::class, + 'testing-nontext' => DummyNonTextContentHandler::class, + ] ); + + return [ + 'same text' => [ + $this->makeContent( "aaa\nbbb\nccc" ), + $this->makeContent( "aaa\nbbb\nccc" ), + "", + ], + 'different text' => [ + $this->makeContent( "aaa\nbbb\nccc" ), + $this->makeContent( "aaa\nxxx\nccc" ), + " aaa aaa\n-bbb+xxx\n ccc ccc", + ], + 'no right content' => [ + $this->makeContent( "aaa\nbbb\nccc" ), + null, + "-aaa+ \n-bbb \n-ccc ", + ], + 'no left content' => [ + null, + $this->makeContent( "aaa\nbbb\nccc" ), + "- +aaa\n +bbb\n +ccc", + ], + 'no content' => [ + null, + null, + new InvalidArgumentException( '$oldContent and $newContent cannot both be null' ), + ], + 'non-text left content' => [ + $this->makeContent( '', 'testing-nontext' ), + $this->makeContent( "aaa\nbbb\nccc" ), + new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ), + ], + 'non-text right content' => [ + $this->makeContent( "aaa\nbbb\nccc" ), + $this->makeContent( '', 'testing-nontext' ), + new InvalidArgumentException( 'TextSlotDiffRenderer does not handle DummyNonTextContent' ), + ], + ]; + } + + // no separate test for getTextDiff() as getDiff() is just a thin wrapper around it + + /** + * @return TextSlotDiffRenderer + */ + private function getTextSlotDiffRenderer() { + $slotDiffRenderer = new TextSlotDiffRenderer(); + $slotDiffRenderer->setStatsdDataFactory( new NullStatsdDataFactory() ); + $slotDiffRenderer->setLanguage( Language::factory( 'en' ) ); + $slotDiffRenderer->setWikiDiff2MovedParagraphDetectionCutoff( 0 ); + $slotDiffRenderer->setEngine( TextSlotDiffRenderer::ENGINE_PHP ); + return $slotDiffRenderer; + } + + /** + * Convert a HTML diff to a human-readable format and hopefully make the test less fragile. + * @param string diff + * @return string + */ + private function getPlainDiff( $diff ) { + $replacements = [ + html_entity_decode( ' ' ) => ' ', + html_entity_decode( '−' ) => '-', + ]; + return str_replace( array_keys( $replacements ), array_values( $replacements ), + trim( strip_tags( $diff ), "\n" ) ); + } + + /** + * @param string $str + * @param string $model + * @return null|TextContent + */ + private function makeContent( $str, $model = CONTENT_MODEL_TEXT ) { + return ContentHandler::makeContent( $str, null, $model ); + } + +} |