-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1761, #1613] Updated to Drupal 11.2 and Rector 2. #1827
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
""" WalkthroughThis change updates Drupal core and related development dependencies in Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Composer
participant Drupal
participant StaticTools
Developer->>Composer: Update composer.json dependencies
Composer->>Drupal: Install/upgrade core and dev packages
Composer->>StaticTools: Install/upgrade PHPStan, Rector
Developer->>Drupal: Adjust config and annotations
Drupal->>Drupal: Apply new memory cache parameter
StaticTools->>Drupal: Analyze code with updated rules
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)tests/phpunit/CircleCiConfigTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information tests/phpunit/Drupal/DatabaseSettingsTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information tests/phpunit/Drupal/EnvironmentSettingsTest.phpERROR: Referenced sniff "Drupal" does not exist Run "phpcs --help" for usage information
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (55)
📒 Files selected for processing (19)
🧰 Additional context used🧠 Learnings (3)📓 Common learningscomposer.json (2)Learnt from: CR Learnt from: CR web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php (2)Learnt from: AlexSkrypnyk Learnt from: AlexSkrypnyk 🧬 Code Graph Analysis (3).vortex/tests/bats/_helper.workflow.bash (1)
tests/phpunit/Drupal/DatabaseSettingsTest.php (1)
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (47)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (15)
.vortex/installer/tests/Fixtures/install/_baseline/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SettingsTestCase.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.acquia.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (8)
composer.json(2 hunks)tests/phpunit/Drupal/SettingsTestCase.php(2 hunks)web/modules/custom/ys_base/tests/src/Traits/MockTrait.php(1 hunks)web/sites/default/default.services.yml(1 hunks)web/sites/default/default.settings.php(2 hunks)web/sites/default/includes/providers/settings.acquia.php(1 hunks)web/sites/default/includes/providers/settings.container.php(1 hunks)web/sites/default/includes/providers/settings.lagoon.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
composer.json (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (11)
web/sites/default/default.services.yml (1)
241-244: LGTM! Well-documented memory optimization parameter.The addition of
entity.memory_cache.slotswith a clear explanation of its impact on memory usage versus database requests is a good configuration addition. The default value of 1000 is reasonable for most use cases.web/sites/default/default.settings.php (2)
70-73: LGTM! Clearer database configuration instructions.The updated instructions are more straightforward by removing the @code/@Endcode references and providing clearer guidance for copying and uncommenting the sample database configuration.
315-315: LGTM! Accurate terminology correction.Changing "Update Manager" to "Update Status" is more precise for describing the fallback HTTP functionality for fetching security advisories.
composer.json (2)
16-17: LGTM! Consistent Drupal core version updates.The Drupal core packages are consistently updated from 11.1.8 to 11.2.2 across all core dependencies. This maintains version alignment and brings in the latest features and security updates.
Also applies to: 38-38
41-41: LGTM! Modern static analysis and refactoring tools.The upgrades to PHPStan v2 and Rector v2, along with the explicit
phpstan/phpstandependency, enhance the project's code quality tooling. These major version updates bring improved performance and new features for better code analysis and automated refactoring.Also applies to: 47-47, 49-49
web/sites/default/includes/providers/settings.acquia.php (1)
24-24: LGTM! Appropriate PHPStan suppression for dynamic require.The PHPStan ignore directive is justified here since the Acquia settings file is environment-specific and won't exist in all contexts where static analysis runs. This prevents false positives while maintaining code quality checks elsewhere.
tests/phpunit/Drupal/SettingsTestCase.php (1)
132-132: LGTM! Standardized suppression annotation syntax.The updates to quote the PHPMD warning types in
@SuppressWarningsannotations improve consistency and clarity. The suppressions forPHPMD.ElseExpressionare appropriate given the complex conditional logic in these test utility methods.Also applies to: 311-311
web/modules/custom/ys_base/tests/src/Traits/MockTrait.php (2)
33-34: LGTM! Annotation syntax updated for static analysis compatibility.The suppression annotations have been updated to use quoted warning names, which aligns with newer static analysis tool requirements and improves compatibility with the Rector 2 update.
37-37: Good improvement! Ensures proper array indexing after filtering.The addition of
array_values()ensures the filtered array has consecutive numeric keys, preventing potential issues with gaps in array indexing that could occur whenarray_filter()removes elements.web/sites/default/includes/providers/settings.container.php (1)
15-20: Good refactoring to optimize environment variable access.This change eliminates redundant
getenv()calls by storing the environment variable value in a local variable. This improves performance by reducing system calls and enhances code readability.web/sites/default/includes/providers/settings.lagoon.php (1)
64-68: Excellent refactoring for LAGOON_ROUTES handling.This change perfectly follows the optimization pattern by storing the environment variable value and using it consistently throughout the code block, eliminating redundant
getenv()calls.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1827 +/- ##
===========================================
- Coverage 74.14% 74.02% -0.12%
===========================================
Files 84 84
Lines 4835 4813 -22
Branches 35 35
===========================================
- Hits 3585 3563 -22
Misses 1250 1250 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aaf5f74 to
d1d325e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/sites/default/includes/providers/settings.lagoon.php (1)
35-39: Repeated getenv() call – previous remark still appliesThe condition still invokes
getenv('LAGOON_GIT_BRANCH')twice. Saving the value once before theelseifwould keep the code consistent with the refactor you applied immediately afterwards and avoids the extra syscall.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (48)
.vortex/installer/tests/Fixtures/install/_baseline/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/DatabaseSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SettingsTestCase.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.acquia.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_valkey/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (16)
composer.json(2 hunks)tests/phpunit/CircleCiConfigTest.php(3 hunks)tests/phpunit/Drupal/DatabaseSettingsTest.php(1 hunks)tests/phpunit/Drupal/EnvironmentSettingsTest.php(2 hunks)tests/phpunit/Drupal/SettingsTestCase.php(2 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(6 hunks)web/modules/custom/ys_base/tests/src/Kernel/ExampleTest.php(3 hunks)web/modules/custom/ys_base/tests/src/Traits/MockTrait.php(1 hunks)web/modules/custom/ys_base/tests/src/Unit/ExampleTest.php(3 hunks)web/sites/default/default.services.yml(1 hunks)web/sites/default/default.settings.php(2 hunks)web/sites/default/includes/providers/settings.acquia.php(1 hunks)web/sites/default/includes/providers/settings.container.php(1 hunks)web/sites/default/includes/providers/settings.lagoon.php(2 hunks)web/themes/custom/your_site_theme/tests/src/Kernel/ExampleTest.php(3 hunks)web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
composer.json (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-docs
🔇 Additional comments (17)
web/sites/default/includes/providers/settings.container.php (1)
15-16: Good optimization to reduce redundant getenv() calls.Storing the environment variable in a local variable eliminates redundant
getenv()calls and improves both performance and code readability.Also applies to: 20-20
web/sites/default/default.services.yml (1)
241-244: Well-documented memory cache configuration addition.The new
entity.memory_cache.slotsparameter provides useful control over entity caching with clear documentation about the performance trade-offs. The default value of 1000 is reasonable for most use cases.composer.json (1)
16-17: Systematic dependency updates align with PR objectives.The updates successfully achieve the stated goals:
- Drupal core packages upgraded to 11.2.2
- Static analysis tooling modernized with PHPStan 2.x and Rector 2.x
- New explicit phpstan/phpstan dependency ensures version consistency
Given the retrieved learning that these updates are managed via RenovateBot with compatibility checks, the version selections appear well-vetted.
Also applies to: 38-38, 41-41, 47-47, 49-49
web/sites/default/includes/providers/settings.acquia.php (1)
24-24: Appropriate PHPStan suppression for platform-specific include.The targeted suppression directive properly handles legitimate static analysis warnings for the dynamic Acquia environment settings file include, aligning with the upgraded PHPStan 2.x tooling.
tests/phpunit/Drupal/SettingsTestCase.php (1)
132-132: Proper PHPMD annotation syntax update.The quoted format for PHPMD suppression annotations ensures compatibility with updated static analysis tooling and follows more explicit syntax conventions.
Also applies to: 311-311
web/sites/default/default.settings.php (1)
70-74: Doc-block tweaks look goodComment-only change improves wording and removes the unneeded “copy / uncomment” boilerplate. No further action required.
web/sites/default/includes/providers/settings.lagoon.php (1)
64-69: Good use of a local var for LAGOON_ROUTESCaching the env value once and re-using it improves readability and avoids redundant calls. 👍
web/modules/custom/ys_base/tests/src/Kernel/ExampleTest.php (1)
7-9: Migration to PHP 8 attributes approvedSwitching from doc-block annotations to
#[DataProvider]/#[Group]is correct for PHPUnit ≥ 10 and keeps tests future-proof.tests/phpunit/Drupal/DatabaseSettingsTest.php (1)
7-9: Attribute migration looks correctThe switch to
#[Group]and#[DataProvider]is syntactically accurate and matches PHPUnit 10 conventions.tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
7-9: PHPUnit annotations successfully modernized to PHP 8 attributes.The conversion from docblock annotations (
@group,@dataProvider) to PHP 8 attributes (#[Group],#[DataProvider]) is correctly implemented. This modernization aligns with PHPUnit 10+ standards and improves type safety.Also applies to: 23-23, 29-29
tests/phpunit/CircleCiConfigTest.php (1)
6-7: PHPUnit annotations successfully modernized to PHP 8 attributes.The conversion from docblock annotations to PHP 8 attributes is correctly implemented across the class and both test methods. The modernization maintains all original functionality while improving compatibility with PHPUnit 10+.
Also applies to: 22-22, 49-49, 223-223
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
7-8: Comprehensive PHPUnit annotation modernization successfully completed.All docblock annotations have been correctly converted to PHP 8 attributes across the class and all four test methods. The conversion maintains test functionality while modernizing to PHPUnit 10+ standards.
Also applies to: 19-19, 85-85, 172-172, 340-340, 572-572
web/modules/custom/ys_base/tests/src/Unit/ExampleTest.php (1)
7-8: PHPUnit annotation modernization correctly handles multiple attributes.The conversion successfully handles both single and multiple attribute scenarios, including the
testMultiplicationmethod with multiple#[Group]attributes. All docblock annotations have been properly converted to PHP 8 attributes.Also applies to: 17-17, 23-24, 50-51, 78-80
web/themes/custom/your_site_theme/tests/src/Kernel/ExampleTest.php (1)
7-8: Consistent PHPUnit annotation modernization across kernel tests.The conversion maintains consistency with other test files in the codebase. All docblock annotations have been properly converted to PHP 8 attributes, including proper handling of multiple group attributes on the
testMultiplicationmethod.Also applies to: 17-17, 23-24, 51-52, 80-82
web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php (3)
7-9: Imports look correct for PHPUnit 10 attribute usage
The addedDataProviderandGroupattribute imports are accurate and scoped to the right namespace. No concerns here.
17-18: Confirm PHPUnit ≥ 10.1 in CI to support class-level#[Group]attribute
This attribute is only honoured by PHPUnit 10+. Double-check thatcomposer.jsonnow requires a compatible PHPUnit version and that CI images were updated accordingly.
51-53: Method grouping is clear
Tagging subtraction tests with'unit:subtraction'refines filtering granularity—nice touch.
| * @SuppressWarnings("PHPMD.CyclomaticComplexity") | ||
| * @SuppressWarnings("PHPMD.ElseExpression") | ||
| */ | ||
| protected function prepareMock(string $class, array $methods_map = [], array|bool $args = []): MockObject { | ||
| $methods = array_filter(array_keys($methods_map)); | ||
| $methods = array_values(array_filter(array_keys($methods_map))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling onlyMethods([]) may explode when no methods supplied
MockBuilder::onlyMethods() throws if given an empty array. When $methods_map is empty the current code still calls it, risking an exception. Guard the call:
@@
- $methods = array_values(array_filter(array_keys($methods_map)));
+ $methods = array_values(array_filter(array_keys($methods_map)));
@@
- $mock = $mock->onlyMethods($methods)->getMock();
+ if ($methods) {
+ $mock = $mock->onlyMethods($methods)->getMock();
+ }
+ else {
+ $mock = $mock->getMock();
+ }📝 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.
| * @SuppressWarnings("PHPMD.CyclomaticComplexity") | |
| * @SuppressWarnings("PHPMD.ElseExpression") | |
| */ | |
| protected function prepareMock(string $class, array $methods_map = [], array|bool $args = []): MockObject { | |
| $methods = array_filter(array_keys($methods_map)); | |
| $methods = array_values(array_filter(array_keys($methods_map))); | |
| * @SuppressWarnings("PHPMD.CyclomaticComplexity") | |
| * @SuppressWarnings("PHPMD.ElseExpression") | |
| */ | |
| protected function prepareMock(string $class, array $methods_map = [], array|bool $args = []): MockObject { | |
| $methods = array_values(array_filter(array_keys($methods_map))); | |
| if ($methods) { | |
| $mock = $mock->onlyMethods($methods)->getMock(); | |
| } else { | |
| $mock = $mock->getMock(); | |
| } |
🤖 Prompt for AI Agents
In web/modules/custom/ys_base/tests/src/Traits/MockTrait.php around lines 33 to
37, the code calls MockBuilder::onlyMethods() with an array derived from
$methods_map without checking if it is empty, which can cause an exception. Fix
this by adding a condition to call onlyMethods() only if the array of methods is
not empty, otherwise skip that call to prevent the exception.
| #[DataProvider('dataProviderAdd')] | ||
| #[Group('addition')] | ||
| public function testAdd(int $a, int $b, int $expected, string|null $expectExceptionMessage = NULL): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Optional: use class-constant reference to avoid string typos
DataProvider accepts a callable; using [self::class, 'dataProviderAdd'] (or self::class . '::dataProviderAdd' in PHPUnit 10.5+) makes renames safer than a bare string.
- #[DataProvider('dataProviderAdd')]
+ #[DataProvider([self::class, 'dataProviderAdd'])]📝 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.
| #[DataProvider('dataProviderAdd')] | |
| #[Group('addition')] | |
| public function testAdd(int $a, int $b, int $expected, string|null $expectExceptionMessage = NULL): void { | |
| #[DataProvider([self::class, 'dataProviderAdd'])] | |
| #[Group('addition')] | |
| public function testAdd(int $a, int $b, int $expected, string|null $expectExceptionMessage = NULL): void { |
🤖 Prompt for AI Agents
In web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php around lines
23 to 25, the DataProvider attribute currently uses a string literal
'dataProviderAdd' which is prone to typos and harder to refactor. Change the
DataProvider annotation to use a class-constant callable reference like
[self::class, 'dataProviderAdd'] or self::class . '::dataProviderAdd' (for
PHPUnit 10.5+) to make renaming safer and avoid string typos.
| #[DataProvider('dataProviderMultiplication')] | ||
| #[Group('multiplication')] | ||
| #[Group('skipped')] | ||
| public function testMultiplication(int $a, int $b, int $expected, string|null $expectExceptionMessage = NULL): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider a skip attribute instead of an extra “skipped” group
If these tests are meant to be conditionally disabled, use #[TestDox('…')]; $this->markTestSkipped() at runtime or the @requires/#[Requires] attributes rather than a custom group. Otherwise, leaving the additional group is harmless.
🤖 Prompt for AI Agents
In web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php around lines
80 to 83, replace the custom 'skipped' group attribute with a proper skip
mechanism. Remove the #[Group('skipped')] attribute and instead use
$this->markTestSkipped() inside the test method to conditionally skip tests at
runtime, or apply the appropriate @requires or #[Requires] attributes to declare
skip conditions. This aligns with best practices for skipping tests instead of
using a custom group.
d1d325e to
5e2b9f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (7)
web/sites/default/includes/providers/settings.lagoon.php (1)
35-39: Consider further optimization to eliminate redundant getenv() callWhile this change improves performance by storing the environment variable, the condition on line 35 still calls
getenv('LAGOON_GIT_BRANCH')before storing it. A previous review suggested a more complete optimization to eliminate this redundancy.web/modules/custom/ys_base/tests/src/Unit/ExampleTest.php (2)
23-24: Optional: use callable reference in#[DataProvider]attributes
Passing the provider name as a bare string is legal but typo-prone and fragile during renames. Using an explicit callable ([self::class, 'dataProviderAdd']) makes static analysis happier and avoids silent failures.- #[DataProvider('dataProviderAdd')] + #[DataProvider([self::class, 'dataProviderAdd'])]Apply the same pattern to
dataProviderSubtractanddataProviderMultiplication.Also applies to: 50-51, 78-79
78-81: Prefer proper skip mechanism over sentinel “skipped” group
If these tests are meant to be disabled, mark them with$this->markTestSkipped()or an appropriate#[Requires]/#[Depends]attribute instead of a custom#[Group('skipped')], which may be overlooked by tooling.web/themes/custom/your_site_theme/tests/src/Kernel/ExampleTest.php (2)
23-24: Optional: switch#[DataProvider]argument to callable for safer refactors
Replace the string literals with[self::class, 'methodName']callables to eliminate hard-coded strings and gain IDE/analysis support.
Example:- #[DataProvider('dataProviderAdd')] + #[DataProvider([self::class, 'dataProviderAdd'])]Repeat for the other provider attributes.
Also applies to: 51-52, 80-81
80-83: Replace “skipped” group with an explicit skip instruction
Use PHPUnit’s skip facilities (markTestSkipped(),#[Requires], etc.) rather than a semantic group label that depends on external filtering.web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php (2)
23-24: Data provider string → callable (optional improvement)
Swap the string with a callable array to catch typos at compile time:- #[DataProvider('dataProviderAdd')] + #[DataProvider([self::class, 'dataProviderAdd'])]Apply similarly to the subtraction and multiplication tests.
Also applies to: 51-52, 80-81
80-83: Avoid custom ‘skipped’ group; leverage PHPUnit skip features
Relying on a bespoke group name can lead to unintended executions. UsemarkTestSkipped()or#[Requires]to express the intent unambiguously.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (55)
.vortex/installer/tests/Fixtures/install/_baseline/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/DatabaseSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SettingsTestCase.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/modules/custom/sw_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/_baseline/web/themes/custom/star_wars/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/ciprovider_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_all_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deploy_type_none_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/deps_updates_provider_ci_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/modules/custom/sw_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.services.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/default.settings.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.acquia.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/sites/default/includes/providers/settings.container.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/docroot/themes/custom/star_wars/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_acquia/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/hosting_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Traits/MockTrait.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/modules/custom/the_force_base/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/web/themes/custom/lightsaber/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/provision_database_lagoon/web/sites/default/includes/providers/settings.lagoon.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_valkey/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/tests/src/Functional/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/tests/src/Kernel/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/theme_custom/web/themes/custom/light_saber/tests/src/Unit/ExampleTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/tests/phpunit/CircleCiConfigTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/timezone_circleci/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (18)
composer.json(2 hunks)tests/phpunit/CircleCiConfigTest.php(3 hunks)tests/phpunit/Drupal/DatabaseSettingsTest.php(1 hunks)tests/phpunit/Drupal/EnvironmentSettingsTest.php(2 hunks)tests/phpunit/Drupal/SettingsTestCase.php(2 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(6 hunks)web/modules/custom/ys_base/tests/src/Functional/ExampleTest.php(3 hunks)web/modules/custom/ys_base/tests/src/Kernel/ExampleTest.php(3 hunks)web/modules/custom/ys_base/tests/src/Traits/MockTrait.php(1 hunks)web/modules/custom/ys_base/tests/src/Unit/ExampleTest.php(3 hunks)web/sites/default/default.services.yml(1 hunks)web/sites/default/default.settings.php(2 hunks)web/sites/default/includes/providers/settings.acquia.php(1 hunks)web/sites/default/includes/providers/settings.container.php(1 hunks)web/sites/default/includes/providers/settings.lagoon.php(2 hunks)web/themes/custom/your_site_theme/tests/src/Functional/ExampleTest.php(3 hunks)web/themes/custom/your_site_theme/tests/src/Kernel/ExampleTest.php(3 hunks)web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
composer.json (2)
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Composer dependencies are updated automatically with compatibility checks via RenovateBot
Learnt from: CR
PR: drevops/vortex#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-18T15:00:28.327Z
Learning: Node.js dependencies are updated in theme directories via RenovateBot
web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php (2)
Learnt from: AlexSkrypnyk
PR: #1693
File: .vortex/installer/tests/Fixtures/install/deploy_type_none_gha/.github/workflows/build-test-deploy.yml:1-4
Timestamp: 2025-06-01T08:08:59.071Z
Learning: Test fixture files (typically located in directories containing "fixture" or "test") should not be reviewed for code quality issues as they may intentionally contain incomplete, invalid, or malformed data for testing purposes.
Learnt from: AlexSkrypnyk
PR: #1693
File: .vortex/installer/tests/Fixtures/install/hosting_acquia/docs/deployment.md:6-8
Timestamp: 2025-06-01T08:09:18.993Z
Learning: Do not review files in test fixture directories (e.g., files under paths containing /tests/Fixtures/ or similar test fixture patterns). These files are used for testing purposes and should remain as-is.
🧬 Code Graph Analysis (1)
web/themes/custom/your_site_theme/tests/src/Unit/ExampleTest.php (2)
web/modules/custom/ys_base/tests/src/Unit/ExampleTest.php (3)
DataProvider(23-35)DataProvider(50-62)Group(17-104)web/themes/custom/your_site_theme/tests/src/Unit/YourSiteThemeUnitTestBase.php (1)
YourSiteThemeUnitTestBase(20-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (16)
web/sites/default/default.settings.php (2)
70-73: Documentation improvement looks good.The removal of
@codeand@endcodetags makes the database configuration instructions clearer for developers who need to copy and uncomment the sample code.
315-315: Correct terminology update.Good catch updating "Update Manager" to "Update Status" to match the actual Drupal module name.
web/sites/default/includes/providers/settings.acquia.php (1)
24-24: Appropriate PHPStan suppression directive.The
@phpstan-ignore-next-linedirective is correctly placed to suppress static analysis warnings for the dynamic require statement, which is appropriate since the file path may not exist in all environments.tests/phpunit/Drupal/SettingsTestCase.php (1)
132-132: Standardized PHPMD suppression annotation syntax.Good update to quote the PHPMD rule names in the
@SuppressWarningsannotations, which standardizes the syntax and aligns with best practices.Also applies to: 311-311
web/sites/default/includes/providers/settings.container.php (1)
15-16: Good optimization of environment variable usage.Storing the
VORTEX_LOCALDEV_URLenvironment variable in a local variable eliminates redundantgetenv()calls and improves code readability. This is a solid performance optimization.Also applies to: 20-20
web/sites/default/default.services.yml (1)
241-244: Well-documented new configuration parameter.The
entity.memory_cache.slotsparameter is properly implemented with clear documentation explaining the memory vs. performance trade-off. The default value of 1000 seems reasonable and the comments help administrators understand when they might need to adjust this setting.composer.json (2)
16-17: LGTM: Consistent Drupal core updatesThe Drupal core packages are consistently updated from ~11.1.8 to ~11.2.2 across all three dependencies, which aligns with the PR objectives.
Also applies to: 38-38
41-41: LGTM: Development tool updates align with PR objectivesThe updates to PHPStan (v2.0) and addition of Rector (v2.1) align with the PR objectives. The addition of direct
phpstan/phpstandependency supports the upgradedphpstan-drupalpackage.Also applies to: 47-47, 49-49
web/sites/default/includes/providers/settings.lagoon.php (1)
64-68: LGTM: Clean environment variable optimizationThe refactoring properly stores
getenv('LAGOON_ROUTES')in a variable before use, eliminating redundant calls and improving readability.web/modules/custom/ys_base/tests/src/Kernel/ExampleTest.php (1)
7-8: LGTM: Well-executed PHPUnit modernizationThe conversion from PHPUnit docblock annotations to PHP 8 attributes is properly implemented with correct use statements and syntax. This modernization aligns with PHPUnit 10 best practices and is consistent with similar updates across the codebase.
Also applies to: 17-17, 23-24, 50-51, 78-80
web/modules/custom/ys_base/tests/src/Traits/MockTrait.php (2)
33-34: LGTM: Improved PHPMD suppression annotation formattingThe addition of double quotes around warning names improves consistency with suppression annotation formatting across the codebase.
37-37: Previous review concern remains unaddressedWhile the
array_values()addition improves array handling, the previous review comment identified a potential issue whereonlyMethods([])could throw an exception when given an empty array. This concern has not been addressed in the current changes.The issue occurs on line 50 where
onlyMethods($methods)is called without checking if$methodsis empty.Also applies to: 50-50
tests/phpunit/Drupal/DatabaseSettingsTest.php (1)
7-8: LGTM: Consistent PHPUnit modernizationThe conversion from PHPUnit docblock annotations to PHP 8 attributes is properly implemented, maintaining all test metadata while modernizing to PHPUnit 10 standards. This is consistent with the broader modernization effort across the test suite.
Also applies to: 15-15, 21-21
tests/phpunit/Drupal/EnvironmentSettingsTest.php (1)
7-9: Attribute migration looks goodThe new
usestatements and attribute syntax are correct for PHPUnit 10+.
No functional impact observed.Also applies to: 23-30
tests/phpunit/CircleCiConfigTest.php (1)
5-8: PHPUnit 10 attribute usage is correct
Group/DataProviderimports and attribute invocations match PHPUnit 10’s API.
Nothing else to flag.Also applies to: 22-23, 49-52, 223-226
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
7-9: Consistent modernisation of test metadataAll
GroupandDataProviderattributes are syntactically correct and the imports are in place.
👍Also applies to: 19-21, 85-87, 172-174, 340-342, 572-574
| #[Group('functional:subtraction')] | ||
| public function testSubtraction(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid characters in group name
Group('functional:subtraction') contains a colon. PHPUnit allows only [A-Za-z0-9_-] in group names and will throw InvalidArgumentException.
-#[Group('functional:subtraction')]
+#[Group('functional-subtraction')]Apply the same rename anywhere the colon variant is used to prevent test-suite failures.
📝 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.
| #[Group('functional:subtraction')] | |
| public function testSubtraction(): void { | |
| #[Group('functional-subtraction')] | |
| public function testSubtraction(): void { |
🤖 Prompt for AI Agents
In web/modules/custom/ys_base/tests/src/Functional/ExampleTest.php at lines
47-48, the group name 'functional:subtraction' uses a colon, which is invalid in
PHPUnit group names. Replace the colon with an allowed character such as a
hyphen or underscore (e.g., 'functional-subtraction') and update all occurrences
of this group name in the codebase to prevent test-suite failures.
| #[Group('functional:subtraction')] | ||
| public function testSubtraction(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invalid characters in group name
Same issue as above – replace the colon with an allowed character.
-#[Group('functional:subtraction')]
+#[Group('functional-subtraction')]Failing to adjust will break the functional test run under PHPUnit 10.
📝 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.
| #[Group('functional:subtraction')] | |
| public function testSubtraction(): void { | |
| #[Group('functional-subtraction')] | |
| public function testSubtraction(): void { |
🤖 Prompt for AI Agents
In web/themes/custom/your_site_theme/tests/src/Functional/ExampleTest.php at
lines 47 to 48, the group name 'functional:subtraction' contains an invalid
colon character. Replace the colon with an allowed character such as a hyphen or
underscore (e.g., 'functional-subtraction' or 'functional_subtraction') to
ensure compatibility with PHPUnit 10 and prevent test run failures.
5e2b9f2 to
cd31314
Compare
|
Related: #1831 |
Closes #1761
Closes #1613
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation
Chores
Tests