feat: add MySQL CI workflow and corresponding test classes for improoved database testing.
#61
Conversation
…oved database testing.
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 41 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 (36)
""" WalkthroughThis update introduces dedicated GitHub Actions workflows for MySQL and PostgreSQL database tests, adds corresponding MySQL- and PostgreSQL-specific test classes with explicit connection credentials, and adjusts existing SQLite test namespaces for clarity. The build and mutation testing workflows are updated to use group filtering for test selection. Composer scripts are modified to run mutation tests only for the SQLite group. The base test case is enhanced to support database credentials configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Database Service
participant PHPUnit (DB Group)
participant Codecov
GitHub Actions->>Database Service: Start DB container with credentials
GitHub Actions->>PHPUnit (DB Group): Run tests filtered by group (mysql/pgsql/sqlite)
PHPUnit (DB Group)->>Database Service: Connect using dsn/username/password
PHPUnit (DB Group)-->>GitHub Actions: Report test results
GitHub Actions->>Codecov: Upload coverage report (using secret)
Possibly related PRs
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 134 134
===========================================
Files 4 4
Lines 525 526 +1
===========================================
+ Hits 525 526 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
tests/mysql/NodeAppendTest.php (1)
13-13: Critical: MySQL test class using SQLite DSN.Same issue as
ExtensibilityTest.php- the DSN should be configured for MySQL, not SQLite.tests/mysql/NodeStateTest.php (1)
13-13: Critical: MySQL test class using SQLite DSN.Same issue as other MySQL test files - the DSN should be configured for MySQL, not SQLite.
tests/mysql/NodeInsertTest.php (1)
13-13: Critical: MySQL test class using SQLite DSN.Same issue as other MySQL test files - the DSN should be configured for MySQL, not SQLite.
Run the following script to verify all MySQL test files have the correct DSN configuration:
#!/bin/bash # Description: Check all MySQL test files for incorrect SQLite DSN usage # Expected: All MySQL test files should use MySQL DSN, not SQLite echo "Checking MySQL test files for SQLite DSN usage..." rg -A 2 -B 2 "sqlite::memory:" tests/mysql/ || echo "No SQLite DSN found in MySQL tests (good)" echo -e "\nChecking all MySQL test files DSN configuration..." rg -A 1 "protected.*dsn" tests/mysql/tests/mysql/ExceptionHandlingTest.php (1)
13-13: Same DSN configuration concern as other MySQL testsThis class has the same potential inconsistency between the 'mysql' group and SQLite DSN. Please see the comment in CacheManagementTest.php for the detailed explanation.
tests/mysql/QueryBehaviorTest.php (1)
13-13: Same DSN configuration concern as other MySQL testsThis class has the same potential inconsistency between the 'mysql' group and SQLite DSN. Please see the comment in CacheManagementTest.php for the detailed explanation.
tests/mysql/TreeTraversalTest.php (1)
13-13: Same DSN configuration concern as other MySQL testsThis class has the same potential inconsistency between the 'mysql' group and SQLite DSN. Please see the comment in CacheManagementTest.php for the detailed explanation.
tests/mysql/NodeDeleteTest.php (1)
13-13: DSN inconsistency detected in MySQL test class.Same issue as in other MySQL test classes - the DSN is configured for SQLite (
'sqlite::memory:') but the class is intended for MySQL testing based on its namespace and group annotation.tests/mysql/ValidationAndStructureTest.php (1)
13-13: DSN inconsistency detected in MySQL test class.Same issue as in other MySQL test classes - the DSN is configured for SQLite (
'sqlite::memory:') but the class is intended for MySQL testing based on its namespace and group annotation.
🧹 Nitpick comments (4)
tests/sqlite/NodePrependTest.php (1)
13-14:string|nullunion seems unnecessary here
$dsnis always initialized with a non-null string in this class, so the|nullpart is redundant unless subclasses/introspection rely on setting it tonulllater. Dropping the union clarifies intent:- protected string|null $dsn = 'sqlite::memory:'; + protected string $dsn = 'sqlite::memory:';If other tests override the property with
null, keep the union; otherwise consider tightening the type.tests/sqlite/QueryBehaviorTest.php (1)
13-14: Drop the nullable part of the$dsntypeThe property is initialized immediately (
'sqlite::memory:') and never set tonull, sostring|nullis overly permissive:- protected string|null $dsn = 'sqlite::memory:'; + protected string $dsn = 'sqlite::memory:';Tightening the type eliminates an unnecessary union and clarifies intent.
tests/sqlite/TreeTraversalTest.php (1)
13-13: Minor style tweak: use the?stringshorthand.In PHP 8+,
?string $dsnis equivalent tostring|null $dsnand is idiomatic.- protected string|null $dsn = 'sqlite::memory:'; + protected ?string $dsn = 'sqlite::memory:';tests/sqlite/NodeInsertTest.php (1)
13-14: Avoid repeated DSN literals across test suitesEvery backend-specific test class re-declares the same
protected string|null $dsnvalue.
Consider hoisting this property (or a getter) into a shared base class/trait to reduce duplication and ease future maintenance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.github/workflows/build.yml(1 hunks).github/workflows/ci-mysql.yml(1 hunks)tests/mysql/CacheManagementTest.php(1 hunks)tests/mysql/ExceptionHandlingTest.php(1 hunks)tests/mysql/ExtensibilityTest.php(1 hunks)tests/mysql/NodeAppendTest.php(1 hunks)tests/mysql/NodeDeleteTest.php(1 hunks)tests/mysql/NodeInsertTest.php(1 hunks)tests/mysql/NodePrependTest.php(1 hunks)tests/mysql/NodeStateTest.php(1 hunks)tests/mysql/QueryBehaviorTest.php(1 hunks)tests/mysql/TreeTraversalTest.php(1 hunks)tests/mysql/ValidationAndStructureTest.php(1 hunks)tests/sqlite/CacheManagementTest.php(1 hunks)tests/sqlite/ExceptionHandlingTest.php(1 hunks)tests/sqlite/ExtensibilityTest.php(1 hunks)tests/sqlite/NodeAppendTest.php(1 hunks)tests/sqlite/NodeDeleteTest.php(1 hunks)tests/sqlite/NodeInsertTest.php(1 hunks)tests/sqlite/NodePrependTest.php(1 hunks)tests/sqlite/NodeStateTest.php(1 hunks)tests/sqlite/QueryBehaviorTest.php(1 hunks)tests/sqlite/TreeTraversalTest.php(1 hunks)tests/sqlite/ValidationAndStructureTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.889Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
🧬 Code Graph Analysis (6)
tests/mysql/NodeAppendTest.php (11)
tests/mysql/ExtensibilityTest.php (1)
Group(10-14)tests/mysql/CacheManagementTest.php (1)
Group(10-14)tests/mysql/ExceptionHandlingTest.php (1)
Group(10-14)tests/mysql/NodePrependTest.php (1)
Group(10-14)tests/mysql/NodeDeleteTest.php (1)
Group(10-14)tests/mysql/NodeInsertTest.php (1)
Group(10-14)tests/mysql/TreeTraversalTest.php (1)
Group(10-14)tests/mysql/QueryBehaviorTest.php (1)
Group(10-14)tests/mysql/NodeStateTest.php (1)
Group(10-14)tests/mysql/ValidationAndStructureTest.php (1)
Group(10-14)tests/base/AbstractNodeAppend.php (1)
AbstractNodeAppend(11-443)
tests/mysql/ExceptionHandlingTest.php (1)
tests/base/AbstractExceptionHandling.php (1)
AbstractExceptionHandling(16-483)
tests/mysql/NodePrependTest.php (11)
tests/mysql/ExtensibilityTest.php (1)
Group(10-14)tests/mysql/CacheManagementTest.php (1)
Group(10-14)tests/mysql/NodeAppendTest.php (1)
Group(10-14)tests/mysql/ExceptionHandlingTest.php (1)
Group(10-14)tests/mysql/NodeDeleteTest.php (1)
Group(10-14)tests/mysql/NodeInsertTest.php (1)
Group(10-14)tests/mysql/TreeTraversalTest.php (1)
Group(10-14)tests/mysql/QueryBehaviorTest.php (1)
Group(10-14)tests/mysql/NodeStateTest.php (1)
Group(10-14)tests/mysql/ValidationAndStructureTest.php (1)
Group(10-14)tests/base/AbstractNodePrepend.php (1)
AbstractNodePrepend(10-239)
tests/mysql/TreeTraversalTest.php (1)
tests/base/AbstractTreeTraversal.php (1)
AbstractTreeTraversal(16-207)
tests/mysql/CacheManagementTest.php (12)
tests/mysql/ExtensibilityTest.php (1)
Group(10-14)tests/mysql/NodeAppendTest.php (1)
Group(10-14)tests/mysql/ExceptionHandlingTest.php (1)
Group(10-14)tests/mysql/NodePrependTest.php (1)
Group(10-14)tests/mysql/NodeDeleteTest.php (1)
Group(10-14)tests/mysql/NodeInsertTest.php (1)
Group(10-14)tests/mysql/TreeTraversalTest.php (1)
Group(10-14)tests/mysql/QueryBehaviorTest.php (1)
Group(10-14)tests/mysql/NodeStateTest.php (1)
Group(10-14)tests/sqlite/CacheManagementTest.php (1)
Group(10-14)tests/mysql/ValidationAndStructureTest.php (1)
Group(10-14)tests/base/AbstractCacheManagement.php (1)
AbstractCacheManagement(15-609)
tests/mysql/NodeDeleteTest.php (1)
tests/base/AbstractNodeDelete.php (1)
AbstractNodeDelete(12-163)
🪛 actionlint (1.7.7)
.github/workflows/ci-mysql.yml
10-10: could not read reusable workflow file for "./.github/workflows/phpunit-database.yml": open /home/jailuser/git/.github/workflows/phpunit-database.yml: no such file or directory
(workflow-call)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (14)
tests/sqlite/NodePrependTest.php (1)
5-5: Namespace relocation aligns with DB-specific test segregation – looks good
Moving the SQLite tests under their own namespace mirrors the new MySQL layout and keeps concerns separated.tests/sqlite/QueryBehaviorTest.php (1)
5-5: Namespace alignment looks good – please double-check Composer autoload mapping
yii2\extensions\nestedsets\tests\sqlitematches the new directory layout, so the change is correct.
Just make surecomposer.json(or any custom autoloader) has the corresponding PSR-4 entry so the class is actually discovered during test runs.tests/sqlite/TreeTraversalTest.php (1)
5-5: Confirm PSR-4 autoloading for the new namespace.Moving the class into
yii2\extensions\nestedsets\tests\sqlitemeans Composer’s autoload-dev section (or PHPUnit’s bootstrap) must map that namespace to thetests/sqlite/path. Otherwise the class won’t be discovered when running the suite.Please double-check
composer.jsonor the PHPUnit config and adjust if missing.tests/sqlite/ValidationAndStructureTest.php (1)
5-5: Ensure PSR-4 autoload still resolves this new namespace
yii2\extensions\nestedsets\tests\sqliteshould still be covered by the existing"yii2\\extensions\\nestedsets\\tests\\" : "tests/"mapping incomposer.json.
If the mapping was narrowed or customized elsewhere, PHPUnit will fail to find the class. Please double-check the autoload-dev section and runcomposer dump-autoload -oto verify.tests/sqlite/ExtensibilityTest.php (1)
5-5: Namespace segregation looks good—confirm PSR-4 autoload still matchesThe new
sqlitesub-namespace clarifies database-specific tests. Please double-check thatcomposer.json(autoload-dev) still mapstests/toyii2\\extensions\\nestedsets\\tests\\so PHPUnit can locate this class without extra configuration.tests/sqlite/NodeAppendTest.php (1)
5-5: Consistent namespace refactor—verify autoloader coverageSame note as for
ExtensibilityTest: ensure your PSR-4 path mapping encompasses the newsqlitesub-namespace so discovery ofNodeAppendTestworks out-of-the-box.tests/sqlite/CacheManagementTest.php (1)
5-5: Namespace updated—check composer mappingLooks correct, but confirm that autoload-dev in
composer.jsonstill resolvesyii2\\extensions\\nestedsets\\tests\\sqlite\\*classes to thetests/directory.tests/sqlite/NodeDeleteTest.php (1)
5-5: Namespace move approved—ensure test discoveryThe move to the
sqlitenamespace is sound. Please verify PHPUnit still discoversNodeDeleteTestunder the existing PSR-4 rule.tests/sqlite/ExceptionHandlingTest.php (1)
5-5: Namespace shift OK—double-check autoloadGood alignment with the new structure. As with the other files, confirm the current autoload-dev stanza covers the
sqlitenamespace to prevent test autoload failures.tests/sqlite/NodeStateTest.php (1)
5-5: LGTM: Namespace change improves test organization.The namespace change from
yii2\extensions\nestedsets\teststoyii2\extensions\nestedsets\tests\sqlitecorrectly organizes SQLite-specific tests under a dedicated namespace..github/workflows/build.yml (2)
29-29: LGTM - Proper test group separationCorrectly excludes MySQL tests from the main PHPUnit workflow, ensuring they run in their dedicated MySQL CI workflow.
37-37: LGTM - Consistent exclusion patternProperly excludes MySQL tests from the compatibility workflow as well, maintaining consistency with the main PHPUnit job.
tests/mysql/TreeTraversalTest.php (1)
10-14: Consistent implementation pattern across MySQL test classesThe test class structure is well-implemented and consistent with the other MySQL test classes. The use of final class, proper namespace, and PHPUnit group attributes follows good practices.
.github/workflows/ci-mysql.yml (1)
22-28: MySQL configuration looks good.The MySQL database configuration is properly set up with:
- Multiple MySQL versions for comprehensive testing
- Correct database environment variables
- Appropriate health check configuration
- Proper PHP extensions enabled
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
tests/mysql/QueryBehaviorTest.php (1)
14-15: Fix property declaration order to comply with PHP CS Fixer rules.The PHP CS Fixer is flagging that the
passwordproperty should be declared before theuserproperty to maintain consistent class element ordering.protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; -protected string $user = 'root'; protected string $password = 'root'; +protected string $user = 'root';tests/mysql/NodePrependTest.php (1)
14-15: Fix property declaration order to comply with PHP CS Fixer rules.The PHP CS Fixer is flagging that the
passwordproperty should be declared before theuserproperty to maintain consistent class element ordering.protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; -protected string $user = 'root'; protected string $password = 'root'; +protected string $user = 'root';tests/mysql/TreeTraversalTest.php (1)
14-15: Fix property declaration order to comply with PHP CS Fixer rules.The PHP CS Fixer is flagging that the
passwordproperty should be declared before theuserproperty to maintain consistent class element ordering.protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; -protected string $user = 'root'; protected string $password = 'root'; +protected string $user = 'root';tests/mysql/NodeDeleteTest.php (1)
14-15: Fix property declaration order to comply with PHP CS Fixer rules.The PHP CS Fixer is flagging that the
passwordproperty should be declared before theuserproperty to maintain consistent class element ordering.protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; -protected string $user = 'root'; protected string $password = 'root'; +protected string $user = 'root';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/ci-mysql.yml(1 hunks)tests/TestCase.php(2 hunks)tests/mysql/CacheManagementTest.php(1 hunks)tests/mysql/ExceptionHandlingTest.php(1 hunks)tests/mysql/ExtensibilityTest.php(1 hunks)tests/mysql/NodeAppendTest.php(1 hunks)tests/mysql/NodeDeleteTest.php(1 hunks)tests/mysql/NodeInsertTest.php(1 hunks)tests/mysql/NodePrependTest.php(1 hunks)tests/mysql/NodeStateTest.php(1 hunks)tests/mysql/QueryBehaviorTest.php(1 hunks)tests/mysql/TreeTraversalTest.php(1 hunks)tests/mysql/ValidationAndStructureTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/mysql/ExtensibilityTest.php
- .github/workflows/ci-mysql.yml
- tests/mysql/ValidationAndStructureTest.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.889Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
🧬 Code Graph Analysis (3)
tests/mysql/NodeAppendTest.php (1)
tests/base/AbstractNodeAppend.php (1)
AbstractNodeAppend(11-443)
tests/mysql/NodeStateTest.php (1)
tests/base/AbstractNodeState.php (1)
AbstractNodeState(11-124)
tests/mysql/NodeInsertTest.php (1)
tests/base/AbstractNodeInsert.php (1)
AbstractNodeInsert(10-473)
🪛 GitHub Actions: ecs
tests/mysql/CacheManagementTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/NodePrependTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/TreeTraversalTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/ExceptionHandlingTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/NodeAppendTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/QueryBehaviorTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/NodeStateTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/NodeDeleteTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
tests/mysql/NodeInsertTest.php
[error] 11-11: PhpCsFixer: Class elements order incorrect. 'password' property should be declared before 'user'.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
🔇 Additional comments (6)
tests/TestCase.php (2)
44-45: Well-designed credential configuration support.The addition of
$userand$passwordproperties with empty string defaults is a clean approach that enables MySQL test classes to specify custom credentials while maintaining backward compatibility with existing SQLite tests.
357-358: Proper integration of database credentials.The credentials are correctly integrated into the database connection configuration, allowing child test classes to override the default empty values for their specific database requirements.
tests/mysql/NodeStateTest.php (1)
10-16: AbstractNodeState and MySQL integration verifiedThe abstract base class
tests/base/AbstractNodeState.phpexists and inherits fromTestCase. InTestCase::mockConsoleApplication, the configured$dsn,$user, and$passwordproperties are passed directly to theConnectioncomponent:
NodeStateTestcorrectly overrides
protected string|null $dsn,
protected string $user, and
protected string $password
with the desired MySQL parameters.- When
$this->dsnis not null, it’s used instead of the default SQLite memory DSN.No further changes are needed.
tests/mysql/NodeInsertTest.php (1)
10-16: Consistent MySQL test class implementation.The class follows the established pattern for MySQL test classes - extends the appropriate abstract base class, uses the correct PHPUnit group, and configures MySQL connection parameters appropriately for CI environments.
tests/mysql/CacheManagementTest.php (1)
13-13: MySQL DSN configuration is correct.The DSN correctly uses MySQL connection string
'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'which is appropriate for the MySQL CI workflow. The past review comment about SQLite DSN appears to be outdated as the current implementation properly configures MySQL connectivity.tests/mysql/ExceptionHandlingTest.php (1)
10-16: Consistent MySQL test class implementation.The class properly extends
AbstractExceptionHandling, uses the correct PHPUnit group for MySQL tests, and configures appropriate MySQL connection parameters for CI environments. The implementation follows the established pattern consistently.
…t organization and include `framework-options` for mutation tests in configuration.
…-versions` to `php-version` in configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/mutation.yml (2)
24-26: Quote the value offramework-optionsto avoid YAML parsing edge-casesUnquoted scalars that contain shell metacharacters (
=and--) plus embedded quotes can trip CI linters or future editors. Wrapping the value in quotes makes the intent explicit and eliminates ambiguity.- framework-options: --test-framework-options="--group=sqlite" + framework-options: "--test-framework-options=\"--group=sqlite\""
24-26: Consider DRYing the database group selection
--group=sqliteis now hard-coded here and incomposer.json. Extracting it to a reusable env variable (e.g.DB_GROUP) keeps the setting in one place and prevents future drift if the group name ever changes.Example:
env: DB_GROUP: sqlite ... framework-options: "--test-framework-options=\"--group=${{ env.DB_GROUP }}\""composer.json (1)
54-55: Duplicated long command strings hinder maintainabilityBoth
mutationandmutation-staticscripts repeat the exact Infection CLI flags (threads, MSI thresholds, group). Extracting a shared Composer script or environment variable would reduce duplication:"scripts": { "infection-common": " --threads=4 --ignore-msi-with-no-mutations --only-covered --min-msi=100 --min-covered-msi=100 --test-framework-options=--group=sqlite", "mutation": "./vendor/bin/infection $(composer config scripts.infection-common)", "mutation-static": "./vendor/bin/infection $(composer config scripts.infection-common) --static-analysis-tool=phpstan", ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci-mysql.yml(1 hunks).github/workflows/mutation.yml(1 hunks)composer.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-mysql.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.889Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.3-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (1)
composer.json (1)
54-55: Re-evaluate the--min-msi=100thresholdsRequiring 100 % mutation score on every run may cause legitimate contributions to fail CI for non-functional reasons (e.g. boundary cases hard to cover). Typically projects settle on 80-90 % to balance safety with practicality. Consider relaxing these numbers or allowing gradual improvement.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/pgsql/NodeInsertTest.php (1)
13-15: Database configuration is correct for PostgreSQL testing.The DSN format and connection parameters are properly configured for PostgreSQL. While hardcoded credentials are acceptable for CI testing, consider using environment variables for better security practices if this pattern is used across multiple test files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/build-pgsql.yml(1 hunks)tests/pgsql/CacheManagementTest.php(1 hunks)tests/pgsql/ExceptionHandlingTest.php(1 hunks)tests/pgsql/ExtensibilityTest.php(1 hunks)tests/pgsql/NodeAppendTest.php(1 hunks)tests/pgsql/NodeDeleteTest.php(1 hunks)tests/pgsql/NodeInsertTest.php(1 hunks)tests/pgsql/NodePrependTest.php(1 hunks)tests/pgsql/NodeStateTest.php(1 hunks)tests/pgsql/QueryBehaviorTest.php(1 hunks)tests/pgsql/TreeTraversalTest.php(1 hunks)tests/pgsql/ValidationAndStructureTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- tests/pgsql/NodeAppendTest.php
- tests/pgsql/NodeDeleteTest.php
- tests/pgsql/TreeTraversalTest.php
- tests/pgsql/ExtensibilityTest.php
- tests/pgsql/QueryBehaviorTest.php
- tests/pgsql/ExceptionHandlingTest.php
- tests/pgsql/NodeStateTest.php
- .github/workflows/build-pgsql.yml
- tests/pgsql/CacheManagementTest.php
- tests/pgsql/ValidationAndStructureTest.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.889Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
🧬 Code Graph Analysis (1)
tests/pgsql/NodePrependTest.php (2)
tests/pgsql/NodeStateTest.php (1)
Group(10-16)tests/base/AbstractNodePrepend.php (1)
AbstractNodePrepend(10-239)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.0
- GitHub Check: MySQL tests. / PHP 8.4-mysql-latest
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.4
- GitHub Check: phpstan / PHP 8.4-ubuntu-latest
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-16
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-15
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-17
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-16
🔇 Additional comments (4)
tests/pgsql/NodePrependTest.php (1)
1-16: CI Configuration Verified – LGTMThe PostgreSQL connection parameters in
NodePrependTestexactly match our GitHub Actions CI setup:
- POSTGRES_DB:
yiitest- POSTGRES_USER:
root- POSTGRES_PASSWORD:
root- Port:
5432No changes required—approving the PostgreSQL test class as-is.
tests/pgsql/NodeInsertTest.php (3)
1-4: LGTM! PHP opening and strict types declaration are correctly implemented.The file follows PHP best practices with proper opening tag and strict types declaration.
5-8: Well-structured namespace and imports.The namespace follows the established pattern for PostgreSQL-specific tests, and the imports are clean and necessary.
10-11: Excellent class structure and PHPUnit configuration.The
finalclass declaration and#[Group('pgsql')]attribute are well-implemented for proper test categorization and inheritance control.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/mysql/NodeStateTest.php (1)
13-16: Fix property ordering to comply with coding standards.The pipeline failure indicates that properties should be ordered alphabetically. The
driverNameproperty should be declared before thedsnproperty.- protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; - protected string $driverName = 'mysql'; - protected string $password = 'root'; - protected string $username = 'root'; + protected string $driverName = 'mysql'; + protected string|null $dsn = 'mysql:host=127.0.0.1;dbname=yiitest;charset=utf8mb4'; + protected string $password = 'root'; + protected string $username = 'root';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
tests/TestCase.php(7 hunks)tests/base/AbstractQueryBehavior.php(1 hunks)tests/mysql/CacheManagementTest.php(1 hunks)tests/mysql/ExceptionHandlingTest.php(1 hunks)tests/mysql/ExtensibilityTest.php(1 hunks)tests/mysql/NodeAppendTest.php(1 hunks)tests/mysql/NodeDeleteTest.php(1 hunks)tests/mysql/NodeInsertTest.php(1 hunks)tests/mysql/NodePrependTest.php(1 hunks)tests/mysql/NodeStateTest.php(1 hunks)tests/mysql/QueryBehaviorTest.php(1 hunks)tests/mysql/TreeTraversalTest.php(1 hunks)tests/mysql/ValidationAndStructureTest.php(1 hunks)tests/pgsql/CacheManagementTest.php(1 hunks)tests/pgsql/ExceptionHandlingTest.php(1 hunks)tests/pgsql/ExtensibilityTest.php(1 hunks)tests/pgsql/NodeAppendTest.php(1 hunks)tests/pgsql/NodeDeleteTest.php(1 hunks)tests/pgsql/NodeInsertTest.php(1 hunks)tests/pgsql/NodePrependTest.php(1 hunks)tests/pgsql/NodeStateTest.php(1 hunks)tests/pgsql/QueryBehaviorTest.php(1 hunks)tests/pgsql/TreeTraversalTest.php(1 hunks)tests/pgsql/ValidationAndStructureTest.php(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/pgsql/NodeDeleteTest.php
- tests/pgsql/ExtensibilityTest.php
- tests/pgsql/TreeTraversalTest.php
🚧 Files skipped from review as they are similar to previous changes (18)
- tests/pgsql/ExceptionHandlingTest.php
- tests/pgsql/NodeInsertTest.php
- tests/pgsql/ValidationAndStructureTest.php
- tests/mysql/TreeTraversalTest.php
- tests/pgsql/NodeAppendTest.php
- tests/pgsql/NodePrependTest.php
- tests/mysql/ExtensibilityTest.php
- tests/mysql/NodeInsertTest.php
- tests/mysql/QueryBehaviorTest.php
- tests/pgsql/QueryBehaviorTest.php
- tests/pgsql/NodeStateTest.php
- tests/pgsql/CacheManagementTest.php
- tests/mysql/CacheManagementTest.php
- tests/mysql/ValidationAndStructureTest.php
- tests/mysql/ExceptionHandlingTest.php
- tests/mysql/NodeAppendTest.php
- tests/mysql/NodePrependTest.php
- tests/TestCase.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/nested-sets-behavior#13
File: src/NestedSetsBehavior.php:145-145
Timestamp: 2025-06-29T11:22:41.889Z
Learning: The user terabytesoftw prefers to keep PRs focused and small, implementing enhancements in separate dedicated PRs rather than bundling multiple changes together.
🧬 Code Graph Analysis (2)
tests/base/AbstractQueryBehavior.php (1)
tests/TestCase.php (1)
replaceQuotes(374-399)
tests/mysql/NodeStateTest.php (1)
tests/base/AbstractNodeState.php (1)
AbstractNodeState(11-124)
🪛 GitHub Actions: ecs
tests/mysql/NodeStateTest.php
[error] 10-10: PhpCsFixer: Class property 'driverName' order is incorrect. Fixable by running with '--fix'.
tests/mysql/NodeDeleteTest.php
[error] 10-10: PhpCsFixer: Class property 'driverName' order is incorrect. Fixable by running with '--fix'.
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: phpunit-compatibility / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: MySQL tests. / PHP 8.4-mysql-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.0
- GitHub Check: MySQL tests. / PHP 8.4-mysql-8.4
- GitHub Check: PostgreSQL tests. / PHP 8.4-pgsql-17
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.3-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (2)
tests/base/AbstractQueryBehavior.php (1)
131-131: Excellent improvement for multi-database testing!The change from hardcoded backticks to using
$this->replaceQuotes('[[lft]]')makes this test assertion database-agnostic. This ensures the test will work correctly across all supported database drivers (MySQL, SQLite, PostgreSQL, Oracle, SQL Server) by dynamically applying the appropriate identifier quoting conventions.This perfectly aligns with the PR's goal of improving database testing infrastructure.
tests/mysql/NodeDeleteTest.php (1)
10-17: LGTM: Well-structured MySQL test class implementation.The implementation follows consistent patterns with other database-specific test classes and properly configures MySQL connection parameters for testing.
…r` build workflow.
…oss `MSSQL` test classes.
…th left attribute.
1 participant
Summary by CodeRabbit
New Features
Tests