Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: --log-junit and --coverage-xml conflicts #1095

Open
ndench opened this issue Feb 16, 2024 · 7 comments
Open

[Bug]: --log-junit and --coverage-xml conflicts #1095

ndench opened this issue Feb 16, 2024 · 7 comments
Labels

Comments

@ndench
Copy link

ndench commented Feb 16, 2024

What Happened

I use the junit and xml coverage output from Pest for Infection.
This has been working until Pest 2.32.0 where junit support was added to 2.x.
Before this, I believe that PHPUnit was outputting it's own Junit output which somehow was working fine.

The previous junit.xml format
<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="/var/www/html/phpunit.xml.dist" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.057603">
    <testsuite name="Project Test Suite" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.057603">
      <testsuite name="P\Tests\CalculatorTest" file="/var/www/html/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(196) : eval()'d code" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.057603">
        <testcase name="__pest_evaluable_it_adds" file="/var/www/html/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(196) : eval()'d code" line="22" class="P\Tests\CalculatorTest" classname="P.Tests.CalculatorTest" assertions="1" time="0.054071"/>
        <testcase name="__pest_evaluable_it_subtracts" file="/var/www/html/vendor/pestphp/pest/src/Factories/TestCaseFactory.php(196) : eval()'d code" line="37" class="P\Tests\CalculatorTest" classname="P.Tests.CalculatorTest" assertions="1" time="0.003532"/>
      </testsuite>
    </testsuite>
  </testsuite>
</testsuites>
The current junit.xml format
<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="/var/www/html/phpunit.xml.dist" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.052996">
    <testsuite name="Project Test Suite" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.052996">
      <testsuite name="Tests\CalculatorTest" file="tests/CalculatorTest.php" tests="2" assertions="2" errors="0" failures="0" skipped="0" time="0.052996">
        <testcase name="it adds" file="tests/CalculatorTest.php::it adds" class="Tests\CalculatorTest" classname="Tests.CalculatorTest" assertions="1" time="0.050272"/>
        <testcase name="it subtracts" file="tests/CalculatorTest.php::it subtracts" class="Tests\CalculatorTest" classname="Tests.CalculatorTest" assertions="1" time="0.002724"/>
      </testsuite>
    </testsuite>
  </testsuite>
</testsuites>

The new format is much better, however I believe the xml coverage report also needs to be updated, as it now references tests that do not exist <covered by="P\Tests\CalculatorTest::__pest_evaluable_it_adds"/>

The coverage-xml report
<?xml version="1.0"?>
<phpunit xmlns="https://schema.phpunit.de/coverage/1.0">
  <file name="Calculator.php" path="/">
    <totals>
      <lines total="17" comments="0" code="17" executable="2" executed="2" percent="100.00"/>
      <methods count="2" tested="2" percent="100.00"/>
      <functions count="0" tested="0" percent="0"/>
      <classes count="1" tested="1" percent="100.00"/>
      <traits count="0" tested="0" percent="0"/>
    </totals>
    <class name="App\Calculator" start="5" executable="2" executed="2" crap="2">
      <namespace name="App"/>
      <method name="add" signature="add($a, $b)" start="7" end="10" crap="1" executable="1" executed="1" coverage="100"/>
      <method name="subtract" signature="subtract($a, $b)" start="12" end="15" crap="1" executable="1" executed="1" coverage="100"/>
    </class>
    <coverage>
      <line nr="9">
        <covered by="P\Tests\CalculatorTest::__pest_evaluable_it_adds"/>
      </line>
      <line nr="14">
        <covered by="P\Tests\CalculatorTest::__pest_evaluable_it_subtracts"/>
      </line>
    </coverage>
    <source>
      <!-- ... -->
    </source>
  </file>
</phpunit>

This causes infection to fail because it cannot find the necessary tests:

In TestFileNameNotFoundException.php line 48:

  [Infection\TestFramework\Coverage\JUnit\TestFileNameNotFoundException]
  For FQCN: P\Tests\CalculatorTest. Junit report: /var/www/html/build/junit.xml

