Problem/Motivation

Spin off of #3523614: [CI] Collect and report deprecation statistics and details.

The DeprecationHandler class was implemented as a replacement of Symfony's PHPUnit-bridge component, and uses initialization logic that is now outdated by more recent PHPUnit developments.

Proposed resolution

In this issue, we will

  1. convert the DeprecationHandler class into a proper PHPUnit 'extension', using the bootstrap logic introduced in PHPUnit 10
  2. introduce proper extension parameters in the phpunit.xml to configure the extension, and a new environment variable that could override the xml configuration at runtime in CI pipelines
  3. deprecate the legacy SYMFONY_DEPRECATIONS_HELPER variable

The DeprecationHandler stopped working on PHPUnit 13.2; PHPUnit 13.3 will make parts of the handler obsolete, this issue will prepare for its adoption by deprecating SYMFONY_DEPRECATIONS_HELPER.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#8 3589108-nr-bot_l1xzv9xj.txt1.19 KBneeds-review-queue-bot

Issue fork drupal-3589108

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.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes

mondrake opened merge request !15704

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: mondrake » Unassigned
Status: Active » Needs review
Issue tags: -PHPUnit 12
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -run-tests.sh
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [CI] Refactor DeprecationHandler in a standard PHPUnit extension » [CI] Refactor DeprecationHandler to a standard PHPUnit extension
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [CI] Refactor DeprecationHandler to a standard PHPUnit extension » Refactor DeprecationHandler to a standard PHPUnit extension
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3589108-nr-bot_l1xzv9xj.txt1.19 KB

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.

mondrake’s picture

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

rebased

mondrake’s picture

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

We need to more aggressively stop using SYMFONY_DEPRECATION_HELPER, since PHPUnit 13.2 has broken our deprecations ignore approach and 13.3 will ship with a feature (Deprecation Filters) that is not compatible with our own BootstrapErrorHandler any more.

On that.

For more info: PHPUnit 13.2 breaks Drupal's approach for ignoring deprecations

mondrake’s picture

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

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

mondrake
Location 🇮🇹
commented
Issue tags: +PHPUnit 13
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Refactor DeprecationHandler to a standard PHPUnit extension » Refactor DeprecationHandler into a standard PHPUnit extension
mondrake’s picture

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

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

In light of Deprecation Filters #6710 we should probably give a better name to the new environment variable, to make it a better fit for PHPUnit 13 when it comes.

On it.

mondrake’s picture

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

mondrake
Location 🇮🇹
commented
Issue tags: +11.5.0 release priority
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +blocker

This is a blocker for adopting PHPUnit 13.

dcam’s picture

dcam commented
Issue summary: View changes
Status: Needs review » Needs work

So this is about bootstrapping the extension with well-defined configuration that's defined with individual variables in phpunit.xml files, not about implementing Extension::bootstrap(). After studying the PHPUnit docs on extensions I was a little surprised to find that we don't actually do anything in that method. That's alright. I figured it out.

I only found a couple of minor, almost nit-picky grammatical problems in comments. If there's a logic problem in the code or something that could cause larger issues in the test system, then I lack the understanding to anticipate them. After studying the changes in detail they look OK to me.

Testing the new deprecation message is simple. Add <env name="SYMFONY_DEPRECATIONS_HELPER" value="ignoreFile=core/.deprecation-ignore.txt"/> to your phpunit.xml and run a Unit test. The deprecation message will be issued.

I added the new configuration XML to test the extension:

  <extensions>
    <!-- Deprecation handler.
       See core/phpunit.xml.dist for details on the extension configuration.
    -->
    <bootstrap class="Drupal\TestTools\Extension\DeprecationBridge\DeprecationHandler">
      <parameter name="enableProjectIgnores" value="true"/>
      <parameter name="projectIgnoreFile" value="core/.deprecation-ignore.txt"/>
      <parameter name="enableDebugClassLoader" value="true"/>
    </bootstrap>
  </extensions>

Toggling the enableProjectIgnores parameter correctly turns the deprecation handling on and off. While disabled, deprecation warnings like this are issued:

Method "Drupal\Core\Field\FormatterInterface::settingsSummary()" might add "array" as a native return type declaration in the future.

...which is what the deprecation handler is intended to silence. While enabled they do not appear in the test results. Similarly, projectIgnoreFile also works as expected or it wouldn't be able to ignore deprecations. But I mangled it just to test what happens and got an /InvalidArgumentException in response.

I don't know how to test the enableDebugClassLoader though. If someone can enlighten me, then I'll work on it.

I'm setting the status to Needs Work for the minor issues I found.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3486376: Extend Symfony DebugClassLoader to report missing cross-module @return types, +#3523614: [CI] Collect and report deprecation statistics and details

Thanks for the review @dcam.

Yes, a bit of explanation would help here.

Deprecations like Method "Drupal\Core\Field\FormatterInterface::settingsSummary()" might add "array" as a native return type declaration in the future. [...] are triggered by the DebugClassloader, not through the tests themselves. When during a test a class is needed, it gets loaded, the DebugClassloader inspects it and triggers deprecations for missing type declarations. This starts happening as soon as the DebugClassloader is enabled. In Drupal core, since #3486376: Extend Symfony DebugClassLoader to report missing cross-module @return types this happens a lot, 3.5-4 million deprecations are triggered on each pipeline run. For this reason we have the 'deprecation ignore file' that shields that from being reported. We need to enable the DebugClassloader and the ignore file in strict sequence (otherwise we may have deprecations reported as not yet shielded), and as early as possible during the test runner initialization (we want to also shield deprecations that are triggered by test framework extended features during the setups before actual test runs).

Right now in HEAD, this is all initialized in the bootstrap.php file that PHPUnit includes before it orderly bootstraps each extension. So moving the initialization to the extension ::bootstrap() would mean 'losing' on the current logic, because other extensions may be bootstrapped earlier than ours, and if they trigger deprecations we might be missing the shield. So in the MR I kept doing a 'pre-boostrap' initialization of the extension during the bootstrap.php inclusion as we know this will happen first. When we reach PHPUnit 13 (and here why this is important to this now so we can prepare), this pre-boostrap will have to happen during the instantiation of the new DeprecationFilter i.e. separately from the extension bootstrap itself.

FWIW, if we ever get to #3523614: [CI] Collect and report deprecation statistics and details, this issue's parent and where part of the MR here is taken from, you can see that we will have also some logic implemented within ::bootstrap() as that will not be sequence critical. But that part is not relevant to the deprecation ignore logic that is in scope of this issue.

I don't know how to test the enableDebugClassLoader though

You can try by setting enableProjectIgnores to true and enableDebugClassLoader to false on the test that triggered the error in #20. No errors should be reported. Then enableDebugClassLoader to true and you should get the error. Also, you may try to remove the #[IgnoreDeprecations] from a test of your choice that has them, and see with enableProjectIgnores set to false how toggling enableDebugClassLoader would change your results.

I thought it would be useful to have an explicit parameter for enabling/disabling the DebugClassloader (it's not an option now), because I predict in the next couple of Drupal majors contrib will struggle a lot to align to #3486376: Extend Symfony DebugClassLoader to report missing cross-module @return types. Type hint deprecations will mix with 'normal' deprecations, and it may be useful for contrib to just disable the former in some cases to focus on the latter. This option will facilitate that.

mondrake’s picture

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

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

Thank you very much for that explanation. I didn't realize that it was the DebugClassLoader that causes those warnings to be issued. I was under the impression that it was caused by PHPUnit. Now I have more complete understanding.

Per my testing, these are the results I got:

  • With both enableProjectIgnores and enableDebugClassLoader set to FALSE no warnings are issued.
  • With enableProjectIgnores set to FALSE and enableDebugClassLoader to TRUE the warnings are issued.
  • With both enableProjectIgnores and enableDebugClassLoader set to TRUE no warnings are issued.
  • The fourth case of enableProjectIgnores being TRUE and enableDebugClassLoader being FALSE doesn't make much sense, I think. But for the record no warnings are issued.

As I understand it, this is the correct behavior. This one looks good to me.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Re the 4th case, we also want to ignore deprecations that are NOT triggered by the DebugClassloader. That's the 'normal' case actually. ATM in HEAD there are few of them, but during a major's lifecycle these pile up to be cleaned later when main starts deviating from the last minor branch.

Right now, in HEAD only these patterns relate to ignoring 'normal' deprecations

# Drupal 13.
%The "cache.backend.memory" service is deprecated in drupal:11.3.0 and is removed from drupal:13.0.0. Use cache.backend.memory.memory instead. See https://www.drupal.org/node/3546856%
%The "cache.static" service is deprecated in drupal:11.3.0 and is removed from drupal:13.0.0. Use the cache.memory bin instead. See https://www.drupal.org/node/3546856%

but in 11.4.x, a much bigger list is present as 11.4.x has not gone through the cleanup to prepare for 12

# Drupal 12.
%ExpectDeprecationTrait is deprecated in drupal:11\.4\.0%
%The "Drupal\\Core\\Database\\Query\\Select::hasAllTags\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\Core\\Database\\Query\\Select::hasAnyTag\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\Core\\Database\\Query\\SelectExtender::hasAllTags\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\Core\\Database\\Query\\SelectExtender::hasAnyTag\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\Core\\Entity\\Query\\QueryBase::hasAllTags\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\Core\\Entity\\Query\\QueryBase::hasAnyTag\(\)" method will require a new "string \.\.\. \$tags" argument in the next major version of its interface%
%The "Drupal\\workspaces\\WorkspaceManager::setActiveWorkspace\(\)" method will require a new "bool \$persist" argument in the next major version of its interface%
%The "Drupal\\(?:Core\\(?:Action|Field|Condition)|(?:user|action_test|action_bulk_test|user_batch_action_test)\\Plugin\\Action)\\.*::execute\(\)" method will require a new "object\|null \$object" argument in the next major version of its interface "Drupal\\Core\\Executable\\ExecutableInterface"%
%The "(?:Drupal\\Core\\Layout\\LayoutPluginManagerInterface|Drupal\\Core\\Block\\BlockManager)::get(?:Sorted|Grouped)Definitions\(\)" method will require a new "string \$label_key" argument in the next major version of its interface "Drupal\\Component\\Plugin\\CategorizingPluginManagerInterface"%
%.*::getDefaultOperations\(\) will require a new "\Drupal\Core\Cache\CacheableMetadata|null \$cacheability" argument in the next major version of its parent class%
%.*::getOperations\(\) will require a new "\Drupal\Core\Cache\CacheableMetadata|null \$cacheability" argument in the next major version of its interface%
%Class "Drupal\\migrate_drupal\\Plugin\\migrate\\source\\DrupalSqlBase" as extended by "Drupal\\(ban|block|block_content|comment|config_translation|contact|content_translation|field|file|filter|image|language|menu_link_content|migrate_drupal|node|path|responsive_image|search|shortcut|system|taxonomy|update|user)\\[^"]+" is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0%
%Class "Drupal\\migrate_drupal\\Plugin\\migrate\\source\\DrupalSqlBase" as extended by "Drupal\\Tests\\migrate_drupal\\[^"]+" is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0%
%Migrate source plugin "md_empty" used in migration "(d6_upload_field|user_picture_field)" is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0%
%Trait "Drupal\\migrate_drupal\\Plugin\\migrate\\source\\I18nQueryTrait" used by "Drupal\\(block_content|menu_link_content|taxonomy)\\[^"]+" is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0%
%Drupal\\migrate_drupal\\Plugin\\MigrateFieldPluginManager\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533566%
%The "plugin.manager.migrate.field" service is deprecated in drupal:11.3 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533566%
%Drupal\\block\\Plugin\\migrate\\process\\BlockPluginId is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\process\\BlockRegion is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\process\\BlockSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\process\\BlockTheme is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\process\\BlockVisibility is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\process\\RolesLookup is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\FieldType\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement.%
%The "plugin.manager.archiver" service is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3556927%
%Drupal\\Core\\Archiver\\ArchiverManager is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3556927%
%Drupal\\field\\Plugin\\migrate\\process\\ProcessField\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement.%
%Drupal\\field\\Plugin\\migrate\\process\\d6\\FieldFormatterSettingsDefaults is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d6\\FieldInstanceWidgetSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d7\\FieldBundle is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldInstanceDefaults is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldInstanceOptionTranslation is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldInstanceSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldOptionTranslation is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\field\\Plugin\\migrate\\process\\d[67]\\FieldTypeDefaults is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\file\\Plugin\\migrate\\process\\d6\\FieldFile is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\file\\Plugin\\migrate\\process\\d6\\FileUri is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\filter\\Plugin\\migrate\\process\\FilterID is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\filter\\Plugin\\migrate\\process\\FilterSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\filter\\Plugin\\migrate\\process\\d6\\FilterFormatPermission is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\image\\Plugin\\migrate\\process\\d6\\ImageCacheActions is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\language\\Plugin\\migrate\\process\\ContentTranslationEnabledSetting is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\language\\Plugin\\migrate\\process\\LanguageDomains is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\language\\Plugin\\migrate\\process\\LanguageNegotiation is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\language\\Plugin\\migrate\\process\\LanguageTypes is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\link\\Plugin\\migrate\\process\\FieldLink is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\node\\Plugin\\migrate\\process\\d6\\NodeUpdate7008 is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\path\\Plugin\\migrate\\process\\PathSetTranslated is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\responsive_image\\Plugin\\migrate\\process\\ImageStyleMappings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\search\\Plugin\\migrate\\process\\SearchConfigurationRankings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\system\\Plugin\\migrate\\process\\d6\\SystemUpdate7000 is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\taxonomy\\Plugin\\migrate\\process\\TargetBundle is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\user\\Plugin\\migrate\\process\\ConvertTokens is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\user\\Plugin\\migrate\\process\\ProfileFieldSettings is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\user\\Plugin\\migrate\\process\\UserUpdate8002 is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\user\\Plugin\\migrate\\process\\d6\\ProfileFieldOptionTranslation is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\user\\Plugin\\migrate\\process\\d6\\UserUpdate7002 is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533560%
%Drupal\\block\\Plugin\\migrate\\destination\\EntityBlock::import\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\comment\\Plugin\\migrate\\destination\\EntityComment::import\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\migrate\\Plugin\\Derivative\\MigrateEntityComplete\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\migrate\\Plugin\\migrate\\destination\\EntityContentComplete\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\node\\Plugin\\migrate\\destination\\EntityNodeType::import\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\search\\Plugin\\migrate\\destination\\EntitySearchPage::import\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\shortcut\\Plugin\\migrate\\destination\\EntityShortcutSet\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\system\\Plugin\\migrate\\destination\\d7\\ThemeSettings\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%Drupal\\user\\Plugin\\migrate\\destination\\EntityUserRole\(\) is deprecated in drupal:11.3.0 and is removed from drupal:12.0.0. There is no replacement. See https://www.drupal.org/node/3533565%
%The \$long argument to Drupal\\filter\\Plugin\\Filter\\Filter(Align|AutoP|Caption|Html|HtmlEscape|HtmlImageSecure|Url)::tips\(\) is deprecated in drupal:11.4.0 and is removed from drupal:12.0.0. Only short tips are used. See https://www.drupal.org/node/3567879%
mondrake’s picture

