Introduce PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION

This is a good idea. I saw this issue a few days ago, and thought 'yes nice'. But now that I am hitting the PHPUnit deprecations in my work to add attributes, it has become 'yes must' !

Are you saying that we just need to set the environment variable PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION to 1 or 0 and the phpunit binary will use that value? I've just looked in run-tests.sh and there is no reference to that variable or the --fail-on-phpunit-deprecation option.

I will try it and see what happens.

No it's a Drupal thing, not PHPUnit.

Drupal's PHPUnit bootstrap script activates DeprecationHandler, that introspects PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION in its ::getConfiguration() method. That method is invoked by PhpUnitTestRunner::runCommand that does this

    // If the deprecation handler bridge is active, we need to fail when there
    // are deprecations that get reported (i.e. not ignored or expected).
    $deprecationConfiguration = DeprecationHandler::getConfiguration();
    if ($deprecationConfiguration !== FALSE) {
      $command[] = '--fail-on-deprecation';
      if ($deprecationConfiguration['failOnPhpunitDeprecation']) {
        $command[] = '--fail-on-phpunit-deprecation';
      }
    }

that is, it adds (or not) --fail-on-phpunit-deprecation to the PHPUnit CLI sub-process spawned by run-tests.sh.

Thanks for that. In #3527579-9: Add #[Group] attributes for Core 11.2 and PHPUnit 11 → I used your suggestion of setting the plain variable PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION: 0 and this does prevent the internal phpunit deprecation log messages. I am also working on making some changes in our downstream test branches to demonstrate the issue.

To implement this, we could just add the new variable to the hidden-variables file and document how to use it. But I think it may be better to add our own differently-named variable that we set, and the users can customise, or enter in the pipeline form, then we convert that to the actual value for PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION: 0 in the script. This will give us the ability to add further options or enhancements in future, if they become available. I will add that to the MR and see how it looks.

I think that setting the variable with the expected default value that will avoid disruption in contrib might be the quickest/easiest in this case.
Core does exactly the same thing https://git.drupalcode.org/project/drupal/-/blob/11.x/.gitlab-ci.yml#L70, so I think this is good for a RTBC.

Otherwise we are blocked on pushing 11.2 as the new default-ref for all contrib.

I was thinking we could create our own variable say _PHPUNIT_DEPRECATIONS which we can allow to have multiple values, to cater for future scenarios, eg. default 0 which would set PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION=0, 1 would set PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION=1 but it could also control other deprecations. If we didn't do that now, are we making it harder to implement in future?

Maybe it is OK to create this real variable anyway, but should it be in .variables.yml or hidden-variables.yml ?

Regardless, this is NW because the variable description needs to be tidied.

I have fixed the variable's description. If this is a blocker for the next release (i.e. updating default-ref) then we can go with this as-is. If we do follow up with the idea of our own variable then the standard one can be moved into the hidden-variables.yml file then.

Back to RTBC if you want to merge and do the release.

Yeah, for now we'd keep it in the "variables" so it's exposed in the Web UI form, but a possible follow-up might change that. I think this should at least help with the disruption.

There are some other issues, like 📌 Exclude node_modules folder from artifacts Active , which might be needed as part of the new release, to give maintainers options in case they run into issues. I know that core is also working on improving things on their end.

Merged. Thanks!