Fix deprecated assert_options() function usage for PHP 8.3

Attempt to pass sniffers

combined

Locally I'm getting following

php -dzend.assertions=1 \
vendor/bin/phpunit -c core/phpunit.xml.dist --colors=always --debug \
core/tests/Drupal/Tests/Core/Access/CsrfAccessCheckTest.php
PHPUnit 9.6.8 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\Core\Access\CsrfAccessCheckTest
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testAccessTokenPass' started
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testAccessTokenPass' ended
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testCsrfTokenInvalid' started
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testCsrfTokenInvalid' ended
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testCsrfTokenMissing' started
Test 'Drupal\Tests\Core\Access\CsrfAccessCheckTest::testCsrfTokenMissing' ended


Time: 00:00.085, Memory: 4.00 MB

OK (3 tests, 9 assertions)

Remaining self deprecation notices (6)

  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: Constant ASSERT_EXCEPTION is deprecated

  1x: Function assert_options() is deprecated

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 (1)

  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

one more attempt, no way to set it runtime

+++ b/core/modules/jsonapi/tests/src/Unit/EventSubscriber/ResourceResponseValidatorTest.php
@@ -87,11 +89,13 @@ public function testDoValidateResponse() {
     // Reset the original assert values.
...
+    // phpcs:ignore Generic.PHP.DeprecatedFunctions.Deprecated
     assert_options(ASSERT_ACTIVE, $assert_active_default);

this test needs rework

/var/www/html/web $ php -r 'ini_set("zend.assertions", 1);'
PHP Warning:  zend.assertions may be completely enabled or disabled only in php.ini in Command line code on line 1

Warning: zend.assertions may be completely enabled or disabled only in php.ini in Command line code on line 1

combo with 🐛 Fix deprecated overloaded function usage in PHP 8.3 Fixed

The CR https://www.drupal.org/node/3105918 → needs update and new CR

+++ b/core/lib/Drupal/Component/Assertion/Handle.php
@@ -21,9 +21,11 @@ class Handle {
   public static function register() {
     // Since we're using exceptions, turn error warnings off.
...
     assert_options(ASSERT_WARNING, FALSE);
...
     // Turn exception throwing on.
...
     assert_options(ASSERT_EXCEPTION, TRUE);

The class is deprecated and no-op with 8.3

According to the file doc-block this file is not API yet #3032787: [META] Start creating the public PHP API of the JSON:API module →

@longwave at slack suggested

the method under test is just

  public function doValidateResponse(Response $response, Request $request) {
    assert($this->validateResponse($response, $request), 'A JSON:API response failed validation (see the logs for details). Please report this in the issue queue on drupal.org');
  }

all we are doing is testing that assert_options() works, no? we should not test PHP features

i guess calling validateResponse() is a side effect of the PHP feature but i am not sure this test is worth keeping

there is code to protect against the require-dev dependency not being present, maybe the test can mock that instead?

Attempt to fix test

The remaining \Drupal\Tests\system\Functional\Entity\EntityFormTest::testValidationHandlers() failure somehow also related to assertion settings for validation but can;t find the reason

fix cs

Re-roll after 🐛 Fix deprecated overloaded function usage in PHP 8.3 Fixed

Still can't get why assertion \Drupal\Core\Entity\ContentEntityBase::preSave() is not working in Drupal\Tests\system\Functional\Entity\EntityFormTest::testValidationHandlers()

Locally the test passed when zend.assertions=1 which is enabled in CI https://dispatcher.drupalci.org/job/drupal_patches/193991/artifact/jenki... (grep for zend.assertions)

This assertion was added in 📌 Follow up for "Adding Assertions to Drupal - Test Tools" Fixed instead of exception, so probably easier to revert the hunk

Fix with TODO to decide

New failures missed in 🐛 Fix deprecated overloaded function usage in PHP 8.3 Fixed
https://dispatcher.drupalci.org/job/drupal_patches/195171/testReport/Sit...

Unsilenced deprecation notices (2)

  2x: Calling ReflectionProperty::setValue() with a single argument is deprecated
    1x in SettingsTest::testGetInstanceReflection from Drupal\Tests\Core\Site
    1x in SettingsTest::testFakeDeprecatedSettings from Drupal\Tests\Core\Site

Tests are green, looking for reviews

Random failure on pgsql could gone after 🐛 TestSiteApplicationTest requires a database despite being a unit test Needs work

+++ b/core/assets/scaffold/files/example.settings.local.php
@@ -32,8 +32,11 @@
-assert_options(ASSERT_ACTIVE, TRUE);
-assert_options(ASSERT_EXCEPTION, TRUE);
+if (PHP_VERSION_ID < 80300) {
+  // phpcs:ignore Generic.PHP.DeprecatedFunctions.Deprecated
+  assert_options(ASSERT_ACTIVE, TRUE);
+  assert_options(ASSERT_EXCEPTION, TRUE);
+}

+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -1058,10 +1058,14 @@ public static function bootEnvironment($app_root = NULL) {
+        if (PHP_VERSION_ID < 80300) {
+          // Web tests are to be conducted with runtime assertions active.
+          // phpcs:ignore Generic.PHP.DeprecatedFunctions.Deprecated
+          assert_options(ASSERT_ACTIVE, TRUE);
+          // Force assertion failures to be thrown as exceptions.
+          // phpcs:ignore Generic.PHP.DeprecatedFunctions.Deprecated
+          assert_options(ASSERT_EXCEPTION, TRUE);
+        }

this change means core no longer can manage assertions

+++ b/core/tests/bootstrap.php
@@ -176,7 +176,10 @@ class_alias('\Drupal\Tests\DocumentElement', '\Behat\Mink\Element\DocumentElemen
 // Runtime assertions. PHPUnit follows the php.ini assert.active setting for
 // runtime assertions. By default this setting is on. Ensure exceptions are
 // thrown if an assert fails.
-assert_options(ASSERT_EXCEPTION, TRUE);
+if (PHP_VERSION_ID < 80300) {
+  // phpcs:ignore Generic.PHP.DeprecatedFunctions.Deprecated
+  assert_options(ASSERT_EXCEPTION, TRUE);
+}

@larowlan pointed that CR probably is enough

The CR should say

- runtime code should have zend.assertions=0 or (-1) as assert.active
- tests which has requirement to catch assertions should add doc-block @requires setting zend.assertions 1

https://wiki.php.net/rfc/assert-string-eval-cleanup#formal_proposal

Deprecate the assert.active INI setting and ASSERT_ACTIVE constant, the zend_assertions INI setting has superseded it for a while.

Existing docs https://www.drupal.org/docs/drupal-apis/runtime-assertions#s-configuring... → needs to update about assert.active

There's similar issue found ✨ Use zend.assertions PHP setting instead of assert.active Closed: duplicate

The issue posted in #22 is focused the idea is to replace assert.active by zend.assertions in the places where it is used (.htaccess and maybe in user.ini if this file gets added to the codebase). I've marked that issue as duplicated, but now I'm not sure because this issue is dealing with something related but not the same,. Should I reopen the other or deal with the .htaccess here as well?

Just a heads up that 📌 Retrieve field type category information in \Drupal\Core\Field\FieldTypePluginManager::getGroupedDefinitions Fixed added another assertion test so that one needs to be updated now as well.

Thank you! On my side just going to unblock CI image build as we decided to move to latest devian stable but it needs new build infra(
#3365510: Create a DrupalCI Environment for PHP 8.3 →

Workaround for #24

Now running PHP 8.3 beta3 in CI

new failure

<pre style="white-space: pre-wrap">&amp;lt;</pre> is identical to <pre style="white-space: pre-wrap"></pre>
Failed asserting that two strings are identical.
Expected :''
Actual   :'&lt;'
<Click to see difference>

 /var/www/html/web/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
 /var/www/html/web/vendor/phpunit/phpunit/src/Framework/Constraint/IsIdentical.php:79
 /var/www/html/web/core/modules/text/tests/src/Kernel/TextSummaryTest.php:248
 /var/www/html/web/core/modules/text/tests/src/Kernel/TextSummaryTest.php:192
 /var/www/html/web/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

PHP 8.2 image using libxml libXML Compiled Version => 2.9.10 but 8.3 using 2.11.4

Locally on PHP 8.1 with libxml 2.9.14 the test case also fails

Filed new issue for that 📌 TextSummaryTest:testLength() fails on some libxml versions Closed: outdated

PHP RC3 CI run https://www.drupal.org/pift-ci-job/2776702 →

+++ b/core/tests/Drupal/Tests/Core/Site/SettingsTest.php
@@ -146,7 +146,7 @@ public function testGetInstanceReflection() {
-    $instance_property->setValue(NULL);
+    $instance_property->setValue(NULL, NULL);

@@ -201,7 +201,7 @@ public function testFakeDeprecatedSettings(array $settings_config, string $setti
-    $instance_property->setValue($deprecated_settings);
+    $instance_property->setValue(NULL, $deprecated_settings);

used to split it out to 📌 Fix deprecated ReflectionProperty::setValue() usage for PHP 8.3 Needs review

Filed CR https://www.drupal.org/node/3391611 → and opened MR to get faster CI

MR also adds daily run of CI using PHP 8.3 - probably it needs to be extracted to separate issue

patch from 26 still applicable until merged 📌 Fix deprecated ReflectionProperty::setValue() usage for PHP 8.3 Needs review (RTBC ATM)

so that's only blocker for compatibility left

Re-roll after 📌 Fix deprecated ReflectionProperty::setValue() usage for PHP 8.3 Needs review

Updated IS with API changes

This change IMO fits in scope as subscribers's methods are not a part of API and the method becomes unused

+++ b/core/modules/jsonapi/src/EventSubscriber/ResourceResponseValidator.php
@@ -105,16 +105,8 @@ public function onResponse(ResponseEvent $event) {
-    $this->doValidateResponse($response, $event->getRequest());
-  }
-
-  /**
-   * Wraps validation in an assert to prevent execution in production.
-   *
-   * @see self::validateResponse
-   */
-  public function doValidateResponse(Response $response, Request $request) {
-    assert($this->validateResponse($response, $request), 'A JSON:API response failed validation (see the logs for details). Report this in the issue queue on drupal.org');
+    // Wraps validation in an assert to prevent execution in production.
+    assert($this->validateResponse($response, $event->getRequest()), 'A JSON:API response failed validation (see the logs for details). Report this in the issue queue on drupal.org');

as the test for the method is removed but maybe better to keep this public method without test?

Left some questions on the MR.

Updated IS with link to docs to update and rerolled

Primary questions according review

- revert assertion added in 10.1 to ContentEntityBase (went in without CR so could be reverted as well) #3081646-37: Follow up for "Adding Assertions to Drupal - Test Tools" →

- should we keep assert_options() in codebase (settings.local.php and Kernel) - I think it needs to removal or rewrite https://git.drupalcode.org/project/drupal/-/merge_requests/4933#note_222712

- addition of daily run could be moved to separate issue

FYI GA of PHP 8.3.0 Nov 23 2023

Added commit with removal of assert_options() - official docs updated already https://www.php.net/manual/en/function.assert-options

probably at https://www.drupal.org/node/2492225 → nothing to change as both assert.active and assert.exception are default to 1 (that's why RFC accepted)

@andypost, thanks for keeping Drupal up to date with PHP!

I review this issue and as often happens catch and/or longwave have addressed things that I notice as well.

I left a suggestion for text change in the MR.

Other than that the change records needs updated as listed.

• The behavior change mentioned at https://git.drupalcode.org/project/drupal/-/merge_requests/4933#note_222796 should be in the CR
• The suggested changes to the CR in #21 have not been made

I left this place the same as it was before 10.1 (where it was commited without change record).

So I reverted this part of commit as explained https://git.drupalcode.org/project/drupal/-/merge_requests/4933#note_223111

The change itself https://git.drupalcode.org/project/drupal/-/commit/e434d61d4cc1fec047a89...

it went in without CR so we can be sure it unused

I went ahead and added https://www.drupal.org/node/3397979 → on the basis that just in case someone runs into this on production, they'd have a chance of finding out why. But the assertion should prevent it from happening in the vast majority of cases so hopefully no-one will have to.

I had one question about a follow-up and applied @quietone's wording suggestion, this looks good to me and happy to commit it if someone else will RTBC it.

All feedback has been addressed, this all looks OK to me now.

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Leaving the documentation updates tag. Also adding to Drupal 10.2.0 release highlights and added a note to the release notes.

Updated CR https://www.drupal.org/node/3105918 → to point new one

On docs page it's not clear how to remove mention from htaccess

Filed follow-up to decide about clean-up of it and update docs there

andypost → closed merge request !4933

Didn't mean to move this back to needs review, back to fixed again!

andypost → closed merge request !4934

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

The change record recommends setting ini_set('zend.assertions', 1); but that not gonna always work.

There is a note about that on php.net.

Note:
If a process is started in production mode, zend.assertions cannot be changed at runtime, since the code for assertions was not generated.
If a process is started in development mode, zend.assertions cannot be set to -1 at runtime.

Not sure what we can do here. Probably just recommend configuring it in php.ini file.

Yes, as link to official docs said, to use assertions you need to enable development mode via pho.ini and then you can disable(0) it via ini_set('zend.assertions', 0); and enable when needed in tests