Problem/Motivation
In order for our tests to run, we need to specify a minimum version of phpspec/prophecy. We rely on a bugfix introduced in 1.4: https://github.com/phpspec/prophecy/issues/139
See comment #5 for analysis and solution.
Steps to repro:
Checkout 8.4.x branch of core.
Assume we've made the changes from #2887000: composer.json does not constrain Symfony components to minor and patch versions that are compatible with Drupal or apply the patch in #66.
$ composer update --prefer-lowest --prefer-dist
[ stuff happens]
$ composer show *prophecy
phpspec/prophecy v1.3.1 Highly opinionated mocking framework for PHP 5.3+
$ ./vendor/bin/phpunit -c core/ --testsuite unit
Somewhere about 75% the test run will fatal out. You'll see this in your log file:
PHP Fatal error: Declaration of Double\FormStateInterface\P1510::getCompleteForm() must be compatible with & Drupal\Core\Form\FormStateInterface::getCompleteForm() in /Users/paul/pj2/drupal/vendor/phpspec/prophecy/src/Prophecy/Doubler/Generator/ClassCreator.php(49) : eval()'d code on line 2
This tells us that we have some kind of incompatibility with prophecy.
We increase the minimum requirement of phpspec/prophecy to ^1.4 and this error no longer happens, and tests finish normally.
Proposed resolution
Limit our minimum prophecy version to ^1.4
Remaining tasks
DrupalCI could catch these real actual constraints if we test them: #2874198: Create and run dependency regression tests for core
User interface changes
API changes
Data model changes
Problem/Motivation
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#11 | 2900800_11.patch | 430 bytes | Mile23 |
#5 | 2900800_5.patch | 426 bytes | Mile23 |
#3 | prophecy13-test-only.patch | 2.87 KB | mpdonadio |
prophecy14.patch | 420 bytes | mpdonadio | |
Comments
Comment #2
Mile23Comment #3
mpdonadioOK, let's show the failing run to see if we get more info.
Comment #5
Mile23Running the tests manually using
--testsuite unit
, I eventually see this log info:So that tells us that from Drupal perspective, the error comes about in
FormStateDecoratorBaseTest::setUp()
. Does that method do anything weird? No:Since the error happens during
setUp()
and not during a test, we know that the problem has to do with prophecy not fully implementingFormStateInterface
in some way, and that our test method is not mucking with the mock.The interface method that's generating the error is
getCompleteForm()
which returns a reference to an array.And behold: https://github.com/phpspec/prophecy/issues/139 which is titled "Fatal error when mocking reference returning interface methods"
We consult the changelog for 1.4.0: https://github.com/phpspec/prophecy/blob/v1.4.0/CHANGES.md And there it is: "Support for mocking classes with methods that return references (thanks @edsonmedina)"
So: We either need to change the test (and probably others) so it uses phpunit mocks, or we need to specify a minimum version for prophecy of 1.4. I vote for the latter.
The patch is a re-upload of the original one so it's last in line.
Comment #6
Mile23Comment #7
Mile23Comment #8
dawehnerI totally agree on not changing the tests but rather rely on the version number for the fix.
Comment #9
larowlanRequeued after #2887000: composer.json does not constrain Symfony components to minor and patch versions that are compatible with Drupal
Comment #11
Mile23Reroll.
Comment #12
Mile23Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedStraight reroll. Back to RTBC per #8.
Comment #16
effulgentsia CreditAttribution: effulgentsia at Acquia commentedThanks, this is great. Pushed to 8.5.x and cherry picked to 8.4.x.
Comment #17
effulgentsia CreditAttribution: effulgentsia at Acquia commentedTagging for release notes just in case, but I don't know if we really need to mention it. The installed version in our composer.lock file is 1.7.0, so this issue does not change what someone gets from doing a
composer install
, it only changes what someone would get from doing acomposer update --prefer-lowest
. And it's also only arequire-dev
dependency.