Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center... or if we do then depend on a specific commit.
Drupal core currently depends on various `-dev` versions in its composer.json.
We run into a problem when we run field UI tests which caused by a recent change in behat/mink.
Storytime:
This seems to be a core issue. The `FieldUITestTrait::fieldUIAddNewField()` function calls an `assertRaw()`, and passes a `TranslatableMarkup` to it. `assertRaw()` is a legacy wrapper for `assertSession()->responseContains()`. `responseContains()` calls `stripos()` which encountering an object (which has `__toString()` defined), tries to convert it to `int` for some reason (just php things), and because that's not possible, the test fails.
http://php.net/manual/en/function.stripos.php
If needle is not a string, it is converted to an integer and applied as the ordinal value of a character.
My colleague created a pull request to fix this problem in Mink: https://github.com/minkphp/Mink/pull/760
Comments
Comment #2
mxr576Comment #3
cilefen CreditAttribution: cilefen at Institute for Advanced Study commentedSome sites use https://packagist.org/packages/webflo/drupal-core-strict because of inaccurate constraints.
Comment #4
mxr576Thanks for the tip @cilefen it seems a good workaround but definitely there is problem with should be solved.
If I spin up a new instance by using the drupal/drupal project for running a test site by default I still get the latest commit from master branch of Mink because it does not leverage webflo/drupal-core-strict.
Comment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedPerhaps duplicate of #2948700: Casting $text value to string in responseContains/responseNotContains methods.
Comment #6
mxr576It is not a duplicate, that you added as related is caused by this issue. Maybe I should add a [policy] prefix to this issue's title, because this issue is about do not add dev dependencies to Drupal core carelessly, at least depend on a specific commit.
Comment #7
Mile23Doing a bunch of work wrt this on #2976407: Use drupalci.yml for composer dependency min/max testing - DO NOT COMMIT
We use behat/mink 1.7.x-dev because of #2808085: Pipe char in locators break Mink and Symfony element search
And behat/mink-selenium2-driver 1.3.x-dev because of: #2944291: Upgrade behat/mink-selenium2-driver to 1.3.x-dev with a follow-up to #2947517: Selenium driver: API to get remote file paths
Comment #8
Chi CreditAttribution: Chi commentedI counted only two dev dependencies there. Both under require-dev section.
There is a solid case for using dev dependencies in composer.json. The upstream package may have a critical bug that has been fixed but not released yet. Once the package gets new release we can move back to stable version.
I think we need to figure out how to track changes in upstream (TODOs, followup issues, etc).
Comment #9
Mile23Well there are a two different problems here:
1) Too specific: We generally don't want -dev version constraints, especially in Drupal component dependencies. So if we make a -dev commit to a composer.json file, we should have a follow-up to move it to the next stable release at some point in the future.
2) Not specific enough: We have two -dev version requirements, but at least one of them requires a specific commit that doesn't have a release (as far as I can tell). So if a user says
composer updatethen they'll lose the specific commit from the lock file and tests won't pass. In this instance it's not such a big deal because it's require-dev, but for policy it could be a problem. So the solution would be to add a commit hash where appropriate, with a follow-up issue to make sure we can eventually bump the minimum to the next release.Basically I propose that the policy should be: Commit whatever requirement fits the need, but if it's -dev, also add a follow-up to evaluate and move to stable at a later date.
Comment #10
Chi CreditAttribution: Chi commentedThis sounds good to me.
Comment #11
mxr576I do agree with this, but in that cause instead of adding "foo/bar: dev-master" to the composer.json we should rather add "foo/bar: dev-master#c0mm1tS4a". The first requirement basically equivalent of "hey grab the latest - bleeding edge code - from the dev branch _always_", however we do know for sure that which specific commit we actually need to fix a problem so we should not install the latest code version always.
In case of minkphp/behat the root of this problem is that they are working on the upcoming 2.0.x version in the master branch and they have already introduced BC breaking changes. Using master branch for "next" releases is also common pattern.
Comment #13
greg.1.anderson CreditAttribution: greg.1.anderson at Pantheon commentedThe two components in question still have not made stable releases, despite requests dating back years. Regrettably,
dev-master#shaonly works in top-level composer.json files, and is treated likedev-masteranywhere else.