Change SYMFONY_DEPRECATIONS_HELPER back to strict

1. +++ b/core/tests/Drupal/Tests/Component/Serialization/YamlSymfonyTest.php
   @@ -20,6 +20,12 @@ class YamlSymfonyTest extends YamlTestBase {
       * @covers ::decode
       * @dataProvider providerEncodeDecodeTests
   +   *
   +   * @group legacy
   +   * @expectedDeprecation Using the Yaml::PARSE_KEYS_AS_STRINGS flag is deprecated since Symfony 3.4 as it will be removed in 4.0. Quote your keys when they are evaluable instead.
   +   *
   +   * Note the expected deprecation and legacy should be removed when the
   +   * SymfonyYaml::PARSE_KEYS_AS_STRINGS flag is removed from YamlSymfony.
       */
      public function testEncodeDecode($data) {
   

   I guess it's easier to wait for that issue to land and then remove them. The only reason we're not seeing tons of these is because we use the extension on testbot, but anyone running the tests locally or elsewhere would still get them lots of other tests too?

2. +++ b/core/tests/Drupal/Tests/Core/Entity/EntityLinkTest.php
   @@ -120,6 +127,13 @@ public function testLink($entity_label, $link_text, $expected_text, $link_rel =
   +   *
   +   * @group legacy
   +   *
   +   * Note this is only a legacy test because it triggers a call to
   +   * \Drupal\Core\Entity\EntityTypeInterface::getLabelCallback() which is mocked
   +   * and triggers a deprecation error. Remove when ::getLabelCallback() is
   +   * removed.
       */
   

   just commenting on this as a cross-reference for #2949964: Add an EntityOwnerTrait to standardize the base field needed by EntityOwnerInterface as we have a similar case there.

I was about to do a courtesy reroll after #2962736: phpunit.xml.dist has an incorrect instruction on how to disable deprecation errors was committed. However, in #2962736-5: phpunit.xml.dist has an incorrect instruction on how to disable deprecation errors #5 @alexpott mentions:

Eventually we want to swap this back [to a commented line]

+++ b/core/phpunit.xml.dist
@@ -28,8 +28,8 @@
-    <!-- To disable deprecation testing uncomment the next line. -->
-    <env name="SYMFONY_DEPRECATIONS_HELPER" value="weak_vendors"/>
+    <!-- To disable deprecation testing set the value to disabled. -->
+    <env name="SYMFONY_DEPRECATIONS_HELPER" value="strict"/>

In the patch the SYMFONY_DEPRECATIONS_HELPER value is updated to 'strict'.

In line with the comment in#2962736-5, I would like to suggest this environment variable is omitted completely, so its value will default to 'strict'. See ./vendor/symfony/phpunit-bridge/Tests/DeprecationErrorHandler/default.phpt or https://symfony.com/blog/new-in-symfony-2-7-phpunit-bridge :

This behavior can be controlled with the SYMFONY_DEPRECATIONS_HELPER environment variable (default value = strict)

1. +++ b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php
   @@ -139,11 +140,12 @@ public function testBuild() {
        $forum_manager = $this->getMock('Drupal\forum\ForumManagerInterface');
   

   Maybe this should be moved down to the first place it's used now?

2. +++ b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php
   @@ -232,3 +233,17 @@ public function testValidateHasTermsForVocabularyNoAccess() {
   +class TestUrl extends Url {
   

   This seems useful enough to sit outside of FormUninstallValidatorTest.

3. +++ b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php
   @@ -232,3 +233,17 @@ public function testValidateHasTermsForVocabularyNoAccess() {
   +  protected $string;
   ...
   +  public function __construct($string) {
   ...
   +  public function toString($collect_bubbleable_metadata = FALSE) {
   

   Should these have docblocks?

Was hoping to RTBC but found a few doc things that should be fixed.

1. +++ b/core/modules/forum/src/Breadcrumb/ForumBreadcrumbBuilderBase.php
   @@ -59,6 +66,7 @@ public function __construct(EntityManagerInterface $entity_manager, ConfigFactor
        $this->forumManager = $forum_manager;
        $this->setStringTranslation($string_translation);
   +    $this->termStorage = $entity_manager->getStorage('taxonomy_term');
      }
    
   

   I suppose it is very unlikely that anyone subclassed this..

2. +++ b/core/modules/forum/tests/src/Unit/Breadcrumb/ForumListingBreadcrumbBuilderTest.php
   @@ -177,6 +179,8 @@ public function testBuild() {
        );
    
   +    $forum_manager = $this->getMock('Drupal\forum\ForumManagerInterface');
   +
   

   change this to ::class while we move it around? you do it elsewhere..

3. +++ b/core/modules/serialization/src/EntityResolver/UuidResolver.php
   @@ -15,16 +15,15 @@ class UuidResolver implements EntityResolverInterface {
       *
       * @var \Drupal\Core\Entity\EntityManagerInterface
       */
   -  protected $entityManager;
   +  protected $entityRepository;
   

   wrong @var

4. +++ b/core/modules/serialization/src/EntityResolver/UuidResolver.php
   @@ -15,16 +15,15 @@ class UuidResolver implements EntityResolverInterface {
       *
   -   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
   -   *   The entity manager.
   +   * @param \Drupal\Core\Entity\EntityRepositoryInterface $entity_repository
       */
   -  public function __construct(EntityManagerInterface $entity_manager) {
   -    $this->entityManager = $entity_manager;
   +  public function __construct(EntityRepositoryInterface $entity_repository) {
   +    $this->entityRepository = $entity_repository;
      }
   

   missing description

5. +++ b/core/scripts/run-tests.sh
   @@ -826,11 +826,7 @@ function simpletest_script_run_one_test($test_id, $test_class) {
        }
        else {
   -      // Prevent deprecations caused by vendor code calling deprecated code.
   -      // This also prevents mock objects in PHPUnit 6 triggering silenced
   -      // deprecations from breaking the test suite. We should consider changing
   -      // this to 'strict' once PHPUnit 4 is no longer used.
   -      putenv('SYMFONY_DEPRECATIONS_HELPER=weak_vendors');
   +      putenv('SYMFONY_DEPRECATIONS_HELPER=strict');
        }
   

   was wondering if we should also remove this and rely on the default or whatever it is set, but I guess it's fine?

1. Fair enough :)

Looks good now I think!

+1 to RTBC

Re: #24.2 - I only suggested moving it because the usages had been removed, and the only remaining usage was now miles away to it seemed odd. Maybe we should open a follow-up to replace all getMock() usages to use ::class 😉.

+++ b/core/scripts/run-tests.sh
@@ -310,7 +310,12 @@ function simpletest_script_help() {
+              this is not set the value specified in the
+              SYMFONY_DEPRECATIONS_HELPER environment variable, or the value
+              specified in core/phpunit.xml (if it exists), or the default value
+              will used. The default is that any unexpected silenced deprecation
+              error will fail tests.

will be used. Fixed on commit.

Committed c2585a1 and pushed to 8.6.x. Thanks!