Pest: `junit` report is broken

Created on 30 May 2020  ·  11Comments  ·  Source: pestphp/pest

How to reproduce:

Source class:

# src/SourceFile.php

namespace PestExample;

final class SourceFile
{
    public function add(int $a, int $b): int
    {
        return $a + $b;
    }
}

test file:

# tests/SourceFileTest.php

use PestExample\SourceFile;

it('can add two integers', function (): void {
    $source = new SourceFile();

    $result = $source->add(3, 2);

    $this->assertGreaterThan(0, $result);
});

Command line:

vendor/bin/pest --coverage-xml=pest-coverage-xml --log-junit="pest-coverage-xml/junit.xml"

generated files:

tree pest-coverage-xml

pest-coverage-xml
├── SourceFile.php.xml
├── index.xml
└── junit.xml

And the incorrect junit.xml report looks like:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.006614">
    <testsuite name="P\Tests\SourceFileTest" file="/pest-example/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(185) : eval()'d code" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.006614">
      <testcase name="it can add two integers" assertions="1" time="0.006614"/>
    </testsuite>
  </testsuite>
</testsuites>

What's wrong here? look at the following line:

<testsuite name="P\Tests\SourceFileTest" file="pest-example/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(185) : eval()'d code" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.006614">

and in particular look at the file attribute:

file="/pest-example/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(185) : eval()'d code"

It's not a valid test file.


❗❗

Expected: /pest-example/tests/SourceFileTest.php
Actual: /pest-example/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(185) : eval()'d code

If I run the same test but written in PHPUnit style, I get the correct junit report:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.007011">
    <testsuite name="Test Suite" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.007011">
      <testsuite name="PestExample\Test\PhpUnitSourceFileTest" file="/pest-example/tests/PhpUnitSourceFileTest.php" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.007011">
        <testcase name="test_it_can_add_two_integers" class="PestExample\Test\PhpUnitSourceFileTest" classname="PestExample.Test.PhpUnitSourceFileTest" file="/pest-example/tests/PhpUnitSourceFileTest.php" line="14" assertions="1" time="0.007011"/>
      </testsuite>
    </testsuite>
  </testsuite>
</testsuites>

Related to and blocks https://github.com/pestphp/pest/issues/4

bug

All 11 comments

That sounds normal to me.

Behind the scenes pest creates real classes to make the integration seamless with PHPUnit.

Not sure if we plan to fix this.

Behind the scenes pest creates real classes to make the integration seamless with PHPUnit.

That's perfectly fine, but let me disagree with you that the current junit report "sounds normal".

So the purpose of junit report is to

  1. give the information what class is used
  2. give the information what file is used
  3. give the information what test cases are used (methods in plain PHPUnit, or test() / it() blocks in Pest)
  4. give the information about _metrics_, the most important ones are the time that test took to execute and the line of covered code

However, we see that the file attribute has invalid values:

/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(185) : eval()'d code

Two issues here

  1. it's not a real file of executed test (SourceFileTest.php in our example)
  2. it's not a valid path to file (note the (185) : eval()'d code postfix)

And now, let me describe the importance of the junit report for Infection.

Imagine, you have 1000 tests, and only 2 of them covers line 33 that is currently being mutated by Infection.

Infection does its best to run only those tests that covers the mutated line (line 33 in our example). All other 998 tests are not executed.

And it does that thanks to the information fetched from junit report. Without it, all 1000 tests will be executed for each mutation, which leads to completely unusable mutation testing technic for the real projects.

Do you have any ideas how we can improve that?

Got it.

Without junit report, no infection adapter right?

Well, we _can_ do it, but it will be barely useful. The worst thing is the time.

Another example:

  • without junit: if we have 500 mutations and 1000 tests, it will be 500*1000=500,000 executed tests in the worst scenario
  • with junit: if we have 500 mutations and 1000 tests, and each line is covered by 2-3 tests, we will have 500*3=1500 executed tests in the worst scenario, so it's 333 times better :)

Let me know if I can help with anything here.

I will take a better look at this tomorrow.

Just pushed a very native solution for this: https://github.com/pestphp/pest/tree/feat/poc-junit.

Can you try it out and tell me if something is missing?

It's much better! But still not enough.

Currently, it generates such junit report:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
    <testsuite name="P\Tests\SourceFileTest" file="/pest/src/Factories/TestCaseFactory.php(187) : eval()'d code" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
      <testcase name="it can add two integers" class="Tests\SourceFileTest" classname="Tests.SourceFileTest" file="/pest-example/tests/SourceFileTest.php" assertions="1" time="0.022643"/>
    </testsuite>
  </testsuite>
</testsuites>

where <testcase node contains the correct file=/pest-example/tests/SourceFileTest.php, but with incorrect FQCN Tests\SourceFileTest

but node <testsuite contains correct name="P\Tests\SourceFileTest" but incorrect file file="/pest/src/Factories/TestCaseFactory.php(187) : eval()'d code"

