Update PHPUnit to 4.x

PHPunit 4.0.0 was released. https://github.com/sebastianbergmann/phpunit/wiki/Release-Announcement-f...

This one's older, let's use this issue.

Oops. We need a new patch.

Removing the staticExpects in AccessManagerTest and ReverseProxySubscriberUnitTest does not make the tests fail, but it might affect what lines of code they cover.

However, the staticExpects in EntityManagerTest is a problem.
In EntityManager::buildBaseFieldDefinitions() it calls $base_field_definitions = $class::baseFieldDefinitions($entity_type); and then foreaches over it. Removing the staticExpects makes this null, so you get a nice Invalid argument supplied for foreach().

Not sure what to do there.

@cosmicdreams: do you have something already, or can I safely make a patch without being in your way?

@cosmicdreams
Do you have a list of tests which fail after the update?

Whath happens if you use the native phpunit test runner?

nothing currently, it somehow hands off to Drupal's runner. If you update composer.json and run composer update it will use the 4.0 runner and it fails because we use the goofy staticExpects method that is gone because it was apparently sort of broken. I was going to take a swing then saw this was sprinkled in our base entity tests and I don't have the chops to testing EntityAPI.

http://phpunit.de/manual/current/en/appendixes.upgrading.html#appendixes...

Yeah thats the error I mentioned. staticExpects was removed as is documented in the upgrade guide I posted.
Fatal error: Call to undefined method Mock_Request_5a8ef19b::staticExpects() in

I called it out in #7 as well.

This updates PHPUnit to 4.1.1. I will continue working on replacing staticExpects() usages.

This patch contains the upgrade and the necessary staticExpects() conversions.

Some tests ran through with this. Lets see how testbot likes it.

cool its green. can we have the changes excluding /vendor so dreditor doesnt crash?

Is it possible to split those functional test changes into a separate issue?

Is it possible to split those functional test changes into a separate issue?

What changes? There is nothing in here that isn't necessary for the upgrade.

I think he meant fix the tests first since the changes will work in the current version of phpunit, then do the upgrade changes.

Postponed on #2278705: Make all tests compatible with PHPUnit 4.

Here is a patch to only upgrade PHPUnit and not touch any other files.

Instructions to generate this patch:

1. Edit composer.json change phpunit version to 4.1.*
2. Remove /core/vendor/phpunit
3. remove /core/vendor/bin/phpunit
4. run "composer update phpunit/phpunit"

Thank you for the instructions!

#2278705: Make all tests compatible with PHPUnit 4 is in so we'll need a re-roll and this is back active!

Yeah I couldn't apply it either. I had to export with --binary to get the patch to re-apply. Lets see what the bot says.

gugh, the simpletest.module fix.

Looks like the latest patches have the whole zend framework in them, I also tried the approach in #35 manually and it resulted in the same.

Dunno why but the steps in 35 still work for me.

Now including the simpletest.module fix too :) And patch made with --binary option too... hence the size increase.

After applying the patch with -a the following changes outsite /core/vendor/phpunit and core/vendor/sebastian are made:

git st | grep -v 'core/vendor/sebastian' | grep -v 'core/vendor/phpunit'
M  composer.json
M  composer.lock
M  core/modules/simpletest/simpletest.module
M  core/vendor/bin/phpunit
M  core/vendor/composer/autoload_classmap.php
M  core/vendor/composer/include_paths.php
M  core/vendor/composer/installed.json

Which are all expected.

The patch adds a few new packages to the sebastian vendor directory, but since they are all licensed under the BSD-3, this still allows us to add the code to our own repo. Other than that, the patch only changes core's composer.json and simpletest.module, and updates phpunit packages, so I think we're good to go!

Thanks for helping move this forward, people!

Ignore that. It's feedback for an old patch.

Holy crow. This patch officially wins the "Most Deceptively Simple Looking But Biggest Pain In The Rear To Actually Commit Patch" award. ;)

For some reason—I suspect because I'm on Mac OS which is case-insensitive, and there are file renames here—the patch would not apply for me, even with git apply --binary --index. It complained loudly about a bunch of files already in my working directory. Neither a git clean -df; git reset --hard nor a fresh git clone resolved the matter, nor did blitzing the core/vendor/phpunit dir.

What we ended up resorting to was chx applying the patch on his local clone of D8 and then tarring it up and sending it to me over Skype for me to execute on my machine. :P However, because of various reasons, that needed to actually be done 3(!) times, at ~145M of transfer a pop. :)

Long story short, committed and pushed to 8.x, and I added chx to the commit credit here, because without him, this never would've been committed, at least by me. :P Thanks!

All PHPUnit tests are being ignored by run-tests.sh now.
The last core test run before this patch had ~72000 passes. The first one after this had ~67000, and we have ~5000 unit test assertions.

The typehint changes to simpletest.module explains the problem. What we have as $test_suite is actually an array of test suites.
We need to retrieve the test from the suite.

If this patch has ~72000 passes, it's good.

Hmmmm, it didn't get back to 70K, but I do now see unit tests like AddRoleUserTest and LocalActionManagerTest in the list, which aren't on https://qa.drupal.org/8.x-status.

So maybe my math is off?

Yeah, we're still missing tests.

I can't see a pattern yet, but one that is missing and that's contributing 2800 assertions is SystemControllerTest. What's weird is that I can run it if I pass it explicitly but the discovery doesn't find it:

$ php core/scripts/run-tests.sh --list | grep SystemControllerTest

Finds nothing. But...

$ php core/scripts/run-tests.sh --class "Drupal\system\Tests\Controller\SystemControllerTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\system\Tests\Controller\SystemControllerTest

Test run started:
  Sunday, June 15, 2014 - 10:35

Test summary
------------

Drupal\system\Tests\Controller\SystemControllerTest          2808 passes                                      

Test run duration: 4 sec

Can we do this?

Or without is_subclass_of().

+++ b/core/modules/simpletest/simpletest.module
@@ -789,8 +789,8 @@ function simpletest_phpunit_get_available_tests($module = NULL) {
+      if (is_subclass_of($name, '\Drupal\Tests\UnitTestCase') && !array_key_exists($name, $test_classes) && (!$module || substr($name, 0, $n) == $prefix)) {

Why does is_subclass_of() work, while instanceof doesn't?

Because $test is PHPUnit_Framework_TestSuite, while $name is the test class itself.
#57 works great, and has 72K passes, and php core/scripts/run-tests.sh --list | grep SystemControllerTest works for it. Thanks @olli!

I knew there was no way we were getting off that easily. :)

Committed and pushed to 8.x. Thanks so much for the quick find + fix!

Thanks @webchick!

Follow-up: #2294731: Simpletest fails to run PHPUnit on Windows