We are on a specific commit for behat/mink-goutte-driver in composer.json and we were waiting on a release. The release is here. Let's update! We can also update behat/* while we are at it.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is an external library upgrade
Issue priority Critical because we need to move away from a specific commit as discussed in #2400407: [meta] Ensure vendor (PHP) libraries are on latest stable release
Disruption Shouldn't be disruptive
CommentFileSizeAuthor
#2 upgrade_behat_mink_and-2578815-2.patch190.89 KBhussainweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hussainweb created an issue. See original summary.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented
Status: Active » Needs review
FileSize
upgrade_behat_mink_and-2578815-2.patch190.89 KB

Here is the patch.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

In case anyone wants to review the changes. I ran the command after updating the composer file manually. The constraint for behat/mink-goutte-driver is ~1.2.

hw@d8:/var/www/d8task-[git 8.0.x] $ composer update behat/*
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Removing behat/mink (v1.6.1)
  - Installing behat/mink (v1.7.0)
    Loading from cache

  - Removing behat/mink-browserkit-driver (v1.2.0)
  - Installing behat/mink-browserkit-driver (v1.3.0)
    Loading from cache

  - Removing behat/mink-goutte-driver (dev-master cc5ce11)
  - Installing behat/mink-goutte-driver (v1.2.0)
    Loading from cache

Writing lock file
Generating autoload files
> Drupal\Core\Composer\Composer::preAutoloadDump
> Drupal\Core\Composer\Composer::ensureHtaccess
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Green

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

We should change the rules of critical version updates to non dev dependencies IMHO :) Updating those packages though is for sure never a bad idea.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

It's not just that this is a minor version upgrade (which should be critical), but also the fact that we are locked on a specific commit. We discussed earlier that we should not release the RC or final (can't remember which) in this state.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Priority: Critical » Major

Since this is/should be require-dev downgrading to major. We should update the docs for that.

I do think having to run dev releases of vendor libraries is a sign of a more serious issue (for a start that they haven't cut a tag for months).

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

Sure, I don't argue against that, but its just ridiculous as tests are not what you run on production.

I do think having to run dev releases of vendor libraries is a sign of a more serious issue
Well, it was our decision to update to guzzle 6, while guzzle 5 was still supported.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Yes it was, after asking how long it would be supported for and being told "at least a year", on May 27th.

https://twitter.com/mtdowling/status/603603747856621568

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

@catch: I am not sure if I understand you on #7. This update removes the dev version and moves to a stable. It was on a specific commit as we needed a particular fix but it is released now.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@hussainweb I mean we use these libraries only for testing, not at runtime. So there are no release blocking implications of being on a commit hash. I'll commit this later either way.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb at Axelerant commented

That's true, and it makes sense. There should be no release blocking implications but we should keep a sane version identifier available at all times. It would affect how Drupal would be used with other components in a composer workflow.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 74a8fc8 on 8.0.x 
    Issue #2578815 by hussainweb: Upgrade behat/mink and behat/mink-goutte-...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.