mondrake
Location 🇮🇹
commented
Category: Feature request » Task

Given it's on the path to PHPUnit 13, I think this is a task rather than a feature.

catch made their first commit to this issue’s fork.

catch’s picture

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

Committed/pushed to main, thanks!

This needs a backport MR for 11.x, although wondering how much we definitely need a backport here, it might be OK to let things diverge at this point with the test runner.

catch closed merge request !15704

  • catch committed 30a8b5bf on main 
    task: #3589108 Refactor DeprecationHandler into a standard PHPUnit...
mondrake’s picture

mondrake
Location 🇮🇹
commented

I think we need to backport in this case, to give people the chance to react to

@trigger_error("Using the SYMFONY_DEPRECATIONS_HELPER environment variable to configure test runs is deprecated in drupal:11.5.0 and is removed from drupal:12.0.0. See https://www.drupal.org/node/3594014", E_USER_DEPRECATED);

still in D11.

mondrake opened merge request !16308

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Patch (to be ported) » Needs review
dcam’s picture

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

The 11.x diff is nearly identical to the main branch diff. The only differences are the removal property hooks and the change from DrupalDebugClassLoader to DebugClassLoader. As noted above, there is no DrupalDebugClassLoader in D11.

Per my testing, these are the results I got:

  • Before adding the phpunit.xml configuration no deprecation warnings were issued.
  • With both enableProjectIgnores and enableDebugClassLoader set to FALSE normal deprecation warnings were issued such as for deprecated classes or properties.
  • With enableProjectIgnores set to FALSE and enableDebugClassLoader to TRUE additional deprecation warnings about things like "this method may add a return typehint someday" were issued.
  • With both enableProjectIgnores and enableDebugClassLoader set to TRUE no warnings are issued.
  • The fourth case of enableProjectIgnores being TRUE and enableDebugClassLoader no warnings are issued.

