From: addshore <addshorewiki@gmail.com>
Date: Wed, 26 Feb 2014 21:47:40 +0000 (+0100)
Subject: Add tests for ArrayDiffFormatter and DiffOp
X-Git-Tag: 1.31.0-rc.0~16810^2
X-Git-Url: http://git.cyclocoop.org/%22.%24h.%22?a=commitdiff_plain;h=3d1fba6e726fb9e977a23e1c4e080ccbbd9fd51d;p=lhc%2Fweb%2Fwiklou.git

Add tests for ArrayDiffFormatter and DiffOp

This change also adds some tested getters to
the DiffOp class to improve testability of the class

Change-Id: I26eafd791b9892f565a18d0502474ef9c24f9a29
---

diff --git a/includes/diff/ArrayDiffFormatter.php b/includes/diff/ArrayDiffFormatter.php
index 331ce7d160..543ee180e3 100644
--- a/includes/diff/ArrayDiffFormatter.php
+++ b/includes/diff/ArrayDiffFormatter.php
@@ -29,48 +29,49 @@
  * @ingroup DifferenceEngine
  */
 class ArrayDiffFormatter extends DiffFormatter {
+
 	/**
-	 * @param $diff
+	 * @param Diff $diff A Diff object.
 	 * @return array
 	 */
 	public function format( $diff ) {
 		$oldline = 1;
 		$newline = 1;
 		$retval = array();
-		foreach ( $diff->edits as $edit ) {
-			switch ( $edit->type ) {
+		foreach ( $diff->getEdits() as $edit ) {
+			switch ( $edit->getType() ) {
 				case 'add':
-					foreach ( $edit->closing as $l ) {
+					foreach ( $edit->getClosing() as $line ) {
 						$retval[] = array(
 							'action' => 'add',
-							'new' => $l,
+							'new' => $line,
 							'newline' => $newline++
 						);
 					}
 					break;
 				case 'delete':
-					foreach ( $edit->orig as $l ) {
+					foreach ( $edit->getOrig() as $line ) {
 						$retval[] = array(
 							'action' => 'delete',
-							'old' => $l,
+							'old' => $line,
 							'oldline' => $oldline++,
 						);
 					}
 					break;
 				case 'change':
-					foreach ( $edit->orig as $i => $l ) {
+					foreach ( $edit->getOrig() as $key => $line ) {
 						$retval[] = array(
 							'action' => 'change',
-							'old' => $l,
-							'new' => isset( $edit->closing[$i] ) ? $edit->closing[$i] : null,
+							'old' => $line,
+							'new' => $edit->getClosing( $key ),
 							'oldline' => $oldline++,
 							'newline' => $newline++,
 						);
 					}
 					break;
 				case 'copy':
-					$oldline += count( $edit->orig );
-					$newline += count( $edit->orig );
+					$oldline += count( $edit->getOrig() );
+					$newline += count( $edit->getOrig() );
 			}
 		}
 
diff --git a/includes/diff/DairikiDiff.php b/includes/diff/DairikiDiff.php
index c47ecedb79..351a9dd6e9 100644
--- a/includes/diff/DairikiDiff.php
+++ b/includes/diff/DairikiDiff.php
@@ -31,23 +31,42 @@
  * @ingroup DifferenceEngine
  */
 abstract class DiffOp {
+
 	public $type;
 	public $orig;
 	public $closing;
 
+	public function getType() {
+		return $this->type;
+	}
+
+	public function getOrig() {
+		return $this->orig;
+	}
+
+	public function getClosing( $i = null ) {
+		if( $i === null ) {
+			return $this->closing;
+		}
+		if( array_key_exists( $i, $this->closing ) ) {
+			return $this->closing[$i];
+		}
+		return null;
+	}
+
 	abstract public function reverse();
 
 	/**
 	 * @return int
 	 */
-	function norig() {
+	public function norig() {
 		return $this->orig ? count( $this->orig ) : 0;
 	}
 
 	/**
 	 * @return int
 	 */
-	function nclosing() {
+	public function nclosing() {
 		return $this->closing ? count( $this->closing ) : 0;
 	}
 }
@@ -60,7 +79,7 @@ abstract class DiffOp {
 class DiffOpCopy extends DiffOp {
 	public $type = 'copy';
 
-	function __construct( $orig, $closing = false ) {
+	public function __construct( $orig, $closing = false ) {
 		if ( !is_array( $closing ) ) {
 			$closing = $orig;
 		}
@@ -71,7 +90,7 @@ class DiffOpCopy extends DiffOp {
 	/**
 	 * @return DiffOpCopy
 	 */
-	function reverse() {
+	public function reverse() {
 		return new DiffOpCopy( $this->closing, $this->orig );
 	}
 }
@@ -84,7 +103,7 @@ class DiffOpCopy extends DiffOp {
 class DiffOpDelete extends DiffOp {
 	public $type = 'delete';
 
-	function __construct( $lines ) {
+	public function __construct( $lines ) {
 		$this->orig = $lines;
 		$this->closing = false;
 	}
@@ -92,7 +111,7 @@ class DiffOpDelete extends DiffOp {
 	/**
 	 * @return DiffOpAdd
 	 */
-	function reverse() {
+	public function reverse() {
 		return new DiffOpAdd( $this->orig );
 	}
 }
@@ -105,7 +124,7 @@ class DiffOpDelete extends DiffOp {
 class DiffOpAdd extends DiffOp {
 	public $type = 'add';
 
-	function __construct( $lines ) {
+	public function __construct( $lines ) {
 		$this->closing = $lines;
 		$this->orig = false;
 	}
@@ -113,7 +132,7 @@ class DiffOpAdd extends DiffOp {
 	/**
 	 * @return DiffOpDelete
 	 */
-	function reverse() {
+	public function reverse() {
 		return new DiffOpDelete( $this->closing );
 	}
 }
@@ -126,7 +145,7 @@ class DiffOpAdd extends DiffOp {
 class DiffOpChange extends DiffOp {
 	public $type = 'change';
 
-	function __construct( $orig, $closing ) {
+	public function __construct( $orig, $closing ) {
 		$this->orig = $orig;
 		$this->closing = $closing;
 	}
@@ -134,7 +153,7 @@ class DiffOpChange extends DiffOp {
 	/**
 	 * @return DiffOpChange
 	 */
-	function reverse() {
+	public function reverse() {
 		return new DiffOpChange( $this->closing, $this->orig );
 	}
 }
@@ -180,7 +199,7 @@ class DiffEngine {
 	 * @param $to_lines
 	 * @return array
 	 */
-	function diff( $from_lines, $to_lines ) {
+	public function diff( $from_lines, $to_lines ) {
 		wfProfileIn( __METHOD__ );
 
 		// Diff and store locally
@@ -659,6 +678,10 @@ class DiffEngine {
  * @ingroup DifferenceEngine
  */
 class Diff {
+
+	/**
+	 * @var DiffOp[]
+	 */
 	public $edits;
 
 	/**
@@ -669,11 +692,18 @@ class Diff {
 	 *   Typically these are lines from a file.
 	 * @param $to_lines array An array of strings.
 	 */
-	function __construct( $from_lines, $to_lines ) {
+	public function __construct( $from_lines, $to_lines ) {
 		$eng = new DiffEngine;
 		$this->edits = $eng->diff( $from_lines, $to_lines );
 	}
 
+	/**
+	 * @return array|DiffOp[]
+	 */
+	public function getEdits() {
+		return $this->edits;
+	}
+
 	/**
 	 * Compute reversed Diff.
 	 *
@@ -684,7 +714,7 @@ class Diff {
 	 * @return Object A Diff object representing the inverse of the
 	 *   original diff.
 	 */
-	function reverse() {
+	public function reverse() {
 		$rev = $this;
 		$rev->edits = array();
 		/** @var DiffOp $edit */
@@ -700,7 +730,7 @@ class Diff {
 	 *
 	 * @return bool True if two sequences were identical.
 	 */
-	function isEmpty() {
+	public function isEmpty() {
 		foreach ( $this->edits as $edit ) {
 			if ( $edit->type != 'copy' ) {
 				return false;
@@ -717,7 +747,7 @@ class Diff {
 	 *
 	 * @return int The length of the LCS.
 	 */
-	function lcs() {
+	public function lcs() {
 		$lcs = 0;
 		foreach ( $this->edits as $edit ) {
 			if ( $edit->type == 'copy' ) {
@@ -736,7 +766,7 @@ class Diff {
 	 *
 	 * @return array The original sequence of strings.
 	 */
-	function orig() {
+	public function orig() {
 		$lines = array();
 
 		foreach ( $this->edits as $edit ) {
@@ -756,7 +786,7 @@ class Diff {
 	 *
 	 * @return array The sequence of strings.
 	 */
-	function closing() {
+	public function closing() {
 		$lines = array();
 
 		foreach ( $this->edits as $edit ) {
@@ -798,7 +828,7 @@ class MappedDiff extends Diff {
 	 * @param $mapped_to_lines array This array should
 	 *   have the same number of elements as $to_lines.
 	 */
-	function __construct( $from_lines, $to_lines,
+	public function __construct( $from_lines, $to_lines,
 		$mapped_from_lines, $mapped_to_lines ) {
 		wfProfileIn( __METHOD__ );
 
@@ -922,7 +952,7 @@ class WordLevelDiff extends MappedDiff {
 	 * @param $orig_lines
 	 * @param $closing_lines
 	 */
-	function __construct( $orig_lines, $closing_lines ) {
+	public function __construct( $orig_lines, $closing_lines ) {
 		wfProfileIn( __METHOD__ );
 
 		list( $orig_words, $orig_stripped ) = $this->split( $orig_lines );
diff --git a/tests/phpunit/includes/diff/ArrayDiffFormatterTest.php b/tests/phpunit/includes/diff/ArrayDiffFormatterTest.php
new file mode 100644
index 0000000000..50c5c572e4
--- /dev/null
+++ b/tests/phpunit/includes/diff/ArrayDiffFormatterTest.php
@@ -0,0 +1,116 @@
+<?php
+
+/**
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ *
+ * @group Diff
+ */
+class ArrayDiffFormatterTest extends MediaWikiTestCase {
+
+	/**
+	 * @param Diff $input
+	 * @param array $expectedOutput
+	 * @dataProvider provideTestFormat
+	 * @covers ArrayDiffFormatter::format
+	 */
+	public function testFormat( $input, $expectedOutput ) {
+		$instance = new ArrayDiffFormatter();
+		$output = $instance->format( $input );
+		$this->assertEquals( $expectedOutput, $output );
+	}
+
+	private function getMockDiff( $edits ) {
+		$diff = $this->getMockBuilder( 'Diff' )
+			->disableOriginalConstructor()
+			->getMock();
+		$diff->expects( $this->any() )
+			->method( 'getEdits' )
+			->will( $this->returnValue( $edits ) );
+		return $diff;
+	}
+
+	private function getMockDiffOp( $type = null, $orig = array(), $closing = array() ) {
+		$diffOp = $this->getMockBuilder( 'DiffOp' )
+			->disableOriginalConstructor()
+			->getMock();
+		$diffOp->expects( $this->any() )
+			->method( 'getType' )
+			->will( $this->returnValue( $type ) );
+		$diffOp->expects( $this->any() )
+			->method( 'getOrig' )
+			->will( $this->returnValue( $orig ) );
+		if( $type === 'change' ) {
+			$diffOp->expects( $this->any() )
+				->method( 'getClosing' )
+				->with( $this->isType( 'integer' ) )
+				->will( $this->returnCallback( function() {
+					return 'mockLine';
+				} ) );
+		} else {
+			$diffOp->expects( $this->any() )
+				->method( 'getClosing' )
+				->will( $this->returnValue( $closing ) );
+		}
+		return $diffOp;
+	}
+
+	public function provideTestFormat() {
+		$emptyArrayTestCases = array(
+			$this->getMockDiff( array() ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'add' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'delete' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'change' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'copy' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'FOOBARBAZ' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'add', 'line' ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'delete', array(), array( 'line' ) ) ) ),
+			$this->getMockDiff( array( $this->getMockDiffOp( 'copy', array(), array( 'line' ) ) ) ),
+		);
+
+		$otherTestCases = array();
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'add', array( ), array( 'a1' ) ) ) ),
+			array( array( 'action' => 'add', 'new' => 'a1', 'newline' => 1 ) ),
+		);
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'add', array( ), array( 'a1', 'a2' ) ) ) ),
+			array(
+				array( 'action' => 'add', 'new' => 'a1', 'newline' => 1 ),
+				array( 'action' => 'add', 'new' => 'a2', 'newline' => 2 ),
+			),
+		);
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'delete', array( 'd1' ) ) ) ),
+			array( array( 'action' => 'delete', 'old' => 'd1', 'oldline' => 1 ) ),
+		);
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'delete', array( 'd1', 'd2' ) ) ) ),
+			array(
+				array( 'action' => 'delete', 'old' => 'd1', 'oldline' => 1 ),
+				array( 'action' => 'delete', 'old' => 'd2', 'oldline' => 2 ),
+			),
+		);
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'change', array( 'd1' ), array( 'a1' ) ) ) ),
+			array( array( 'action' => 'change', 'old' => 'd1', 'new' => 'mockLine', 'newline' => 1, 'oldline' => 1 ) ),
+		);
+		$otherTestCases[] = array(
+			$this->getMockDiff( array( $this->getMockDiffOp( 'change', array( 'd1', 'd2' ), array( 'a1', 'a2' ) ) ) ),
+			array(
+				array( 'action' => 'change', 'old' => 'd1', 'new' => 'mockLine', 'newline' => 1, 'oldline' => 1 ),
+				array( 'action' => 'change', 'old' => 'd2', 'new' => 'mockLine', 'newline' => 2, 'oldline' => 2 ),
+			),
+		);
+
+		$testCases = array();
+		foreach( $emptyArrayTestCases as $testCase ) {
+			$testCases[] = array( $testCase, array() );
+		}
+		foreach( $otherTestCases as $testCase ) {
+			$testCases[] = array( $testCase[0], $testCase[1] );
+		}
+		return $testCases;
+	}
+
+}
diff --git a/tests/phpunit/includes/diff/DiffOpTest.php b/tests/phpunit/includes/diff/DiffOpTest.php
new file mode 100644
index 0000000000..fe2c566fa0
--- /dev/null
+++ b/tests/phpunit/includes/diff/DiffOpTest.php
@@ -0,0 +1,73 @@
+<?php
+
+//Load our FakeDiffOp
+require_once( __DIR__ . DIRECTORY_SEPARATOR . 'FakeDiffOp.php' );
+
+/**
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ *
+ * @group Diff
+ */
+class DiffOpTest extends MediaWikiTestCase {
+
+	/**
+	 * @covers DiffOp::getType
+	 */
+	public function testGetType() {
+		$obj = new FakeDiffOp();
+		$obj->type = 'foo';
+		$this->assertEquals( 'foo', $obj->getType() );
+	}
+
+	/**
+	 * @covers DiffOp::getOrig
+	 */
+	public function testGetOrig() {
+		$obj = new FakeDiffOp();
+		$obj->orig = array( 'foo' );
+		$this->assertEquals( array( 'foo' ), $obj->getOrig() );
+	}
+
+	/**
+	 * @covers DiffOp::getClosing
+	 */
+	public function testGetClosing() {
+		$obj = new FakeDiffOp();
+		$obj->closing = array( 'foo' );
+		$this->assertEquals( array( 'foo' ), $obj->getClosing() );
+	}
+
+	/**
+	 * @covers DiffOp::getClosing
+	 */
+	public function testGetClosingWithParameter() {
+		$obj = new FakeDiffOp();
+		$obj->closing = array( 'foo', 'bar', 'baz' );
+		$this->assertEquals( 'foo' , $obj->getClosing( 0 ) );
+		$this->assertEquals( 'bar' , $obj->getClosing( 1 ) );
+		$this->assertEquals( 'baz' , $obj->getClosing( 2 ) );
+		$this->assertEquals( null , $obj->getClosing( 3 ) );
+	}
+
+	/**
+	 * @covers DiffOp::norig
+	 */
+	public function testNorig() {
+		$obj = new FakeDiffOp();
+		$this->assertEquals( 0, $obj->norig() );
+		$obj->orig = array( 'foo' );
+		$this->assertEquals( 1, $obj->norig() );
+	}
+
+	/**
+	 * @covers DiffOp::nclosing
+	 */
+	public function testNclosing() {
+		$obj = new FakeDiffOp();
+		$this->assertEquals( 0, $obj->nclosing() );
+		$obj->closing = array( 'foo' );
+		$this->assertEquals( 1, $obj->nclosing() );
+	}
+
+}
diff --git a/tests/phpunit/includes/diff/DiffTest.php b/tests/phpunit/includes/diff/DiffTest.php
new file mode 100644
index 0000000000..1911c82aae
--- /dev/null
+++ b/tests/phpunit/includes/diff/DiffTest.php
@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * @licence GNU GPL v2+
+ * @author Adam Shorland
+ *
+ * @group Diff
+ */
+class DiffTest extends MediaWikiTestCase {
+
+	/**
+	 * @covers Diff::getEdits
+	 */
+	public function testGetEdits() {
+		$obj = new Diff( array(), array() );
+		$obj->edits = 'FooBarBaz';
+		$this->assertEquals( 'FooBarBaz', $obj->getEdits() );
+	}
+
+}
diff --git a/tests/phpunit/includes/diff/DifferenceEngineTest.php b/tests/phpunit/includes/diff/DifferenceEngineTest.php
index f95eb5e8f6..e1a69e3bc5 100644
--- a/tests/phpunit/includes/diff/DifferenceEngineTest.php
+++ b/tests/phpunit/includes/diff/DifferenceEngineTest.php
@@ -6,6 +6,7 @@
  * @todo tests for the rest of DifferenceEngine!
  *
  * @group Database
+ * @group Diff
  *
  * @licence GNU GPL v2+
  * @author Katie Filbert < aude.wiki@gmail.com >
diff --git a/tests/phpunit/includes/diff/FakeDiffOp.php b/tests/phpunit/includes/diff/FakeDiffOp.php
new file mode 100644
index 0000000000..70c8f64a57
--- /dev/null
+++ b/tests/phpunit/includes/diff/FakeDiffOp.php
@@ -0,0 +1,11 @@
+<?php
+
+/**
+ * Class FakeDiffOp used to test abstract class DiffOp
+ */
+class FakeDiffOp extends DiffOp {
+
+	public function reverse() {
+		return null;
+	}
+}