Load configuration even if libxml entity loader is disabled

[jwoschitz]

This PR addresses a problem which occurs if the bootstrap file contains logic which disables the libxml entity loader.

The following warning will be issued:

Warning: simplexml_load_file(): I/O warning : failed to load external entity "/home/the/project/path/phpunit.xml" in /home/the/project/path/vendor/brianium
/paratest/src/ParaTest/Runners/PHPUnit/Configuration.php on line 39

And the following exception will be thrown:

[RuntimeException] No path or configuration provided (tests must end with Test.php)

It would be quite inconvenient to have to bypass the composer autoloading just to handle this case manually. Therefore I just wrapped the simplexml_load_file() call into a block which enables the libxml entity loading temporarily and restores the setting after the xml file has been parsed.

More information: http://phpsecurity.readthedocs.org/en/latest/Injection-Attacks.html#defenses-against-xml-external-entity-injection

[julianseeger]

Thanks for the PR.
I didn't examine the patch yet, but the build seems to fail xml related. Can you fix this please?
Maybe you forgot to reset the state if no exception ist thrown / if the test passes and affect other tests this way. Therefore you should move the libxml_disable_entity_loader($before); into the tearDown method.

[jwoschitz]

Thank you for looking into that.

I adapted the test but I just moved the restoration of the state before to the end of the test. I did not want to introduce some state in the testcase just for this one test (that would be needed if it is located in the tearDown). Also I replaced the re-throwing of the exception with an assertion.

Unfortunately the build is still failing but I cannot reproduce these issues on my local setup.

It seems that it is triggered by a changed order of the xml attributes in the generated JUnit log files.

1) ParaTest\Logging\JUnit\WriterTest::testSingleFileLog
Failed asserting that two DOM documents are equal.

--- Expected
+++ Actual

@@ @@
<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
- <testsuite name="AnotherUnitTestInSubLevelTest" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/level1/AnotherUnitTestInSubLevelTest.php" tests="3" assertions="3" failures="0" errors="0" time="0.005295">
+ <testsuite name="AnotherUnitTestInSubLevelTest" tests="3" assertions="3" failures="0" errors="0" time="0.005295" file="/home/brian/Projects/parallel-phpunit/test/fixtures/tests/level1/AnotherUnitTestInSubLevelTest.php">

Since it seems only to fail for tests with PHPUNIT_VERSION='4.0.*' I guess it is related to a change to an external dependency.

Though the expected format seems to match with the format mentioned in the phpunit documentation:
http://phpunit.de/manual/4.0/en/logging.html

If you can give me any pointers I will gladly try to resolve these issues.

[julianseeger]

The error in the build is related to #91
But the three failures probably got introducedby you (I rescheduled the travis build).

I'm going to take a look in the failures in ~7 hours, maybe a can narrow it down.
Did all dependent versions match when you tried to reproduce it locally? (you can see the version-hashes in the travis log)

[jwoschitz]

I was able to reproduce it locally after updating phpunit 4.0.* to the most current version

Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Removing phpunit/phpunit (4.0.17)
  - Installing phpunit/phpunit (4.0.18)
    Downloading: 100%

I think this behaviour is related to the changes in this commit sebastianbergmann/phpunit@7dadedf

It changed the way equality between two xml strings is compared.

When I updated the composer.json like this (which is one commit before the one mentioned above):

{
    "name": "brianium/paratest",
    "require": {
        /* ... */
        "phpunit/phpunit": "4.0.x-dev#6a604dd492c8994345f600b309a8135cdd92b701",
        /* ... */
    },
    /* ... */
}

Then test ParaTest\Logging\JUnit\WriterTest had no failures.

I don't know exactly how to proceed from here, we could just adapt the expected output.

On the other hand the order of certain xml attributes should not matter if they are contained in the same node. But this fix / issue would be more suited for the phpunit project.

All of this seems not related to the current PR. Maybe you can validate it by just re-scheduling the travis build for the master? I am pretty confident that it will break as well.

[julianseeger]

Unfortunately, you were right.
As I like to follow the rule "never check on a broken build", we have to fix that first (or make someone fix it ^^).

[julianseeger]

Ok now finally: thanks for the contribution, the PR got merged.

Just FTR: your test could be less complex if you would just do something like

} catch (\Exception $e) {
    $this->fail('Could not instantiate Configuration: ' . $e->getMessage());
}

or just not catch the exception.