Fix deprecations in tests #390
Conversation
norkunas
added
enhancement
|
Warning Rate limit exceeded@norkunas has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
""" WalkthroughThe changes introduce compatibility for Doctrine ORM's native lazy objects by updating configuration, conditional logic, and test handling. A scheduled CI workflow trigger is added. Deprecation warnings are suppressed, and configuration files are loaded based on PHP version and Doctrine features. Tests are conditionally skipped or configured to match the runtime environment. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Kernel
participant DoctrineORM
participant ConfigLoader
TestRunner->>Kernel: Start test suite
Kernel->>ConfigLoader: Check PHP version & LegacyReflectionFields
alt PHP >= 8.4 and LegacyReflectionFields exists
ConfigLoader->>Kernel: Load config.yaml (native lazy objects)
else
ConfigLoader->>Kernel: Load config_old_proxy.yaml (legacy proxies)
end
Kernel->>DoctrineORM: Initialize ORM with selected config
TestRunner->>TestRunner: Run/skip tests based on environment
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/tests.yml(1 hunks)src/Services/MeilisearchService.php(2 hunks)tests/Integration/AggregatorTest.php(2 hunks)tests/Kernel.php(2 hunks)tests/baseline-ignore(1 hunks)tests/config/config.yaml(1 hunks)tests/config/config_old_proxy.yaml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Tests
tests/Integration/AggregatorTest.php
[warning] 40-40: Test skipped: 'testAggregatorProxyClass' skipped because proxies are not wrapped anymore with lazy native objects.
src/Services/MeilisearchService.php
[warning] 23-23: TooManyPublicMethods: The class MeilisearchService has 11 public methods. Consider refactoring MeilisearchService to keep number of public methods under 10.
[warning] 23-23: ExcessiveClassComplexity: The class MeilisearchService has an overall complexity of 55 which is very high. The configured complexity threshold is 50.
[warning] 23-23: CouplingBetweenObjects: The class MeilisearchService has a coupling between objects value of 15. Consider to reduce the number of dependencies under 13.
[warning] 44-44: LongVariable: Avoid excessively long variable names like $classToSerializerGroup. Keep variable name length under 20.
[warning] 52-52: StaticAccess: Avoid using static access to class '\Symfony\Component\PropertyAccess\PropertyAccess' in method '__construct'.
[warning] 337-337: LongVariable: Avoid excessively long variable names like $searchableEntitiesChunk. Keep variable name length under 20.
[warning] 385-385: StaticAccess: Avoid using static access to class '\Doctrine\ORM\Proxy\DefaultProxyClassNameResolver' in method 'resolveClass'.
[warning] 389-389: StaticAccess: Avoid using static access to class '\Doctrine\Common\Util\ClassUtils' in method 'resolveClass'.
🪛 actionlint (1.7.7)
.github/workflows/tests.yml
11-11: unexpected key "schedule" for "push" section. expected one of "branches", "branches-ignore", "paths", "paths-ignore", "tags", "tags-ignore", "types", "workflows"
(syntax-check)
🪛 Gitleaks (8.26.0)
tests/config/config_old_proxy.yaml
3-3: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (9)
tests/config/config_old_proxy.yaml (1)
1-27: LGTM! Configuration correctly implements legacy proxy support.This configuration file appropriately handles backward compatibility for environments without native lazy objects support. The framework secret is a standard Symfony test key, not a security concern.
tests/config/config.yaml (1)
16-17: LGTM! Correctly enables native lazy objects support.The configuration changes appropriately enable native lazy objects and disable deprecated proxy class auto-generation, aligning with modern Doctrine ORM best practices.
tests/baseline-ignore (1)
4-9: LGTM! Appropriately suppresses transitional deprecation warnings.The added baseline entries correctly suppress deprecation warnings for legacy Doctrine ORM proxy features during the transition to native lazy objects.
tests/Integration/AggregatorTest.php (2)
7-7: LGTM! Correctly imports the class for conditional logic.The import is appropriately added to support the conditional test skipping logic.
39-41: LGTM! Conditional test skip logic is correct.The test is appropriately skipped when native lazy objects are available, as the proxy wrapping behavior being tested is no longer relevant in that environment.
src/Services/MeilisearchService.php (2)
8-8: LGTM! Import added for native lazy objects compatibility.The import is correctly added to support the new conditional check for Doctrine ORM's native lazy objects feature.
378-381: LGTM! Native lazy objects compatibility check implemented correctly.The conditional logic correctly handles the transition to Doctrine ORM's native lazy objects by checking for
LegacyReflectionFieldsexistence. When native lazy objects are available, it appropriately returns$object::classdirectly, bypassing the proxy resolution logic.The hierarchical approach (native lazy objects → ORM v3+ → legacy) ensures backward compatibility while leveraging newer features when available.
tests/Kernel.php (2)
10-10: LGTM! Import added for test environment compatibility.The import is correctly added to support the conditional configuration loading based on native lazy objects availability.
33-37: LGTM! Conditional configuration loading handles Doctrine ORM compatibility correctly.The logic properly handles the transition to native lazy objects by:
- Loading
config.yamlonly when both PHP 8.4+ andLegacyReflectionFieldsare available (native lazy objects supported)- Falling back to
config_old_proxy.yamlfor older PHP versions or when native lazy objects aren't availableThis ensures the test environment uses the appropriate Doctrine ORM configuration matching the runtime capabilities.
|
Ffs, coderabbit commitable suggestion duplicates the whole block in CI |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
41-72: Matrix / exclude list drift
php-version: '8.0'is excluded, yet'8.0'isn’t present in the matrix, making the exclusion a no-op and potentially confusing. Consider dropping the redundant entry (lines 68-69) or adding'8.0'back into the matrix intentionally.Similarly,
'8.4'is listed; ensureshivammathur/setup-phpcan actually install it (currently only nightly builds).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/tests.yml
11-11: key "on" is duplicated in workflow. previously defined at line:3,col:1
(syntax-check)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #390 +/- ##
============================================
+ Coverage 88.37% 88.40% +0.02%
Complexity 1 1
============================================
Files 20 20
Lines 886 888 +2
============================================
+ Hits 783 785 +2
Misses 103 103 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
norkunas
force-pushed
the
ci
branch
6 times, most recently
from
8fd9a02 to
8bdf75b
Compare
norkunas
added
the
skip-changelog
|
bors hates me |
Pull Request
Related issue
Fixes #389
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores