Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .docker/cli.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
#
# hadolint global ignore=DL3018,SC2174
#
# @see https://hub.docker.com/r/uselagoon/php-8.3-cli-drupal/tags
# @see https://hub.docker.com/r/uselagoon/php-8.4-cli-drupal/tags
# @see https://github.com/uselagoon/lagoon-images/tree/main/images/php-cli-drupal

FROM uselagoon/php-8.3-cli-drupal:26.2.0
FROM uselagoon/php-8.4-cli-drupal:26.2.0

# Add missing variables.
# @todo Remove once https://github.com/uselagoon/lagoon/issues/3121 is resolved.
Expand Down
4 changes: 2 additions & 2 deletions .docker/php.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
#
# hadolint global ignore=DL3018
#
# @see https://hub.docker.com/r/uselagoon/php-8.3-fpm/tags
# @see https://hub.docker.com/r/uselagoon/php-8.4-fpm/tags
# @see https://github.com/uselagoon/lagoon-images/tree/main/images/php-fpm

ARG CLI_IMAGE
# hadolint ignore=DL3006
FROM ${CLI_IMAGE:-cli} AS cli

FROM uselagoon/php-8.3-fpm:26.2.0
FROM uselagoon/php-8.4-fpm:26.2.0

RUN apk add --no-cache tzdata

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
#
# hadolint global ignore=DL3018,SC2174
#
# @see https://hub.docker.com/r/uselagoon/php-8.3-cli-drupal/tags
# @see https://hub.docker.com/r/uselagoon/php-8.4-cli-drupal/tags
# @see https://github.com/uselagoon/lagoon-images/tree/main/images/php-cli-drupal

FROM uselagoon/php-8.3-cli-drupal:__VERSION__
FROM uselagoon/php-8.4-cli-drupal:__VERSION__

# Add missing variables.
# @todo Remove once https://github.com/uselagoon/lagoon/issues/3121 is resolved.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
#
# hadolint global ignore=DL3018
#
# @see https://hub.docker.com/r/uselagoon/php-8.3-fpm/tags
# @see https://hub.docker.com/r/uselagoon/php-8.4-fpm/tags
# @see https://github.com/uselagoon/lagoon-images/tree/main/images/php-fpm

ARG CLI_IMAGE
# hadolint ignore=DL3006
FROM ${CLI_IMAGE:-cli} AS cli

FROM uselagoon/php-8.3-fpm:__VERSION__
FROM uselagoon/php-8.4-fpm:__VERSION__

RUN apk add --no-cache tzdata

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<arg name="parallel" value="10"/>