Can we somehow have this:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
-    <testsuite name="P\Tests\SourceFileTest" file="/pest/src/Factories/TestCaseFactory.php(187) : eval()'d code" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
+    <testsuite name="P\Tests\SourceFileTest" file="/pest-example/tests/SourceFileTest.php" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
      <testcase name="it can add two integers" class="Tests\SourceFileTest" classname="Tests.SourceFileTest" file="/pest-example/tests/SourceFileTest.php" assertions="1" time="0.022643"/>
    </testsuite>
  </testsuite>
</testsuites>

or at least this

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
    <testsuite name="P\Tests\SourceFileTest" file="/pest/src/Factories/TestCaseFactory.php(187) : eval()'d code" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.022643">
-      <testcase name="it can add two integers" class="Tests\SourceFileTest" classname="Tests.SourceFileTest" file="/pest-example/tests/SourceFileTest.php" assertions="1" time="0.022643"/>
+      <testcase name="it can add two integers" class="P\Tests\SourceFileTest" classname="Tests.SourceFileTest" file="/pest-example/tests/SourceFileTest.php" assertions="1" time="0.022643"/>

    </testsuite>
  </testsuite>
</testsuites>

In other words, we need to have a node where we both have P\Test\SourceFileTest as FQCL and /pest-example/tests/SourceFileTest.php as a file

Explanation:

  • P\Test\SourceFileTest is used in coverage-xml report as a FQCN, but not Test\SourceFileTest (not the P\ prefix, I guess this is what Pest adds)

@nunomaduro with the following 2 changes in your branch, I was able to generate a correct junit

diff --git a/native/phpunit/phpunit/src/Util/Log/JUnit.php b/native/phpunit/phpunit/src/Util/Log/JUnit.php
index 3a21ead..87afb77 100644
--- a/native/phpunit/phpunit/src/Util/Log/JUnit.php
+++ b/native/phpunit/phpunit/src/Util/Log/JUnit.php
@@ -198,8 +198,7 @@ final class JUnit extends Printer implements TestListener
                 if ($suite instanceof \Pest\Contracts\Test) {
                     $testSuite->setAttribute('file', $suite->getFilename());
                 } else {
-                    $class = new \ReflectionClass($suite->getName());
-                    $testSuite->setAttribute('file', $class->getFileName());
+                    $testSuite->setAttribute('file', $suite->tests()[0]->getFilename());
                 }
             } catch (\ReflectionException $e) {
             }
@@ -303,7 +302,7 @@ final class JUnit extends Printer implements TestListener
         // @codeCoverageIgnoreEnd

         if ($test instanceof \Pest\Contracts\Test) {
-            $testCase->setAttribute('class', $test->getPrintableTestCaseName());
+            $testCase->setAttribute('class', $class->getName());
             $testCase->setAttribute('classname', \str_replace('\\', '.', $test->getPrintableTestCaseName()));
             $testCase->setAttribute('file', $test->getFilename());
         } else {

I've created 2 pest files (tests) with 2 cases (test() functions) inside just to quickly check it.

Resulting junit report is:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="4" assertions="4" errors="0" warnings="0" failures="0" skipped="0" time="0.028040">
    <testsuite name="P\Tests\FirstSourceFileTest" file="/pest-example/tests/FirstSourceFileTest.php" tests="2" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.021014">
      <testcase name="it First file - first case" class="P\Tests\FirstSourceFileTest" classname="Tests.FirstSourceFileTest" file="/pest-example/tests/FirstSourceFileTest.php" assertions="1" time="0.019044"/>
      <testcase name="it First file - second case" class="P\Tests\FirstSourceFileTest" classname="Tests.FirstSourceFileTest" file="/pest-example/tests/FirstSourceFileTest.php" assertions="1" time="0.001970"/>
    </testsuite>
    <testsuite name="P\Tests\SecondSourceFileTest" file="/pest-example/tests/SecondSourceFileTest.php" tests="2" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.007026">
      <testcase name="it Second file - first case" class="P\Tests\SecondSourceFileTest" classname="Tests.SecondSourceFileTest" file="/pest-example/tests/SecondSourceFileTest.php" assertions="1" time="0.003728"/>
      <testcase name="it Second file - second case" class="P\Tests\SecondSourceFileTest" classname="Tests.SecondSourceFileTest" file="/pest-example/tests/SecondSourceFileTest.php" assertions="1" time="0.003297"/>
    </testsuite>
  </testsuite>
</testsuites>

For sure, this is not a ready solution, but probably will speed up your investigation on this topic. What do you think we need to do else to move it forward?

To be honest I am without time for this at the moment. But it's cool that we already know how to fix this problem in pest.

This issue is actually related to the issues we had with TeamCity printer.

I’m closing this issue because it has been inactive for a few months.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

maks-rafalko picture maks-rafalko  ·  6Comments

nunomaduro picture nunomaduro  ·  4Comments

olivernybroe picture olivernybroe  ·  7Comments

nunomaduro picture nunomaduro  ·  7Comments

AlexMartinFR picture AlexMartinFR  ·  6Comments