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

Fixed addTestSuites-Call in case the name attribute is empty and there is only one testsuite within the testsuite. #19

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thirsch
Copy link

@thirsch thirsch commented Sep 13, 2021

We are using phpunit-merger to merge the result of named testsuites of phpunit. Handling of testsuites with only one testsuite as a child is already handled in case of subtestsuites. But was not handled correctly on the top level.

…e is only one testsuite within the testsuite.
@thirsch
Copy link
Author

thirsch commented Sep 15, 2021

Are you still maintaining the project @IchHabRecht? If yes, I have two other features that I can create pull-requests for.

@@ -82,7 +82,8 @@ private function addTestSuites(\DOMElement $parent, array $testSuites)
foreach ($testSuites as $testSuite) {
if (empty($testSuite['@attributes']['name'])) {
if (!empty($testSuite['testsuite'])) {
$this->addTestSuites($parent, $testSuite['testsuite']);
Copy link

@mtamazlicaru mtamazlicaru Sep 19, 2021

Choose a reason for hiding this comment

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

Hi. Thank you for contributing. I am trying also to use this package.
Can we add some tests for the case that you added in order to reproduce the issue then the fix?

@IchHabRecht
Copy link
Contributor

Hi @thirsch,

Thank you for your pull request. To understand the problem and fix, I kindly ask you to share an example of your xml file(s).

@thirsch
Copy link
Author

thirsch commented Sep 20, 2021

Hi @IchHabRecht,

I mixed up the issue description and the root cause a bit. Here is why the fix is required:

If you have for example these two xml files:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092">
    <testsuite name="Tests1" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092">
      <testsuite name="Tests\One\MyTest" file="/opt/project/tests/one/MyTest.php" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092">
        <testcase name="testOne" class="Tests\One\MyTest" classname="Tests.One.MyTest" file="/opt/project/tests/one/MyTest.php" line="9" assertions="1" time="0.030092"/>
      </testsuite>
    </testsuite>
  </testsuite>
</testsuites>

and

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="" tests="2" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.043921">
    <testsuite name="Tests2" tests="2" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.043921">
      <testsuite name="Tests\Two\MySecondTest" file="/opt/project/tests/two/MySecondTest.php" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.043866">
        <testcase name="testTwo" class="Tests\Two\MySecondTest" classname="Tests.Two.MySecondTest" file="/opt/project/tests/two/MySecondTest.php" line="9" assertions="1" time="0.043866"/>
      </testsuite>
      <testsuite name="Tests\Two\MyThirdTest" file="/opt/project/tests/two/MyThirdTest.php" tests="1" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.000055">
        <testcase name="testThree" class="Tests\Two\MyThirdTest" classname="Tests.Two.MyThirdTest" file="/opt/project/tests/two/MyThirdTest.php" line="9" assertions="1" time="0.000055"/>
      </testsuite>
    </testsuite>
  </testsuite>
</testsuites>

with more then one nested testsuites in the second file, the output of the merge will be:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="Tests\One\MyTest" file="0" tests="0" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092" line="9">
    <testcase name="testOne" class="Tests\One\MyTest" classname="Tests.One.MyTest" file="/opt/project/tests/one/MyTest.php" line="9" assertions="1" time="0.030092"/>
  </testsuite>
</testsuites>

The problem: In the second file, testsuite "Tests2" does not only contain one testsuite, but two (Tests\Two\MySecondTest and Tests\Two\MyThirdTest). The example project used to produce the files contains the following structure:

  • tests
    • one
      • MyTest.php
        • function testOne()
    • two
      • MySecondTest.php
        • function testTwo()
      • MyThirdTest.php
        • function testThree()

and the following phpunit.xml:

<?xml version="1.0" encoding="UTF-8"?>
<phpunit
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    colors="true"
    xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
>
    <testsuites>
        <testsuite name="Tests1">
            <directory>tests/one</directory>
        </testsuite>
        <testsuite name="Tests2">
            <directory>tests/two</directory>
        </testsuite>
    </testsuites>
</phpunit>

I've executed the two testsuites separately by invoking vendor/bin/phpunit --testsuite Tests1 --log-junit logs/tests-one.xml and vendor/bin/phpunit --testsuite Tests2 --log-junit logs/tests-two.xml and merged them by calling vendor/bin/phpunit-merger log logs result.xml.

With the provided fix, the script merges the two example files correctly to the following result:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite name="Tests1" tests="0" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092" line="9">
    <testsuite name="Tests\One\MyTest" file="0" tests="0" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.030092" line="9">
      <testcase name="testOne" class="Tests\One\MyTest" classname="Tests.One.MyTest" file="/opt/project/tests/one/MyTest.php" line="9" assertions="1" time="0.030092"/>
    </testsuite>
  </testsuite>
  <testsuite name="Tests2" tests="0" assertions="2" errors="0" warnings="0" failures="0" skipped="0" time="0.043921" line="18">
    <testsuite name="Tests\Two\MySecondTest" file="0" tests="0" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="0.043866" line="9">
      <testcase name="testTwo" class="Tests\Two\MySecondTest" classname="Tests.Two.MySecondTest" file="/opt/project/tests/two/MySecondTest.php" line="9" assertions="1" time="0.043866"/>
    </testsuite>
    <testsuite name="Tests\Two\MyThirdTest" file="0" tests="0" assertions="1" errors="0" warnings="0" failures="0" skipped="0" time="5.5E-5" line="9">
      <testcase name="testThree" class="Tests\Two\MyThirdTest" classname="Tests.Two.MyThirdTest" file="/opt/project/tests/two/MyThirdTest.php" line="9" assertions="1" time="0.000055"/>
    </testsuite>
  </testsuite>
</testsuites>

@IchHabRecht IchHabRecht force-pushed the master branch 2 times, most recently from 425b12d to 76e1b50 Compare October 8, 2021 14:48
@thirsch
Copy link
Author

thirsch commented Jan 25, 2022

Any update, if this will be merged? I've got some more improvements to phpunit-merger and would like to know if you are interested in gettings pullrequests for them?

@marcovo
Copy link

marcovo commented Apr 2, 2022

I'm also interested in this fix. Is there any indication whether/when it will get merged?

@jordan-factor
Copy link

this PR would also fix my usage of this

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.

5 participants