Follow-up to #2461863: Upgrade PHPUnit to the latest stable release
PHPUnit 4.8 is the current stable release series, it became stable on April 3, 2015.
We currently use 4.8.6. There is an update to 4.8.10.
Also, we only require PHPUnit 4.6 but our composer.json requires 4.8.*. Change to allow a lower version of PHPUnit if Drupal is used in a composer workflow. See #2400407-53: [meta] Ensure vendor (PHP) libraries are on latest stable release and #2400407-141: [meta] Ensure vendor (PHP) libraries are on latest stable release for some discussion around this subject.
Beta phase evaluation
Reference: https://www.drupal.org/core/beta-changes
Prioritized changes
External PHP library update
Comment | File | Size | Author |
---|---|---|---|
#28 | upgrade_phpunit_to-2568595-28.patch | 149.73 KB | hussainweb |
#10 | testing-phpunit-4.6-only.patch | 700.58 KB | hussainweb |
Comments
Comment #2
hussainwebThis is the relevant change in composer.json.
Comment #3
hussainwebI should point out again that even though composer.json now requires a minimum of PHPUnit 4.6, the patch actually contains the latest stable, which is 4.8.7.
Comment #4
dawehnerSeriously, let's not downgrade to 4.6, are you sure we don't use any of the newer features? Let's go with ~4.8 ?
Comment #5
hussainwebI believe we are marking all external library upgrades as critical before RC. If this is not the case, please downgrade.
@dawehner: I believe that we should still decrease the minimum required version to the version we actually need. We had changed it to
4.8.*
in #2400407-68: [meta] Ensure vendor (PHP) libraries are on latest stable release just to upgrade but it was not necessary then.Comment #6
hussainweb@dawehner: The update to PHPUnit was committed recently in #2400407-68: [meta] Ensure vendor (PHP) libraries are on latest stable release, and the version constraint was changed just to update. I don't think we have added anything to tests since then (it's only 10 days or so). We can test in a patch to see. I will do that now.
Comment #7
chx CreditAttribution: chx commentedPlease provide more information about what's going on. We are dropping to 4.6 and upgrading to 4.8.7?? *confused*
Comment #8
catchPatch-level releases aren't critical to update to, unless there's something independently critical about them like a security release.
Comment #9
hussainweb@chx: We are not dropping 4.6. I don't think we are using anything new after 4.6 in any of our tests as we upgraded only about 10 days back. I will test to be sure though. I believe we could still use PHPUnit 4.6 and our composer dependencies should point to that. That would be relevant if we use Drupal in a composer workflow.
Mixologic outlined why we should do this in #2400407-53: [meta] Ensure vendor (PHP) libraries are on latest stable release.
Comment #10
hussainwebI am just testing the attached patch with PHPUnit 4.6.10. If it fails, then we could just play it safe and change the version constraint to
~4.8
.Comment #11
hussainwebI'll try to explain this a bit more clearly.
We have tried and kept up to date with all our external libraries over the past few months. It is a good idea to upgrade even if we are not using the new features so that we can be sure that we are on the latest stable branch. As long as there are no BC breaks, it is very simple to keep up to date as well.
However, while updating, we also increase the version in composer.json even if we didn't need to. PHPUnit is one example where this happened; composer/installers is another example (see #139).
Normally, this doesn't matter as we ship with composer.lock, but if used Drupal with a composer based workflow, Drupal's composer.lock would not be used and we may get any version. Also, think of another scenario: Another component needs PHPUnit 4.9, but since we have locked it to PHPUnit 4.8.*, that package won't be able to work with Drupal, even though it should.
To keep things simple, our composer version constraints should just specify the range of versions we can work with rather than locking to just the latest version. This ensures maximum compatibility possible.
I am just waiting on the patch in #10. If it turns green, then we are not using any new features introduced after PHPUnit 4.6.10. If it fails, then we are using newer features and we should modify the composer.json to
~4.8
.FWIW, I am not even sure if 4.6 is required. It could just be that the same thing happened earlier and it got increase in an update, but I don't know when that might have happened and hence using 4.6 as base now (as it is just 10 days old).
Comment #12
dawehnerIMHO we should follow at least the major version in our composer.json file as, there is another level of dependency managment outside of composer.json
People will still build Drupal together how they are used to, so for example, they will see, core ships with 4.8, I'll use feature X, as their module code, might not
add the dependency to 4.8 explicitly. I think this is really likely to happen, so we should better protect against that.
Given that new features are usually introduced in majors, we should at least follow that in our composer.json file.
Certainly +1 to that idea! It would though be great to keep this issue as straightforward as possible as at least according the issue summary is about 4.8.6 to 4.8.7
Comment #13
hussainwebThanks @dawehner. That is certainly an interesting thought.
On thinking more about it, it seems to me that it comes down to using the tool properly. Consider two scenarios:
In both the scenarios above, the problem is averted.
Comment #14
chx CreditAttribution: chx commented> FWIW, I am not even sure if 4.6 is required.
You need 4.6 https://github.com/sebastianbergmann/phpunit/issues/1599
Comment #15
dawehnerWell, I think for contrib this is much less of a problem. Contrib has often a much higher quality than custom modules, but yeah custom modules will rely on that implicit version,
and well, once you then switch over to a composer based workflow, the custom modules might be still in the same repo, at which you then have the issues.
Comment #16
hussainwebThe patch in #10 is green. This means that we don't really need anything more than 4.6, but we should stick to the latest version, of course. I think the patch in #1 is still valid. We should discuss a bit on #12 and #13 before proceeding. I don't think this is very urgent, considering it is only a patch upgrade.
I am not married to either approach but I think the version constraint
~4.6
makes sense as it allows a wider range of components to work with Drupal. If there are significant disadvantages to using that, then we could switch to~4.8
.Comment #17
dawehnerWell, right, this is core, which has certainly a much higher code quality. Personally I'd like to vote on ~4.8
Comment #20
hussainwebThere has been no response here. Also, PHPUnit 5 came out and I am not sure if PHPUnit 4.6 is supported anymore. The website lists 4.8 as supported. The composer.json in this patch now specifies a constraint of
~4.8
.I am still keen to revisit this but I think we have waited long enough. Let's update to 4.8.10. :)
Comment #21
hussainwebOkay, I messed up with that patch in #20. It modified the wrong composer.json file. I am working on a new patch now.
Comment #22
hussainwebHere is the proper patch.
Comment #24
chx CreditAttribution: chx commentedNote we can not update to the latest stable because phpunit 5 simply doesn't support 5.5. (The collaboration is noted.)
Comment #25
dawehnerYeah we have noted that on the update to phpunit 5 issue.
Comment #26
dawehnerThis is a little bit odd
Comment #27
dawehner@hussainweb
Any thoughts about #26?
Comment #28
hussainweb@dawehner: I am not entirely sure. I think that was a composer version mismatch. I upgraded and recreated the patch again. The line had already disappeared due to another issue that got in. Uploading a new patch.
Comment #29
dawehnerAlright, cool
Comment #30
webchickCommitted and pushed to 8.0.x. Thanks!
Also, April 2015? Wonder how this got missed in the last round of updates?
Comment #32
dawehnerThe release in april 2015 was 4.8.0
Comment #33
neclimdulthis is troublesome for at least me. Was calling vendor/bin/phpunit with different php binaries and now they're just outputing a shell script and not running anything. Not sure how we want to proceed.
Comment #34
neclimdulquick follow up fixes it. generating /vendor/bin in windows causes problems it seems. #2580527: Revert vendor/bin/phpunit