Attached patch adds the phpunit library only, in the interest of simplifying #1901670: Start using PHPUnit for unit tests.
The composer.json is pointed to a fork on my github which is necessary to resolve a conflict with symfony yaml. This can be replaced with the official version once symfony 2.2 is released, which should be very soon.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | d8-vendor-phpunit-5.patch | 2.84 MB | msonnabaum |
| #2 | d8-vendor-phpunit-2.patch | 2.79 MB | msonnabaum |
| #1 | d8-vendor-phpunit-1.patch | 2.83 MB | msonnabaum |
| d8-vendor-phpunit.patch | 2.83 MB | msonnabaum |
Comments
Comment #1
msonnabaum commentedAccidentally removed the readme.
Comment #2
msonnabaum commentedAnd here's a version with the latest composer, because things change I guess.
Comment #3
effulgentsia commentedThe concept of using PHPUnit got good support in #1901670: Start using PHPUnit for unit tests and elsewhere, and that issue's integration with Drupal is pretty far along, so makes sense to me to get the library in first, and let that issue go through some additional reviews and rerolls without 3MB of patch baggage.
Comment #4
effulgentsia commentedI RTBC'd #1. #2 is missing composer.json.
Comment #5
msonnabaum commentedI'm dumb. One more time.
Comment #6
msonnabaum commentedComment #7
effulgentsia commentedThanks. Also, recategorizing as a major task, as we've done for prior "add this library" issues.
Comment #8
catchI agree with moving over to phpunit for unit tests in general, although I haven't reviewed the other issue in any depth yet.
Dries usually likes to commit new libraries so moving over to him.
Comment #9
dries commentedCommitted to 8.x. Now go here: http://drupal.org/node/1901670. :)
Comment #10
sunUgh. Wait a moment.
We intensively discussed this topic in the past and everyone agreed that it was one of the worst ideas ever to include simpletest in core and that we do not want to include PHPUnit in core.
Here's the more elaborate battle plan: #1567500: [meta] Pave the way to replace the testing framework with PHPUnit and possibly rewrite Simpletest
Comment #11
msonnabaum commentedVendoring something is very different than what we did with simpletest. See the other issue for why this is necessary.
Comment #12
sunThe only reason I see over there is this:
We decided against that, because
1) Literally every other PHP project that is using PHPUnit requires you to set up PHPUnit on your own, so PHPUnit is expected to exist on your machine already. Due to that, there's also plenty of documentation out there already.
2) A UI-based test runner should not be part of Drupal core in the first place. It may continue to exist in contrib, but there are actually dedicated, non-Drupal PHPUnit UIs available already.
I really do not think we want to include PHPUnit itself in core.
Comment #13
msonnabaum commentedThe main reason is the UI because it's completely out of scope to even discuss the removal of the simpletest UI since there's no way that will happen for D8.
If we don't want to vendor PHPUnit, we need to not ship with any vendored libraries and require a composer install, but that's also way out of scope for this issue.
Comment #14
sunNormally, you install and set up PHPUnit via PEAR or their PHAR in a system-wide location on your machine. I've never seen any other PHP project that directs you to install PHPUnit within/into the code base of your PHP application. Why would you want to install it into Drupal?
Also, I don't see why removing the Simpletest UI can't still happen for D8. To run existing tests, we only need the base test classes and test runner code.
Comment #15
msonnabaum commentedGlobal installs are common because that's all we had with pear. In the ruby/node world it's not uncommon at all to have your test runner included in your dependencies and run a local version.
You're right in that no other project uses a vendored copy of PHPUnit, but other projects also don't commit their vendor dirs. That's a decision we made, so I'm just rolling with that. I'm all for not doing that, but that's a separate issue.
Comment #16
effulgentsia commentedThat's a red herring. Do any other projects add PHPUnit to their composer.json, regardless of whether they commit their /vendor directory to their codebase?
Wouldn't it make sense then to leave PHPUnit in our codebase until after we remove Simpletest UI, and then remove PHPUnit at that time?
Comment #17
msonnabaum commentedIt's not quite a red herring, I just left out some important details.
The right way to do this would be to add it to require-dev:
http://getcomposer.org/doc/04-schema.md#require-dev
I should have done that, but we can change that when we move back to the official repo.
Here's an example of a php project doing this:
https://github.com/guzzle/guzzle/blob/master/composer.json#L58
And here's official PHPUnit documentation on this method:
http://www.phpunit.de/manual/current/en/installation.html#installation.c...
That said, require-dev doesn't change the fact that we decided to ship with all our dependencies committed to our repository. I'd love to not do that, but as I said before, it's out of scope for this issue.
Comment #18
webchickDoesn't look like there's anything left to fix here? And in any case, Dries doesn't need to be assigned anymore.