Problem/Motivation

Splitting from #3055180: [META] Symfony 5 compatibility.

MinkBrowserKitDriver does not yet support Symfony 5, and the PRs have been relatively inactive/unreviewed for over a year.

Issue: https://github.com/minkphp/MinkBrowserKitDriver/issues/139
PR 1: https://github.com/minkphp/MinkBrowserKitDriver/pull/151
PR 2: https://github.com/minkphp/MinkBrowserKitDriver/pull/142

We still have several months before we'll have the Drupal 10 branch opened, so this isn't an urgent issue yet, but it may become urgent as we approach the autumn.

There is a fork, published to packagist, which has PHP 8 compatibility and Symfony 5 support, it may be worth switching to that until the official branch is compatible, at least for patch testing, but possibly during Drupal 10's development if the PRs still haven't been merged.

https://packagist.org/packages/friends-of-behat/mink-browserkit-driver

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Drupal has replaced the behat/mink-browserkit-driver dependency with friends-of-behat/mink-browserkit-driver for PHP 8 and Symfony 5 compatibility.

CommentFileSizeAuthor
#5 3209459-5.patch9.27 KBlongwave
#2 3209459.patch7.48 KBlongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Active » Needs review
FileSize
3209459.patch7.48 KB

I think we should just switch to the fork if the original package is no longer being maintained, we likely need this to get a green run in #3161889: [META] Symfony 6 compatibility as one of the existing tests doesn't like dev releases.

+-----------------------------------------+--------+---------+
| Dev Changes                             | From   | To      |
+-----------------------------------------+--------+---------+
| behat/mink-browserkit-driver            | v1.3.4 | REMOVED |
| friends-of-behat/mink-browserkit-driver | NEW    | v1.5.0  |
+-----------------------------------------+--------+---------+

Status: Needs review » Needs work

The last submitted patch, 2: 3209459.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Tests clean-up (vendor hardening) needs to be fixed

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review
FileSize
3209459-5.patch9.27 KB

Fixed #4.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

I'd like to rtbc but it screaming for Release note snippet in summary or change record

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Title: Workaround MinkBrowserKitDriver Symfony 5 compatibility issues » Switch to friends of behat MinkBrowserKitDriver for PHP 8 and Symfony 5 compatibility

Changing the title to match the proposed solution.

When I opened this I wasn't originally thinking to actually switch to the fork, but friends of behat is relatively well established, and the worst that could happen is it also becomes unmaintained and/or we need to switch back again. So this looks like a good option that will simplify our lives for at least the next year.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Issue summary: View changes

@andypost Good point, updated IS with snippet and added CR: https://www.drupal.org/node/3209701

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Issue summary: View changes
Issue tags: +9.2.0 release notes

I've tweaked the release note mention since the fork page says they'll only merge Symfony compatibility fixes and sync with upstream, it's not a full fork (at least not yet).

Adding the 9.2.0 release notes tag. Briefly discussed this change with @xjm and we're both comfortable making this change in a minor with the release notes mention.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community

Let's get it in!

  • catch committed 031c2a2 on 9.2.x 
    Issue #3209459 by longwave, catch, andypost: Switch to friends of behat...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Any problems we can switch back, but this is pretty trivial and unblocks several things.

Status: Fixed » Closed (fixed)

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