So the backport looks good to me.

catch closed merge request !16308

  • catch committed 94e2b1a5 on 11.x 
    task: #3589108 Refactor DeprecationHandler into a standard PHPUnit...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed 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.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Published CR after chaning the target branch.

acbramley’s picture

acbramley commented

I think this has broken phpunit (next major) jobs for contrib projects until the gitlab template is updated https://git.drupalcode.org/project/diff/-/jobs/10935984

mondrake’s picture

mondrake
Location 🇮🇹
commented

https://git.drupalcode.org/project/gitlab_templates/-/work_items/3572422 is the gitlab_templates issue about making necessary adjustments for #39.

fjgarlin’s picture

fjgarlin commented 
 /**
   * Checks legacy SYMFONY_DEPRECATIONS_HELPER env variable is not used.
   */
  #[Before]
  public function checkLegacySymfonyDeprecationHelperEnvVariable(): void {
    if (getenv('SYMFONY_DEPRECATIONS_HELPER') !== FALSE) {
      @trigger_error("Using the SYMFONY_DEPRECATIONS_HELPER environment variable to configure test runs is deprecated in drupal:11.5.0 and is removed from drupal:12.0.0. See https://www.drupal.org/node/3594014", E_USER_DEPRECATED);
    }
  }

Does it really need to trigger an error? It can be just ignored and maybe, optionally, throw a warning, but throwing an error because a variable exists might be too much.

Is there any chance of changing this behaviour?

mondrake’s picture

mondrake
Location 🇮🇹
commented

#41 it's not an error, it's a deprecation (E_USER_DEPRECATED). You can disable failing on deprecations by passing --suppress-deprecations to run-tests.sh command line. But that obviously means that all E_USER_DEPRECATED and E_DEPRECATED "errors" will be skipped.

fjgarlin’s picture

fjgarlin commented

Doh! My bad, I didn't see the E_USER_DEPRECATED part. You are correct. Sorry for the noise.

fjgarlin changed the visibility of the branch 3589108-do-not-trigger-error-on-variable-exists to hidden.

fjgarlin changed the visibility of the branch main to hidden.

acbramley’s picture

acbramley commented

Confirming that adding _PHPUNIT_EXTRA: '--suppress-deprecations' fixes it https://git.drupalcode.org/project/diff/-/jobs/10954987

Status: Fixed » Closed (fixed)

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