Skip to content

Commit

Permalink
Fix JUnit merge issues (#729)
Browse files Browse the repository at this point in the history
  • Loading branch information
Slamdunk authored Feb 9, 2023
1 parent 20fa234 commit 3f5a62a
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 111 deletions.
6 changes: 3 additions & 3 deletions src/JUnit/LogMerger.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

use function array_merge;
use function assert;
use function ksort;

/**
* @internal
Expand Down Expand Up @@ -39,7 +40,6 @@ public function merge(array $junitFiles): TestSuite
$mainSuite->assertions,
$mainSuite->failures,
$mainSuite->errors,
$mainSuite->risky,
$mainSuite->skipped,
$mainSuite->time,
'',
Expand All @@ -55,7 +55,6 @@ public function merge(array $junitFiles): TestSuite
$otherSuite->assertions,
$otherSuite->failures,
$otherSuite->errors,
$otherSuite->risky,
$otherSuite->skipped,
$otherSuite->time,
'',
Expand Down Expand Up @@ -90,13 +89,14 @@ private function mergeSuites(TestSuite $suite1, TestSuite $suite2): TestSuite
);
}

ksort($suites);

return new TestSuite(
$suite1->name,
$suite1->tests + $suite2->tests,
$suite1->assertions + $suite2->assertions,
$suite1->failures + $suite2->failures,
$suite1->errors + $suite2->errors,
$suite1->risky + $suite2->risky,
$suite1->skipped + $suite2->skipped,
$suite1->time + $suite2->time,
$suite1->file,
Expand Down
3 changes: 1 addition & 2 deletions src/JUnit/MessageType.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ enum MessageType
{
case error;
case failure;
case risky;
case skipped;

public function toString(): string
{
return match ($this) {
self::error, self::risky => 'error',
self::error => 'error',
self::failure => 'failure',
self::skipped => 'skipped',
};
Expand Down
25 changes: 0 additions & 25 deletions src/JUnit/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ public function __construct(

final public static function caseFromNode(SimpleXMLElement $node): self
{
$systemOutput = null;
$systemOutputs = $node->xpath('system-out');
if ($systemOutputs !== null && $systemOutputs !== []) {
assert(count($systemOutputs) === 1);
$systemOutput = (string) current($systemOutputs);
}

$getFirstNode = static function (array $nodes): SimpleXMLElement {
assert(count($nodes) === 1);
$node = current($nodes);
Expand All @@ -59,21 +52,6 @@ final public static function caseFromNode(SimpleXMLElement $node): self
$type = $getType($error);
$text = (string) $error;

if ($type === 'PHPUnit\\Framework\\RiskyTest') {
return new TestCaseWithMessage(
(string) $node['name'],
(string) $node['class'],
(string) $node['file'],
(int) $node['line'],
(int) $node['assertions'],
(float) $node['time'],
$type,
$text,
$systemOutput,
MessageType::risky,
);
}

return new TestCaseWithMessage(
(string) $node['name'],
(string) $node['class'],
Expand All @@ -83,7 +61,6 @@ final public static function caseFromNode(SimpleXMLElement $node): self
(float) $node['time'],
$type,
$text,
$systemOutput,
MessageType::error,
);
}
Expand All @@ -102,7 +79,6 @@ final public static function caseFromNode(SimpleXMLElement $node): self
(float) $node['time'],
$type,
$text,
$systemOutput,
MessageType::failure,
);
}
Expand All @@ -128,7 +104,6 @@ final public static function caseFromNode(SimpleXMLElement $node): self
(float) $node['time'],
null,
$text,
$systemOutput,
MessageType::skipped,
);
}
Expand Down
1 change: 0 additions & 1 deletion src/JUnit/TestCaseWithMessage.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public function __construct(
float $time,
public readonly ?string $type,
public readonly string $text,
public readonly ?string $systemOutput,
public readonly MessageType $xmlTagName
) {
parent::__construct($name, $class, $file, $line, $assertions, $time);
Expand Down
72 changes: 39 additions & 33 deletions src/JUnit/TestSuite.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@
use SimpleXMLElement;
use SplFileInfo;

use function array_map;
use function array_sum;
use function assert;
use function count;
use function file_get_contents;

/**
Expand All @@ -19,11 +18,6 @@
*/
final class TestSuite
{
/** @var array<string, TestSuite> */
public readonly array $suites;
/** @var TestCase */
public readonly array $cases;

/**
* @param array<string, TestSuite> $suites
* @param list<TestCase> $cases
Expand All @@ -34,15 +28,12 @@ public function __construct(
public readonly int $assertions,
public readonly int $failures,
public readonly int $errors,
public readonly int $risky,
public readonly int $skipped,
public readonly float $time,
public readonly string $file,
array $suites,
array $cases
public readonly array $suites,
public readonly array $cases
) {
$this->suites = $suites;
$this->cases = $cases;
}

public static function fromFile(SplFileInfo $logFile): self
Expand All @@ -61,41 +52,56 @@ public static function fromFile(SplFileInfo $logFile): self
private static function parseTestSuite(SimpleXMLElement $node, bool $isRootSuite): self
{
if ($isRootSuite) {
foreach ($node->testsuite as $singleTestSuiteXml) {
return self::parseTestSuite($singleTestSuiteXml, false);
}
$tests = 0;
$assertions = 0;
$failures = 0;
$errors = 0;
$skipped = 0;
$time = 0;
} else {
$tests = (int) $node['tests'];
$assertions = (int) $node['assertions'];
$failures = (int) $node['failures'];
$errors = (int) $node['errors'];
$skipped = (int) $node['skipped'];
$time = (float) $node['time'];
}

$count = count($node->testsuite);
$suites = [];
foreach ($node->testsuite as $singleTestSuiteXml) {
$testSuite = self::parseTestSuite($singleTestSuiteXml, false);
$testSuite = self::parseTestSuite($singleTestSuiteXml, false);
if ($isRootSuite && $count === 1) {
return $testSuite;
}

$suites[$testSuite->name] = $testSuite;

if (! $isRootSuite) {
continue;
}

$tests += $testSuite->tests;
$assertions += $testSuite->assertions;
$failures += $testSuite->failures;
$errors += $testSuite->errors;
$skipped += $testSuite->skipped;
$time += $testSuite->time;
}

$cases = [];
foreach ($node->testcase as $singleTestCase) {
$cases[] = TestCase::caseFromNode($singleTestCase);
}

$risky = array_sum(array_map(static function (TestCase $testCase): int {
return (int) (
$testCase instanceof TestCaseWithMessage
&& $testCase->xmlTagName === MessageType::risky
);
}, $cases));
$risky += array_sum(array_map(static function (TestSuite $testSuite): int {
return $testSuite->risky;
}, $suites));

return new self(
(string) $node['name'],
(int) $node['tests'],
(int) $node['assertions'],
(int) $node['failures'],
(int) $node['errors'] - $risky,
$risky,
(int) $node['skipped'],
(float) $node['time'],
$tests,
$assertions,
$failures,
$errors,
$skipped,
$time,
(string) $node['file'],
$suites,
$cases,
Expand Down
2 changes: 1 addition & 1 deletion src/JUnit/Writer.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private function createSuiteNode(TestSuite $parentSuite): DOMElement

$suiteNode->setAttribute('tests', (string) $parentSuite->tests);
$suiteNode->setAttribute('assertions', (string) $parentSuite->assertions);
$suiteNode->setAttribute('errors', (string) ($parentSuite->errors + $parentSuite->risky));
$suiteNode->setAttribute('errors', (string) $parentSuite->errors);
$suiteNode->setAttribute('failures', (string) $parentSuite->failures);
$suiteNode->setAttribute('skipped', (string) $parentSuite->skipped);
$suiteNode->setAttribute('time', (string) $parentSuite->time);
Expand Down
Loading

0 comments on commit 3f5a62a

Please sign in to comment.