From 3ea5d011312db8e9a070450b51cab8bc4c803999 Mon Sep 17 00:00:00 2001 From: "michael@butlerpc.net" Date: Thu, 27 Oct 2016 19:57:49 -0400 Subject: [PATCH 1/4] Fix issue 199, prevent empty line attributes in XML --- src/ParaTest/Logging/JUnit/TestCase.php | 3 +- .../fixtures/results/junit-example-result.xml | 36 +++++++++++++++++++ .../ParaTest/Logging/JUnit/WriterTest.php | 33 ++++++++++++++--- 3 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 test/fixtures/results/junit-example-result.xml diff --git a/src/ParaTest/Logging/JUnit/TestCase.php b/src/ParaTest/Logging/JUnit/TestCase.php index 2582d19f..36f41aa5 100644 --- a/src/ParaTest/Logging/JUnit/TestCase.php +++ b/src/ParaTest/Logging/JUnit/TestCase.php @@ -67,7 +67,8 @@ public function __construct( $this->name = $name; $this->class = $class; $this->file = $file; - $this->line = $line; + // Fix empty line numbers, for proper xml parsing + $this->line = empty($line) ? '0' : $line; $this->assertions = $assertions; $this->time = $time; } diff --git a/test/fixtures/results/junit-example-result.xml b/test/fixtures/results/junit-example-result.xml new file mode 100644 index 00000000..efcb763f --- /dev/null +++ b/test/fixtures/results/junit-example-result.xml @@ -0,0 +1,36 @@ + + + + + + + UnitTestWithClassAnnotationTest::testFalsehood + Failed asserting that true is false. + + /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithClassAnnotationTest.php:20 + + + + + + + UnitTestWithErrorTest::testTruth + Exception: Error!!! + + /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithErrorTest.php:12 + + + + + + + UnitTestWithMethodAnnotationsTest::testFalsehood + Failed asserting that true is false. + + /home/brian/Projects/parallel-phpunit/test/fixtures/tests/UnitTestWithMethodAnnotationsTest.php:18 + + + + + + diff --git a/test/unit/ParaTest/Logging/JUnit/WriterTest.php b/test/unit/ParaTest/Logging/JUnit/WriterTest.php index 139cd845..517251b5 100644 --- 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,30 @@ 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.'); + $this->assertNotEmpty(strpos($xml, 'line="0"'), 'Expected at least one (line="0"), but found none.'); } -} \ No newline at end of file +} From 6dc34d081e7c5bd1b885329c38ab3da2e46a4157 Mon Sep 17 00:00:00 2001 From: "michael@butlerpc.net" Date: Fri, 28 Oct 2016 07:53:02 -0400 Subject: [PATCH 2/4] Don't output empty line attributes --- src/ParaTest/Logging/JUnit/TestCase.php | 2 +- src/ParaTest/Logging/JUnit/Writer.php | 4 ++++ test/unit/ParaTest/Logging/JUnit/WriterTest.php | 1 - 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ParaTest/Logging/JUnit/TestCase.php b/src/ParaTest/Logging/JUnit/TestCase.php index 36f41aa5..1e0f05f2 100644 --- a/src/ParaTest/Logging/JUnit/TestCase.php +++ b/src/ParaTest/Logging/JUnit/TestCase.php @@ -68,7 +68,7 @@ public function __construct( $this->class = $class; $this->file = $file; // Fix empty line numbers, for proper xml parsing - $this->line = empty($line) ? '0' : $line; + $this->line = $line; $this->assertions = $assertions; $this->time = $time; } diff --git a/src/ParaTest/Logging/JUnit/Writer.php b/src/ParaTest/Logging/JUnit/Writer.php index 06ddffba..026233dc 100644 --- a/src/ParaTest/Logging/JUnit/Writer.php +++ b/src/ParaTest/Logging/JUnit/Writer.php @@ -133,6 +133,10 @@ protected function appendCase($suiteNode, TestCase $case) $vars = get_object_vars($case); foreach ($vars as $name => $value) { if (preg_match(static::$caseAttrs, $name)) { + if ($name === 'line' && empty($value)) { + // Don't output blank line attributes + continue; + } $caseNode->setAttribute($name, $value); } } diff --git a/test/unit/ParaTest/Logging/JUnit/WriterTest.php b/test/unit/ParaTest/Logging/JUnit/WriterTest.php index 517251b5..ce0ec152 100644 --- a/test/unit/ParaTest/Logging/JUnit/WriterTest.php +++ b/test/unit/ParaTest/Logging/JUnit/WriterTest.php @@ -73,6 +73,5 @@ public function testThatEmptyLineAttributesConvertToZero() $xml = $writer->getXml(); $this->assertFalse(strpos($xml, 'line=""'), 'Expected no empty line attributes (line=""), but found one.'); - $this->assertNotEmpty(strpos($xml, 'line="0"'), 'Expected at least one (line="0"), but found none.'); } } From f06eb1c18b291de3e4e10b9926d2b938c3177080 Mon Sep 17 00:00:00 2001 From: "michael@butlerpc.net" Date: Sat, 29 Oct 2016 13:12:32 -0400 Subject: [PATCH 3/4] Move line check to a separate function --- src/ParaTest/Logging/JUnit/Writer.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/ParaTest/Logging/JUnit/Writer.php b/src/ParaTest/Logging/JUnit/Writer.php index 026233dc..280683b4 100644 --- a/src/ParaTest/Logging/JUnit/Writer.php +++ b/src/ParaTest/Logging/JUnit/Writer.php @@ -133,8 +133,7 @@ protected function appendCase($suiteNode, TestCase $case) $vars = get_object_vars($case); foreach ($vars as $name => $value) { if (preg_match(static::$caseAttrs, $name)) { - if ($name === 'line' && empty($value)) { - // Don't output blank line attributes + if ($this->isEmptyLineAttribute($name, $value)) { continue; } $caseNode->setAttribute($name, $value); @@ -202,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); + } } From eac9d2cdea64b844828f3e07ae1e3713c49228b6 Mon Sep 17 00:00:00 2001 From: "michael@butlerpc.net" Date: Sat, 29 Oct 2016 13:13:25 -0400 Subject: [PATCH 4/4] Whoops. remove unnecessary comment --- src/ParaTest/Logging/JUnit/TestCase.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ParaTest/Logging/JUnit/TestCase.php b/src/ParaTest/Logging/JUnit/TestCase.php index 1e0f05f2..2582d19f 100644 --- a/src/ParaTest/Logging/JUnit/TestCase.php +++ b/src/ParaTest/Logging/JUnit/TestCase.php @@ -67,7 +67,6 @@ public function __construct( $this->name = $name; $this->class = $class; $this->file = $file; - // Fix empty line numbers, for proper xml parsing $this->line = $line; $this->assertions = $assertions; $this->time = $time;