NCIOCPL / cgov-digital-platform

The Cancer.gov Digital Communications Platform
GNU General Public License v2.0
11 stars 33 forks source link

PHP 8.1 Deprecation Fixes #3533

Open jfrank-nih opened 1 year ago

jfrank-nih commented 1 year ago

Parent Issue: #3532

The PHP 8.1 upgrade surfaced some deprecated functionality within PHP that we are using. A fair amount of these should go away with Drupal 10 and the upgrade to various packages.

Note: because of some problematic dependencies, the core Drupal tests will always throw deprecation errors. In Drupal 9 they've avoided that by ignoring specific deprecation messages, knowing that in Drupal 10 they will go away. Tests can be run with deprecation listeners turned on in phpunit.xml:

  <listeners>
    <listener class="\Drupal\Tests\Listeners\DrupalListener">
    </listener>
    <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener">
    </listener>
  </listeners>

Even with that turned on there are plenty of deprecations that make it through. For these we will either need to:

With the deprecation listeners turned on, here's what I see:


  18x: UiHelperTrait::drupalPostForm() is deprecated in drupal:9.1.0 and is removed from drupal:10.0.0. Use $this->submitForm() instead. See https://www.drupal.org/node/3168858
    10x in JSOnlyAppModulePluginBuildTest::testBuild from Drupal\Tests\cgov_js_app_module\Functional
    8x in CgovAdobeRenderTest::testCgovAdobe from Drupal\Tests\cgov_adobe\Functional

  15x: Calling Drupal\Tests\UiHelperTrait::drupalPostForm() with $path set to NULL is deprecated in drupal:9.2.0 and the method is removed in drupal:10.0.0. Use $this->submitForm() instead. See https://www.drupal.org/node/3168858
    10x in JSOnlyAppModulePluginBuildTest::testBuild from Drupal\Tests\cgov_js_app_module\Functional
    5x in CgovAdobeRenderTest::testCgovAdobe from Drupal\Tests\cgov_adobe\Functional

  8x: Declaring ::setUp without a void return typehint in Drupal\Tests\app_module\Unit\PathProcessorAppModuleTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    8x in DrupalListener::startTest from Drupal\Tests\Listeners

  6x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_js_app_module\Unit\JsOnlyAppModulePluginTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    6x in DrupalListener::startTest from Drupal\Tests\Listeners

  3x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_cts\Unit\ZipCodeLookupResourceTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    3x in DrupalListener::startTest from Drupal\Tests\Listeners

  3x: The Drupal\Tests\json_data_field\Kernel\JsonDataFieldItemTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    3x in DrupalListener::startTest from Drupal\Tests\Listeners

  2x: AssertLegacyTrait::assertText() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseContains() or $this->assertSession()->pageTextContains() instead. See https://www.drupal.org/node/3129738
    1x in CgovAdobeRenderTest::testCgovAdobe from Drupal\Tests\cgov_adobe\Functional
    1x in JSOnlyAppModulePluginBuildTest::testBuild from Drupal\Tests\cgov_js_app_module\Functional

  2x: The Drupal\Tests\json_data_field\Kernel\JsonDataFieldFormatterTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    2x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModuleEntityTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModulePluginBaseTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModuleReferenceFieldFormatterTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModuleReferenceSelectWidgetTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\app_module\Functional\AppModuleRenderArrayBuilderTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModuleRenderArrayBuilderTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Functional\AppModuleRouteTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\app_module\Kernel\AppPathStorageTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Kernel\AppPathStorageTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\app_module\Kernel\AppPathTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\app_module\Kernel\AppPathTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_adobe\Functional\CgovAdobeRenderTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\cgov_adobe\Functional\CgovAdobeRenderTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\cgov_cts\Functional\ZipCodeLookupResourceTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_js_app_module\Functional\JSOnlyAppModulePluginBuildTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\cgov_js_app_module\Functional\JSOnlyAppModulePluginBuildTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: AssertLegacyTrait::verbose() is deprecated in drupal:9.2.0 and is removed from drupal:10.0.0. Use dump() instead. See https://www.drupal.org/node/3197514
    1x in JSOnlyAppModulePluginBuildTest::testBuild from Drupal\Tests\cgov_js_app_module\Functional

  1x: The Drupal\Tests\json_data_field\Functional\JsonDataFieldFormatterTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\json_data_field\Functional\JsonDataFieldWidgetTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

  1x: The Drupal\Tests\json_data_field\Kernel\JsonDataFieldTextareaWidgetTest::$modules property must be declared protected. See https://www.drupal.org/node/2909426
    1x in DrupalListener::startTest from Drupal\Tests\Listeners

