Add tests for ArrayDiffFormatter and DiffOp
authoraddshore <addshorewiki@gmail.com>
Wed, 26 Feb 2014 21:47:40 +0000 (22:47 +0100)
committeraddshore <addshorewiki@gmail.com>
Wed, 26 Feb 2014 21:53:05 +0000 (22:53 +0100)
This change also adds some tested getters to
the DiffOp class to improve testability of the class

Change-Id: I26eafd791b9892f565a18d0502474ef9c24f9a29

includes/diff/ArrayDiffFormatter.php
includes/diff/DairikiDiff.php
tests/phpunit/includes/diff/ArrayDiffFormatterTest.php [new file with mode: 0644]
tests/phpunit/includes/diff/DiffOpTest.php [new file with mode: 0644]
tests/phpunit/includes/diff/DiffTest.php [new file with mode: 0644]
tests/phpunit/includes/diff/DifferenceEngineTest.php
tests/phpunit/includes/diff/FakeDiffOp.php [new file with mode: 0644]

index 331ce7d..543ee18 100644 (file)
  * @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() );
                        }
                }
 
index c47eced..351a9dd 100644 (file)
  * @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 (file)
index 0000000..50c5c57
--- /dev/null
@@ -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 (file)
index 0000000..fe2c566
--- /dev/null
@@ -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 (file)
index 0000000..1911c82
--- /dev/null
@@ -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() );
+       }
+
+}
index f95eb5e..e1a69e3 100644 (file)
@@ -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 (file)
index 0000000..70c8f64
--- /dev/null
@@ -0,0 +1,11 @@
+<?php
+
+/**
+ * Class FakeDiffOp used to test abstract class DiffOp
+ */
+class FakeDiffOp extends DiffOp {
+
+       public function reverse() {
+               return null;
+       }
+}