Problem/Motivation

There are a few classes meant to test Drupal's integration with symfony/phpunit-bridge that are just dead code once PHPUnit 11 native deprecation reporting, with its own tests upstream, is in place.

Proposed resolution

Remove:

  • Drupal\FunctionalTests\Core\Test\PhpUnitBridgeTest
  • Drupal\KernelTests\Core\Test\PhpUnitBridgeTest
  • Drupal\Tests\Core\Listeners\DrupalStandardsListenerDeprecationTest
  • Drupal\Tests\Core\Test\PhpUnitBridgeTest
  • Drupal\Tests\Core\Test\PhpUnitBridgeIsolatedTest
  • Drupal\Tests\ExpectDeprecationTest
  • Drupal\deprecation_test\Deprecation\DrupalStandardsListenerDeprecatedClass

Remaining tasks

Follow ups

Issue fork drupal-3554872

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

mondrake created an issue. See original summary.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Remove test framework code related to no longer used symfony/phpunit-bridge » [D12] Remove test framework code related to no longer used symfony/phpunit-bridge
Status: Postponed » Active
Issue tags: +Major version only

mondrake opened merge request !14800

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Needs work
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [D12] Remove test framework code related to no longer used symfony/phpunit-bridge » Remove test framework code related to no longer used symfony/phpunit-bridge
Status: Needs work » Needs review
Issue tags: -Major version only
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
dcam’s picture

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

I did my best to review the MR. I checked for lingering references to removed classes and ensured there's no dead code that was only being used by the removed classes. I didn't find anything wrong. Since this mostly consists of complete file deletions there isn't much to say about it. This looks good to me.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: Unassigned » mondrake
Status: Reviewed & tested by the community » Needs work

Mmm we probably need to keep the functional test about deprecation during drupalGet, because that checks some bubbling mechanism that is Drupal’s. On it.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: mondrake » Unassigned
Status: Needs work » Needs review

Moved PhpUnitBridgeTest::testErrorOnSiteUnderTest() to BrowserTestBaseTest::testDeprecationTriggeredInSystemUnderTest() so we can still remove the entire test class.

dcam’s picture

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

The moved test was copied faithfully from its original location. It now includes extra comments in its docblock to explain the purpose of the test, so thank you for doing that. The update to the TestHttpClientMiddleware comment is appropriate, especially considering that the referenced function no longer exists.

andypost’s picture

andypost
he/him
Primary language Russian
commented

pipeline failed, moreover I got random test failure of core/tests/Drupal/FunctionalTests/Core/Test/PhpUnitBridgeTest.php and looks a workaround to exclude <directory>../sites/simpletest</directory>

Ref https://git.drupalcode.org/issue/drupal-3569133/-/jobs/8606742

---- Drupal\FunctionalTests\Core\Test\PhpUnitBridgeTest ----
Status      Duration Info                                                                               
--------------------------------------------------------------------------------------------------------
Error         4.313s testSilencedError                                                               
Pass          4.342s testErrorOnSiteUnderTest                                                        
Failure              *** Process execution output ***                                                
    PHPUnit 11.5.50 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.5.3
    Configuration: /builds/issue/drupal-3569133/core/phpunit.xml.dist
    
    E.                                                                  2 / 2 (100%)
    
    HTML output was generated.
    http://localhost/subdirectory/sites/simpletest/browser_output/Drupal_FunctionalTests_Core_Test_PhpUnitBridgeTest-1-17365941.html
    
    
    Time: 00:09.192, Memory: 8.00 MB
    
    Php Unit Bridge (Drupal\FunctionalTests\Core\Test\PhpUnitBridge)
     ✘ Silenced error
       ┐
       ├ UnexpectedValueException: RecursiveDirectoryIterator::__construct(/builds/issue/drupal-3569133/sites/simpletest/42634236/files/php): Failed to open directory: No such file or directory
       │
       │ /builds/issue/drupal-3569133/core/tests/Drupal/TestTools/ErrorHandler/BootstrapErrorHandler.php:66
       │ /builds/issue/drupal-3569133/core/modules/system/tests/modules/deprecation_test/deprecation_test.module:23
       │ /builds/issue/drupal-3569133/core/tests/Drupal/FunctionalTests/Core/Test/PhpUnitBridgeTest.php:36
       ┴
     ✔ Error on site under test
    
    ERRORS!
    Tests: 2, Assertions: 3, Errors: 1.
mondrake’s picture

mondrake
Location 🇮🇹
commented

#14 I rerun the failing jobs and now it’s all green, likely random fails again.

This MR removes the test you indicate, so not sure what’s going on here @andypost

andypost’s picture

andypost
he/him
Primary language Russian
commented

once again https://git.drupalcode.org/issue/drupal-3569133/-/jobs/8607607

andypost’s picture

andypost
he/him
Primary language Russian
commented

I mean this test is flaky so ++ to removal, but MR needs rebase

  • longwave committed 94bbf469 on 11.x 
    test: #3554872 Remove test framework code related to no longer used...

  • longwave committed bd810962 on main 
    test: #3554872 Remove test framework code related to no longer used...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Rebase wasn't needed, MR cleanly applies to both branches. Thanks for cleaning this up!

Committed and pushed bd810962c6b to main and 94bbf4694c8 to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

longwave closed merge request !14800

andypost’s picture

andypost
he/him
Primary language Russian
commented

@mondrake the same error randomly happens but now in other method https://git.drupalcode.org/issue/drupal-3560672/-/jobs/8643277

andypost’s picture

andypost
he/him
Primary language Russian
commented

@mondrake the same error randomly happens but now in other method https://git.drupalcode.org/issue/drupal-3560672/-/jobs/8643277

mondrake’s picture

mondrake
Location 🇮🇹
commented

@andypost it looks like an attempt to install a deprecated module triggers the deprecation error, it's captured by our handler, it's handed over to PHPUnit, and fails there but not clear why. I think I saw this error independently from the changes here, lately. Is there a separate 'random test failure' issue for this?

andypost’s picture

andypost
he/him
Primary language Russian
commented

I found no issue but yep, I'm facing it each time with deprecations of modules or removal of deprecated modules

Status: Fixed » Closed (fixed)

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