<!-- Lint code against platform version specified in composer.json key "config.platform.php". -->
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
<exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
cacheDirectory=".phpunit.cache">
<php>
<!-- Set error reporting to E_ALL. -->
<ini name="error_reporting" value="32767"/>
<ini name="error_reporting" value="30719"/>
<!-- Do not limit the amount of memory tests take to run. -->
<ini name="memory_limit" value="-1"/>
<env name="SIMPLETEST_BASE_URL" value="http://nginx:8080"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@
'*/vendor/*',
'*/node_modules/*',
])
// PHP version upgrade sets - modernizes syntax to PHP 8.3.
// Includes all rules from PHP 5.3 through 8.3.
->withPhpSets(php83: TRUE)
// PHP version upgrade sets - modernizes syntax to PHP 8.4.
// Includes all rules from PHP 5.3 through 8.4.
->withPhpSets(php84: TRUE)
// Code quality improvement sets.
->withPreparedSets(
codeQuality: TRUE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,7 @@ protected function setEnvVars(array $vars): void {
protected static function getRealEnvVarsFilteredNoValues(array $prefixes = []): array {
$vars = getenv();

$vars = array_filter(array_keys($vars), static function (string $key) use ($prefixes): bool {
foreach ($prefixes as $prefix) {
if (str_starts_with($key, $prefix)) {
return TRUE;
}
}

return FALSE;
});
$vars = array_filter(array_keys($vars), static fn(string $key): bool => array_any($prefixes, fn($prefix): bool => str_starts_with($key, (string) $prefix)));

return array_fill_keys($vars, NULL);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<rule ref="Drupal"/>
@@ -30,10 +30,10 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@@ -202,7 +202,7 @@
@@ -194,7 +194,7 @@
* Require settings file.
*/
protected function requireSettingsFile(array $pre_settings = [], array $pre_config = []): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

<rule ref="Drupal"/>
@@ -30,10 +30,10 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@@ -202,7 +202,7 @@
@@ -194,7 +194,7 @@
* Require settings file.
*/
protected function requireSettingsFile(array $pre_settings = [], array $pre_config = []): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract class SettingsTestCase extends TestCase {
*/
protected function setEnvVars(array $vars): void {
// Unset the existing environment variable if not set in the test.
@@ -310,7 +308,6 @@
@@ -302,7 +300,6 @@
* @param string $message
* Message to display on failure.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract class SettingsTestCase extends TestCase {
*/
protected function setEnvVars(array $vars): void {
// Unset the existing environment variable if not set in the test.
@@ -310,7 +308,6 @@
@@ -302,7 +300,6 @@
* @param string $message
* Message to display on failure.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
protected function setEnvVars(array $vars): void {
// Unset the existing environment variable if not set in the test.
@@ -310,7 +309,6 @@
@@ -302,7 +301,6 @@
* @param string $message
* Message to display on failure.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
protected function setEnvVars(array $vars): void {
// Unset the existing environment variable if not set in the test.
@@ -310,7 +309,6 @@
@@ -302,7 +301,6 @@
* @param string $message
* Message to display on failure.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<file>web/sites/default/includes</file>
<file>tests</file>
@@ -30,10 +29,6 @@
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
- <exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,12 @@ protected function subtestDockerComposeDrushPhpIni(): void {
'1024M',
'PHP memory_limit should be 1024M after second change'
);
$this->processRun('docker compose exec -T cli php -r "echo E_ALL;"');
$e_all = trim($this->processGet()->getOutput());
$this->cmd(
'docker compose exec -T cli php -r "echo ini_get(\'error_reporting\');"',
'32767',
'PHP error_reporting should be 32767 (E_ALL) from drush.ini'
$e_all,
'PHP error_reporting should be E_ALL from drush.ini'
);
Comment on lines +310 to 316

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Good approach for PHP version-agnostic testing.

Dynamically retrieving E_ALL from the container makes the test resilient across PHP versions where this constant differs (32767 in PHP 8.3 vs 30719 in PHP 8.4).

💡 Minor: use the return value directly

processRun returns a Process object, so you could simplify by using it directly instead of calling processGet():

-    $this->processRun('docker compose exec -T cli php -r "echo E_ALL;"');
-    $e_all = trim($this->processGet()->getOutput());
+    $e_all = trim($this->processRun('docker compose exec -T cli php -r "echo E_ALL;"')->getOutput());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.vortex/tests/phpunit/Traits/Subtests/SubtestDockerComposeTrait.php around
lines 310 - 316, The test currently calls $this->processRun(...) and then
$this->processGet() to fetch output; simplify by using the Process object
returned from processRun directly (capture its return, call getOutput() and trim
it) and pass that trimmed value into $this->cmd(...) as the expected value;
update the code around processRun / processGet usage inside
SubtestDockerComposeTrait (the processRun call that retrieves E_ALL and the
$this->cmd call) to use the returned Process rather than calling processGet().


$this->logSubstep('Restore drush.ini to original state.');
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"license": "proprietary",
"type": "project",
"require": {
"php": ">=8.3",
"php": ">=8.4",
"composer/installers": "^2.3",
"cweagans/composer-patches": "^2.0",
"drupal/admin_toolbar": "^3.6.3",
Expand Down Expand Up @@ -98,7 +98,7 @@
"bump-after-update": true,
"discard-changes": true,
"platform": {
"php": "8.3.30"
"php": "8.4.17"
},
"sort-packages": true
},
Expand Down
2 changes: 1 addition & 1 deletion phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<arg name="parallel" value="10"/>

<!-- Lint code against platform version specified in composer.json key "config.platform.php". -->
<config name="testVersion" value="8.3"/>
<config name="testVersion" value="8.4"/>

<!-- Exclude theme assets. -->
<exclude-pattern>web\/themes\/custom\/.*\/build\/.*</exclude-pattern>
Expand Down
2 changes: 1 addition & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ parameters:

level: 7

phpVersion: 80330
phpVersion: 80417

paths:
- web/modules/custom
Expand Down
2 changes: 1 addition & 1 deletion phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
cacheDirectory=".phpunit.cache">
<php>
<!-- Set error reporting to E_ALL. -->
<ini name="error_reporting" value="32767"/>
<ini name="error_reporting" value="30719"/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🌐 Web query:

In PHP 8.4 with PHPUnit 11 configuration, is the recommended way to represent E_ALL compared to hardcoded numeric values like 30719?

💡 Result:

Yes. In a PHPUnit 11 phpunit.xml, using:

<php>
  <ini name="error_reporting" value="-1"/>
</php>

is the more future-proof way to represent “report everything”: PHP documents -1 as equivalent to E_ALL, and notes that it will also include any new error levels added in future PHP versions (whereas a hardcoded bitmask may not) [1]. PHPUnit’s <ini> is applied via ini_set(), so providing -1 as a string value fits that mechanism cleanly [2].

Hardcoding numeric values like 30719 is not recommended because the numeric value of E_ALL can change between PHP versions (for example, it changed in PHP 8.4) [3].

Sources: [1] PHP manual (error_reporting) [2] PHPUnit 11.5 manual (XML <ini> uses ini_set()) [3] PHP.Watch (PHP 8.4 E_ALL value change)


Use -1 for a version-agnostic error_reporting setting.

Line 31 hardcodes 30719, which represents E_ALL for the current PHP version. However, the numeric value of E_ALL changes between PHP versions (it changed in PHP 8.4), making hardcoded masks brittle. PHP documents -1 as the equivalent to E_ALL that will automatically include any new error levels added in future PHP versions.

♻️ Proposed change
-        <ini name="error_reporting" value="30719"/>
+        <ini name="error_reporting" value="-1"/>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ini name="error_reporting" value="30719"/>
<ini name="error_reporting" value="-1"/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@phpunit.xml` at line 31, The phpunit.xml currently hardcodes error_reporting
as the numeric mask "30719"; update the <ini name="error_reporting" ...> setting
to use the version-agnostic value "-1" so it represents E_ALL across PHP
versions (replace the value attribute for the error_reporting ini entry).

<!-- Do not limit the amount of memory tests take to run. -->
<ini name="memory_limit" value="-1"/>
<env name="SIMPLETEST_BASE_URL" value="http://nginx:8080"/>
Expand Down
6 changes: 3 additions & 3 deletions rector.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@
'*/vendor/*',
'*/node_modules/*',
])
// PHP version upgrade sets - modernizes syntax to PHP 8.3.
// Includes all rules from PHP 5.3 through 8.3.
->withPhpSets(php83: TRUE)
// PHP version upgrade sets - modernizes syntax to PHP 8.4.
// Includes all rules from PHP 5.3 through 8.4.
->withPhpSets(php84: TRUE)
Comment on lines +82 to +84

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

Rector withPhpSets php84 parameter

💡 Result:

To enable the PHP 8.4 upgrade set in Rector, use the named-argument on withPhpSets():

<?php

use Rector\Config\RectorConfig;

return RectorConfig::configure()
    ->withPhpSets(php84: true);

If you already declare your target PHP version in composer.json, you can usually just do:

return RectorConfig::configure()
    ->withPhpSets();

Rector will load the matching PHP sets automatically. [1]

Notes:

  • withPhpSets(...) collects the rules/sets up to the version you enable; withPhpVersion(...) restricts which rules are allowed to run (a rule runs only if its minimum PHP version matches your configured withPhpVersion). [2]
  • If you get an error like “Unknown named parameter php84”, you’re on an older Rector version—upgrade Rector to a version that supports that parameter. [3]

Sources: [1] [2] [3]


Use lowercase true instead of uppercase TRUE to match the documented Rector convention.

The withPhpSets(php84: true) named parameter is correct and enables PHP 8.4 modernization rules. The comment accurately describes the scope as including all rules from PHP 5.3 through 8.4. However, the code uses TRUE (uppercase constant) instead of the lowercase true boolean literal shown in the official Rector documentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@rector.php` around lines 82 - 84, The call to withPhpSets uses the PHP
boolean constant TRUE in the named parameter php84; replace the uppercase TRUE
with the lowercase boolean literal true in the withPhpSets(php84: ...)
invocation to match Rector documentation and code style (search for the
withPhpSets call and the php84 named parameter).

// Code quality improvement sets.
->withPreparedSets(
codeQuality: TRUE,
Expand Down
10 changes: 1 addition & 9 deletions tests/phpunit/Drupal/SettingsTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,7 @@ protected function setEnvVars(array $vars): void {
protected static function getRealEnvVarsFilteredNoValues(array $prefixes = []): array {
$vars = getenv();

$vars = array_filter(array_keys($vars), static function (string $key) use ($prefixes): bool {
foreach ($prefixes as $prefix) {
if (str_starts_with($key, $prefix)) {
return TRUE;
}
}

return FALSE;
});
$vars = array_filter(array_keys($vars), static fn(string $key): bool => array_any($prefixes, fn($prefix): bool => str_starts_with($key, (string) $prefix)));

return array_fill_keys($vars, NULL);
}
Expand Down
Loading