How to Reproduce

Using reproducer

I created a small reproducer: https://github.com/ndench/pest-junit-xml-conflicts
To use this, you'll need:

  • git
  • docker compose
  • make
  1. git clone [email protected]:ndench/pest-junit-xml-conflicts.git
  2. make vendor
  3. make test

Now you can see the junit output at build/junit.xml and the coverage XML output at build/coverage-xml.
You can also run make infect to see the error from infection.

Switch to Pest 2.31
  1. make pest-2.31
  2. make test

Now you can see the previous build/junit.xml output, and also run make infect to see infection pass.

You can also use make pest-2.32 which will install 2.32.0 to see the first version which introduced the change.
And make pest-latest to get back to the latest version.

From scratch

It's also very easy to do this yourself from a blank project.

  1. Install a fresh php app (framework doesn't matter, I used Symfony in my sample).
  2. Create a PHP file that does something
  3. Create a Pest test for the PHP file
  4. Run pest with the --log-junit and --coverage-xml flags
  5. Note conflicts detailed above in the junit.xml and xml coverage

Sample Repository

https://github.com/ndench/pest-junit-xml-conflicts

Pest Version

2.32.0-2.33.6

PHP Version

8.2.13

Operation System

Linux

Notes

By manually editing the vendor/pestphp/pest/src/Bootstrappers/BootOverrides.php, I can make Pest use the old PHPUnit format to workaround this issue temporarily. But I don't believe this is a solution to the problem on it's own.

image

@ndench ndench added the bug label Feb 16, 2024
@michael-rubel
Copy link

I can confirm that Pest 2.x is unusable with Infection. Before merging a proper JUnit logging it was not failing on CI, but also was not reporting any mutants, you were always getting the 100% MSI.

@nuernbergerA
Copy link
Contributor

Hey @ndench

Can you confirm that it works if you manipulate the coverage XML to match the junit output

<covered by="Tests\CalculatorTest::it adds"/>

@ndench
Copy link
Author

ndench commented Apr 6, 2024

Hey @nuernbergerA, yes I can confirm that changing the coverage XML manually makes infection work.

ie. I made the following change to coverage-xml/index.xml

    <tests>
-      <test name="P\Tests\CalculatorTest::__pest_evaluable_it_adds" size="unknown" status="success"/>
+      <test name="Tests\CalculatorTest::it adds" size="unknown" status="success"/>
-      <test name="P\Tests\CalculatorTest::__pest_evaluable_it_subtracts" size="unknown" status="success"/>
+      <test name="Tests\CalculatorTest::it subtracts" size="unknown" status="success"/>
    </tests>

And the following change to coverage-xml/Calculator.php.xml

    <coverage>
      <line nr="9">
-        <covered by="P\Tests\CalculatorTest::__pest_evaluable_it_adds"/>
+        <covered by="Tests\CalculatorTest::it adds"/>
      </line>
      <line nr="14">
-        <covered by="P\Tests\CalculatorTest::__pest_evaluable_it_subtracts"/>
+        <covered by="Tests\CalculatorTest::it subtracts"/>
      </line>
    </coverage>

@Sjustein
Copy link

Is anything blocking the junit output from changing back? If not, it should be relatively simple to 'rollback' the changes to the junit output right?

Sjustein added a commit to Globy-App/hash-sensitive that referenced this issue Apr 26, 2024
Sjustein added a commit to Globy-App/hash-sensitive that referenced this issue Apr 26, 2024
@nuernbergerA
Copy link
Contributor

Just rolling back isn't the right thing because then the location is messed up again.

I will open a PR that will output a working XML for both scenarios.

@nuernbergerA
Copy link
Contributor

@ndench @michael-rubel @Sjustein

please check the #1145 if anything works now

@Sjustein
Copy link

Infection does indeed understand this format again, but it always reports a 100% mutation score. I'm not sure if this is an issue specific to your implementation though, it might just be something with infection. Thanks for the great speed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants