Merge pull request #223 from michaelbutler/michaelbutler/skipped_fix

Fix issue 199, prevent empty line attributes in XML
paratestphp · Oct 31, 2016 · ae0a5a1 · ae0a5a1
2 parents dddcfa8 + eac9d2c
commit ae0a5a1
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 5 deletions.
diff --git a/src/ParaTest/Logging/JUnit/Writer.php b/src/ParaTest/Logging/JUnit/Writer.php
@@ -133,6 +133,9 @@ protected function appendCase($suiteNode, TestCase $case)
         $vars = get_object_vars($case);
         foreach ($vars as $name => $value) {
             if (preg_match(static::$caseAttrs, $name)) {
+                if ($this->isEmptyLineAttribute($name, $value)) {
+                    continue;
+                }
                 $caseNode->setAttribute($name, $value);
             }
         }
@@ -198,4 +201,15 @@ protected function getSuiteRootAttributes($suites)
             return $result;
         }, array_merge(array('name' => $this->name), self::$defaultSuite));
     }
+
+    /**
+     * Prevent writing empty "line" XML attributes which could break parsers.
+     * @param string $name
+     * @param mixed $value
+     * @return bool
+     */
+    private function isEmptyLineAttribute($name, $value)
+    {
+        return $name === 'line' && empty($value);
+    }
 }
diff --git a/test/fixtures/results/junit-example-result.xml b/test/fixtures/results/junit-example-result.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<testsuites>
+    <testsuite name="test/fixtures/tests/" tests="7" assertions="6" failures="2" errors="1" time="0.007625">
+        <testsuite name="UnitTestWithClassAnnotationTest" tests="3" assertions="3" failures="1" errors="0" time="0.006109" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php">
+            <testcase name="testTruth" class="UnitTestWithClassAnnotationTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php" line="10" assertions="1" time="0.001760"/>
+            <testcase name="testFalsehood" class="UnitTestWithClassAnnotationTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php" line="18" assertions="1" time="0.001195">
+                <failure type="PHPUnit_Framework_ExpectationFailedException">UnitTestWithClassAnnotationTest::testFalsehood
+                    Failed asserting that true is false.
+
+                    /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php:20
+                </failure>
+            </testcase>
+            <testcase name="testArrayLength" class="UnitTestWithClassAnnotationTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php" line="26" assertions="1" time="0.003154"/>
+        </testsuite>
+        <testsuite name="UnitTestWithErrorTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithErrorTest.php" tests="1" assertions="0" failures="0" errors="1" time="0.000397">
+            <testcase name="testTruth" class="UnitTestWithErrorTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithErrorTest.php" line="" assertions="0" time="0.000397">
+                <error type="Exception">UnitTestWithErrorTest::testTruth
+                    Exception: Error!!!
+
+                    /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithErrorTest.php:12
+                </error>
+            </testcase>
+        </testsuite>
+        <testsuite name="UnitTestWithMethodAnnotationsTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php" tests="3" assertions="3" failures="1" errors="0" time="0.001119">
+            <testcase name="testTruth" class="UnitTestWithMethodAnnotationsTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php" line="7" assertions="1" time="0.000341"/>
+            <testcase name="testFalsehood" class="UnitTestWithMethodAnnotationsTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php" line="" assertions="1" time="0.000402">
+                <failure type="PHPUnit_Framework_ExpectationFailedException">UnitTestWithMethodAnnotationsTest::testFalsehood
+                    Failed asserting that true is false.
+
+                    /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php:18
+                </failure>
+            </testcase>
+            <testcase name="testArrayLength" class="UnitTestWithMethodAnnotationsTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php" line="25" assertions="1" time="0.000376"/>
+        </testsuite>
+    </testsuite>
+</testsuites>
diff --git a/test/unit/ParaTest/Logging/JUnit/WriterTest.php b/test/unit/ParaTest/Logging/JUnit/WriterTest.php
@@ -5,19 +5,24 @@
 class WriterTest extends \TestBase
 {
     protected $writer;
+
+    /** @var LogInterpreter */
     protected $interpreter;
     protected $passing;
 
     public function setUp()
     {
         $this->interpreter = new LogInterpreter();
-        $this->writer = new Writer($this->interpreter,  "test/fixtures/tests/");
+        $this->writer = new Writer($this->interpreter, "test/fixtures/tests/");
         $this->passing = FIXTURES . DS . 'results' . DS . 'single-passing.xml';
     }
 
     public function testConstructor()
     {
-        $this->assertInstanceOf('ParaTest\\Logging\\LogInterpreter', $this->getObjectValue($this->writer, 'interpreter'));
+        $this->assertInstanceOf(
+            'ParaTest\\Logging\\LogInterpreter',
+            $this->getObjectValue($this->writer, 'interpreter')
+        );
         $this->assertEquals("test/fixtures/tests/", $this->writer->getName());
     }
 
@@ -44,12 +49,29 @@ public function testWrite()
         $this->addPassingReader();
         $this->writer->write($output);
         $this->assertXmlStringEqualsXmlString(file_get_contents($this->passing), file_get_contents($output));
-        if(file_exists($output)) unlink($output);
+        if (file_exists($output)) {
+            unlink($output);
+        }
     }
 
     protected function addPassingReader()
     {
         $reader = new Reader($this->passing);
-        $this->interpreter->addReader($reader);   
+        $this->interpreter->addReader($reader);
+    }
+
+    /**
+     * Empty line attributes, e.g. line="" breaks Jenkins parsing since it needs to be an integer.
+     * To repair, ensure that empty line attributes are actually written as 0 instead of empty string.
+     */
+    public function testThatEmptyLineAttributesConvertToZero()
+    {
+        $mixed = FIXTURES . DS . 'results' . DS . 'junit-example-result.xml';
+        $reader = new Reader($mixed);
+        $this->interpreter->addReader($reader);
+        $writer = new Writer($this->interpreter, "test/fixtures/tests/");
+        $xml = $writer->getXml();
+
+        $this->assertFalse(strpos($xml, 'line=""'), 'Expected no empty line attributes (line=""), but found one.');
     }
-}
+}