Hey @mcorteel, great work on the fix! 🏅
I've tested it myself on several projects that have been upgraded to PHPUnit 7.2.2 and it works like a charm! Unfortunately I don't have any projects with PHPUnit 5 or less, so if someone can test backwards compatibility?
@DragonBe Backwards compatibility is broken by this fix. I'm not sure what the phabricator policy is regarding this, but I see two options:
- Creating another test engine (one for PHPUnit <= 5 and one for PHPUnit > 5)
- Parsing output differently depending on the PHPUnit version and reuse the old code where needed. This would require a little work to do things properly I guess.
Tough decision to make. Given the fact that most PHP projects are abandoning PHP 5 support is great, but with forcing users to upgrade is not always a good thing, as I'm not sure how this would impact the corporate/enterprise customers.
I do see T7408 is talking about dropping support for older versions, but they're only talking about dropping support for PHP 5.2. That means that 5.3, 5.4, 5.5 and 5.6 still require some form of support. It would be great if there would be some usage stats to give us some insights in the actual platforms running Phabricator.
I didn't even suggest dropping support, I'm merely listing the options to allow both versions to work.
We use a manually patched version of arcanist because it suits our company's use case, but it wouldn't be suitable for a merge in Phabricator's codebase in this state.
I really haven't looked into the backwards compatibility issue as we're already test-driving on PHP 7.3 preview, so I can't help you out there. Your suggestion to detect the PHP version and offer this solution only for versions of PHP 7 and higher seems like the most obvious approach.