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

Add Junit support #291

Merged
merged 1 commit into from
May 5, 2021
Merged

Add Junit support #291

merged 1 commit into from
May 5, 2021

Conversation

olivernybroe
Copy link
Member

Q A
Bug fix? Kinda
New feature? yes
Fixed tickets #48

This PR adds support for Junit reporting.
Junit is used by many third party tools to look into the tests.

In this specific instance I want to add Junit so we can have support for infection php as they rely on junit format.

Adding the support for Pest into infection will not be part of pest codebase and this part will @maks-rafalko do for us.

@olivernybroe
Copy link
Member Author

This PR is awaiting feedback from @maks-rafalko.
I wanna know if the following xml is good enough for infection php

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="/Users/olivernybroe/Code/pest/tests/Unit/TestSuite.php" tests="1" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.005243">
    <testsuite name="P\Tests\Unit\TestSuite" file="/Users/olivernybroe/Code/pest/tests/Unit/TestSuite.php" tests="1" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.005243">
      <testcase name="it does not allow to add the same test description twice" class="Tests\Unit\TestSuite" classname="Tests.Unit.TestSuite" file="/Users/olivernybroe/Code/pest/tests/Unit/TestSuite.php" assertions="2" time="0.005243"/>
    </testsuite>
  </testsuite>
</testsuites>

/**
* @internal This class is not covered by the backward compatibility promise for PHPUnit
*/
final class JUnit extends Printer implements TestListener
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this file is a copy/paste from phpunit, there are very few changes to it. However it was not possible to just make a wrapper class, at least without a lot of reflections on top.

Comment on lines +312 to +317
if (TeamCity::isPestTest($test)) {
$testCase->setAttribute('class', $test->getPrintableTestCaseName());
$testCase->setAttribute('classname', str_replace('\\', '.', $test->getPrintableTestCaseName()));
// @phpstan-ignore-next-line
$testCase->setAttribute('file', $test->__getFileName());
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom logic

Comment on lines +186 to +188
if ($class->hasMethod('__getFileName')) {
$fileName = $class->getMethod('__getFileName')->invoke(null);
} else {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom logic

@@ -22,10 +22,13 @@ parameters:
- "#Method Pest\\\\Support\\\\Reflection::getParameterClassName\\(\\) has a nullable return type declaration.#"
-
message: '#Call to an undefined method PHPUnit\\Framework\\Test::getName\(\)#'
path: src/TeamCity.php
path: src/Logging
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These ignores were needed for both teamcity and junit

@nunomaduro
Copy link
Member

@olivernybroe Once this is ready let me know.

@maks-rafalko
Copy link

maks-rafalko commented May 2, 2021

This is brilliant. I tried to use this branch for Infection for my WIP feature/pest-adapter branch and everything works great so far.

What I did in my branch:

  1. Created PestAdapter
  2. Created e2e test to make sure Infection works properly for Pest
  3. In this test, created 2 source files and 2 pest tests with 1 and 3 cases correspondingly, just to make sure it works for different files for >1 tests cases. Added one "bad" test to make sure Mutant is escaped

And it works.

If someone wants to play with it (that would be great, since I created very basic pest tests), here is how it can be done:

git clone git@github.com:infection/infection.git
cd infection
composer install

git checkout feature/pest-adapter

cd tests/e2e/PestTestFramework
composer install

XDEBUG_MODE=coverage ../../../bin/infection --test-framework=pest --log-verbosity=all -s

Result:
pest-infection2

What's next:

  1. It would be cool if we can test Infection+Pest on some real project. If you can provide me repositories, I'll do this
  2. At this moment, I'm happy with this PR. It does the job for generating correct JUnit XML file - confirmed

I would expect the new Pest release (is it possible to have 1.0.6?) so I can finish Pest Adapter on Infection side, and then release the new version too.

@olivernybroe
Copy link
Member Author

@maks-rafalko Awesome, I'll take this out of draft then as testing it with infection is more something that belongs on infection/infection#1476.

I have some big projects which are pest based mostly, however they are closed source so I'll have to do the testing.
One thing that I think you should try out is running it when you have a mix of pest and phpunit tests and possibly also a data provider in pest.

@nunomaduro This is ready for review now. 👍
Btw. do you know of any pest projects which @maks-rafalko can use for testing out infection and pest integration?

@olivernybroe olivernybroe marked this pull request as ready for review May 2, 2021 20:08
Copy link
Member

@nunomaduro nunomaduro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Don't forget to make a pull request on the docs. After that, you can release a new version of Pest with this.

@maks-rafalko
Copy link

One thing that I think you should try out is running it when you have a mix of pest and phpunit tests and possibly also a data provider in pest.

checked

It works.

@olivernybroe
Copy link
Member Author

Looks good to me. Don't forget to make a pull request on the docs. After that, you can release a new version of Pest with this.

I am actually not sure what to document in the docs. JUnit option was there already, it was just broken. Do we want to have docs about us supporting infection?

@maks-rafalko
Copy link

maks-rafalko commented May 3, 2021

Do we want to have docs about us supporting infection?

that's what I had in mind - to create some page in Pest docs about Infection support. But I wanted to do it once support for Pest is merged to master in Infection, so that it's clear how exactly integration should work (I don't expect any changes to the commands above, but still).

I can open PR later on pest docs. Or, if you are going to do it yourself, please ping me so we can check everything together.

@olivernybroe
Copy link
Member Author

olivernybroe commented May 3, 2021

@maks-rafalko I have very little experience with mutation testing, so would be awesome if you could write the docs for usage with pest 👍
Are you able to make those docs before we make a release in pest? So we can have the docs part of the next release.

@maks-rafalko
Copy link

maks-rafalko commented May 3, 2021

Are you able to make those docs before we make a release in pest? So we can have the docs part of the next release.

but in this case Infection won't be ready. How I see it:

  1. Pest is released with this PR
  2. Infection releases new version with Pest Adapter (it can not be done earlier than Pest release)
  3. At the same time with p. 2 we can release documentation for Pest+Infection integration

Or, do you think it can be done differently? Can pest docs be released independently of pest source code?

@nunomaduro
Copy link
Member

nunomaduro commented May 3, 2021

@maks-rafalko I think @olivernybroe just meant having you doing a pull request for the docs. ( not actually merge them ).

@maks-rafalko
Copy link

Oh, ok. I will do it in the following days, no problem.

@olivernybroe
Copy link
Member Author

Should we merge this into master? 😄

@nunomaduro nunomaduro merged commit ca3f8b5 into pestphp:master May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants