Problem/Motivation

Closures now displays its source location, see merged PR

Steps to reproduce

Run tests using PHP 8.4

Proposed resolution

Update tests to use separate representation for closures for PHP 8.4

Remaining tasks

- review
- commit

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#25 3477366-nr-bot.txt90 bytesneeds-review-queue-bot
#22 3477366-nr-bot.txt90 bytesneeds-review-queue-bot
#20 3477366-nr-bot.txt90 bytesneeds-review-queue-bot
#15 3477366-nr-bot.txt2.07 MBneeds-review-queue-bot

Issue fork drupal-3477366

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

andypost created an issue. See original summary.

andypost opened merge request !9658

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
Issue tags: +PHP 8.4, +Barcelona2024
andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue tags: +needs backport to 10.x
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Neat trick for BC.

Reviewing the change and conditions for 8.4 make sense.

  • longwave committed 97775e78 on 11.x 
    Issue #3477366 by andypost: Fix closures in tests for PHP 8.4
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 97775e7 and pushed to 11.x. Thanks!

MR doesn't apply to 10.4, moving there for backport.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Patch (to be ported) » Needs review

Re-rolled and adjusted line numbers

andypost opened merge request !9772

andypost’s picture

andypost
he/him
Primary language Russian
commented

MR https://git.drupalcode.org/project/drupal/-/merge_requests/9772

andypost closed merge request !9658

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Seems backport has test failures.

Will keep an eye out

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

It was pod's scheduling issue so retried EntityReferenceWidgetTest - guess random failure

---- Drupal\Tests\media_library\FunctionalJavascript\EntityReferenceWidgetTest ----
Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Exception Other      phpunit-16.xml       0 Drupal\Tests\media_library\Function
    PHPUnit Test failed to complete; Error: PHPUnit 9.6.21 by Sebastian
    Bergmann and contributors.
    
    Testing
    Drupal\Tests\media_library\FunctionalJavascript\EntityReferenceWidgetTest
    .E....                                                              6 / 6
    (100%)
    
    Time: 03:29.276, Memory: 8.00 MB
    
    There was 1 error:
    
    1)
    Drupal\Tests\media_library\FunctionalJavascript\EntityReferenceWidgetTest::testWidget
    WebDriver\Exception\ElementClickIntercepted: element click intercepted:
    Element  is not clickable at point (59, 394). Other element would receive
    the click: 
      (Session info: headless chrome=106.0.5249.103)
      (Driver info: chromedriver=106.0.5249.61
    (511755355844955cd3e264779baf0dd38212a4d0-refs/branch-heads/5249@{#569}),platform=Linux
    5.10.225-213.878.amzn2.x86_64 x86_64)
    
    /builds/issue/drupal-3477366/vendor/lullabot/php-webdriver/lib/WebDriver/Exception.php:198
    /builds/issue/drupal-3477366/vendor/lullabot/php-webdriver/lib/WebDriver/AbstractWebDriver.php:216
    /builds/issue/drupal-3477366/vendor/lullabot/php-webdriver/lib/WebDriver/AbstractWebDriver.php:287
    /builds/issue/drupal-3477366/vendor/lullabot/php-webdriver/lib/WebDriver/Container.php:231
    /builds/issue/drupal-3477366/vendor/lullabot/mink-selenium2-driver/src/Selenium2Driver.php:852
    /builds/issue/drupal-3477366/vendor/lullabot/mink-selenium2-driver/src/Selenium2Driver.php:841
    /builds/issue/drupal-3477366/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:123
    /builds/issue/drupal-3477366/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:210
    /builds/issue/drupal-3477366/core/tests/Drupal/FunctionalJavascriptTests/DrupalSelenium2Driver.php:121
    /builds/issue/drupal-3477366/vendor/behat/mink/src/Element/NodeElement.php:176
    /builds/issue/drupal-3477366/vendor/behat/mink/src/Element/NodeElement.php:186
    /builds/issue/drupal-3477366/vendor/behat/mink/src/Element/TraversableElement.php:119
    /builds/issue/drupal-3477366/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTestBase.php:280
    /builds/issue/drupal-3477366/core/modules/media_library/tests/src/FunctionalJavascript/EntityReferenceWidgetTest.php:221
    /builds/issue/drupal-3477366/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
    
    ERRORS!
    Tests: 6, Assertions: 131, Errors: 1.
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Appears all green now

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3477366-nr-bot.txt2.07 MB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Reviewed & tested by the community

some bot's error but rebased

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Reviewed & tested by the community » Needs work

somehow tests fail for 10.4 as branch tests are not green

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Reviewed & tested by the community

proper rebase

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 10.4.x-dev » 10.5.x-dev

now it's 10.5/10.4

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3477366-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Reviewed & tested by the community

false positive

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3477366-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

andypost opened merge request !10162

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

MR opened for 10.5 for bot as patch applicable

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3477366-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

I am not sure the bot looks at the MR base or issue version, maybe it only tries to apply patches to 11.x.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 10.5.x-dev » 10.4.x-dev
Status: Needs review » Fixed

Patch is identical to 11.x version except for the line numbers in error messages.

Committed and pushed 4b3308017f0 to 10.5.x and 7fda601bd78 to 10.4.x. Thanks!

  • longwave committed 7fda601b on 10.4.x 
    Issue #3477366 by andypost: Fix closures in tests for PHP 8.4

(cherry...

  • longwave committed 4b330801 on 10.5.x 
    Issue #3477366 by andypost: Fix closures in tests for PHP 8.4
andypost’s picture

andypost
he/him
Primary language Russian
commented

Thank you! only 2 blockers now left!

andypost closed merge request !10162

andypost closed merge request !9772

Status: Fixed » Closed (fixed)

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