The following items were originally found, but they have since been deleted.

  1x: Return type of NCIOCPL\ClinicalTrialSearch\Model\ModelCommon::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in CTSManagerTest::testGetWithValidNciId from Drupal\Tests\cgov_cts\Unit

  1x: Return type of NCIOCPL\ClinicalTrialSearch\Model\ModelCommon::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in CTSManagerTest::testGetWithValidNciId from Drupal\Tests\cgov_cts\Unit

  1x: Return type of NCIOCPL\ClinicalTrialSearch\Model\ModelCommon::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in CTSManagerTest::testGetWithValidNciId from Drupal\Tests\cgov_cts\Unit

  1x: Return type of NCIOCPL\ClinicalTrialSearch\Model\ModelCommon::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in CTSManagerTest::testGetWithValidNciId from Drupal\Tests\cgov_cts\Unit

  12x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_cts\Unit\CTSManagerTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    12x in DrupalListener::startTest from Drupal\Tests\Listeners

  10x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_cts\Unit\ClinicalTrialsApiProxyTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    10x in DrupalListener::startTest from Drupal\Tests\Listeners

  2x: Declaring ::setUp without a void return typehint in Drupal\Tests\cgov_cts\Unit\CTSViewDetailsBuilderTest is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
    2x in DrupalListener::startTest from Drupal\Tests\Listeners
andyvanavery31 commented 8 months ago

@blairlearn to investigate if this is still an issue.

blairlearn commented 8 months ago

This may still be an issue.

To view deprecation notices:

  1. Edit phpunit.xml and set SYMFONY_DEPRECATIONS_HELPER (line 43) to max[total]=0.
  2. In the container, run blt tests:phpunit:run --no-interaction. NOTE: paratest swallows the deprecation notices

The current deprecation notices are mostly in Drupal\Tests\Listeners\DrupalListener (so, part of Drupal), however there is one in the app_module module's unit tests.

The remaining item, Method "IteratorAggregate::getIterator()" might add "\Traversable" as a native return type is reported in the AppPathManagerUpdatePathDataTest::testGoodEntityNoAlias test, but the deprecation may actually be in the called code.

Remaining self deprecation notices (5)

  2x: Method "Behat\Mink\Element\ElementInterface::getText()" might add "string" as a native return type declaration in the future. Do the same in implementation "Drupal\Tests\DocumentElement" now to avoid errors or add an explicit @return annotation to suppress this message.
    2x in DrupalListener::endTest from Drupal\Tests\Listeners

  2x: Method "Behat\Mink\Element\ElementInterface::waitFor()" might add "mixed" as a native return type declaration in the future. Do the same in implementation "Drupal\Tests\DocumentElement" now to avoid errors or add an explicit @return annotation to suppress this message.
    2x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: Method "IteratorAggregate::getIterator()" might add "\Traversable" as a native return type declaration in the future. Do the same in implementation "Drupal\Core\Entity\ContentEntityBase" now to avoid errors or add an explicit @return annotation to suppress this message.
    1x in AppPathManagerUpdatePathDataTest::testGoodEntityNoAlias from Drupal\Tests\app_module\Unit

Remaining direct deprecation notices (3)

  1x: The "PHPUnit\TextUI\DefaultResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "Drupal\Tests\Listeners\HtmlOutputPrinter".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "Drupal\Tests\Listeners\DrupalListener" class implements "PHPUnit\Framework\TestListener" that is deprecated.
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "Drupal\Tests\Listeners\DrupalListener" class uses "PHPUnit\Framework\TestListenerDefaultImplementation" that is deprecated The `TestListener` interface is deprecated.
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

Other deprecation notices (3)

  1x: The "PHPUnit\Framework\TestCase::addWarning()" method is considered internal This method is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not extend it from "Drupal\Tests\BrowserTestBase".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "PHPUnit\Framework\TestCase::addWarning()" method is considered internal This method is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not extend it from "Drupal\KernelTests\KernelTestBase".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

  1x: The "PHPUnit\Framework\TestCase::addWarning()" method is considered internal This method is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not extend it from "Drupal\Tests\UnitTestCase".
    1x in DrupalListener::endTest from Drupal\Tests\Listeners

 [Acquia\Blt\Robo\Tasks\PhpUnitTask]  Exit code 1  Time 16:02