feat: Add mutation testing configuration and update PHPUnit schema location.
#12
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 88 91 +3
===========================================
Files 2 2
Lines 465 467 +2
===========================================
+ Hits 465 467 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new GitHub Actions workflow for mutation testing was introduced, utilizing a reusable workflow and secret management. The Infection mutation testing tool was added as a development dependency with corresponding composer scripts and plugin configuration. The infection configuration file was reorganized and updated, and the PHPUnit XML schema reference was changed to a local file path. Two new tests were added to verify correct left/right attribute shifting after node deletions in nested sets. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Reusable Workflow (php-forge/actions)
participant Infection Tool
Developer->>GitHub Actions: Push or Pull Request (excluding docs/config)
GitHub Actions->>Reusable Workflow (php-forge/actions): Trigger infection.yml workflow<br/>Pass STRYKER_DASHBOARD_API_KEY
Reusable Workflow->>Infection Tool: Run mutation tests with provided config and scripts
Infection Tool-->>Reusable Workflow: Mutation test results
Reusable Workflow-->>GitHub Actions: Report status
Possibly related PRs
Poem
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: 0
🧹 Nitpick comments (2)
composer.json (1)
53-53: Reevaluate 100% mutation score thresholdsEnforcing both --min-msi=100 and --min-covered-msi=100 will require a perfect mutation score on every run, which may be unachievable and risk breaking your CI. Before locking these values in:
• Install dev dependencies and measure your current baseline:
composer install ./vendor/bin/infection --threads=4 --ignore-msi-with-no-mutations --only-covered --dry-run• If your current mutation score is below 100%, consider setting more attainable thresholds (for example, 90–95%) and incrementally raising them over time.
File to review:
- composer.json → scripts.mutation
infection.json5 (1)
14-14: Evaluate the 45-second timeout in infection.json5
- Location: infection.json5 (line 14)
- Current setting:
"timeout": 45A 45 second timeout may be too low on larger test suites or slower CI agents. Please run your local mutation tests using the Infection binary to confirm there are no timeout failures:
./vendor/bin/infection \ --threads=4 \ --ignore-msi-with-no-mutations \ --only-covered \ --verboseIf you encounter timeout errors, consider increasing this value (e.g., to 60 or 120 seconds).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/mutation.yml(1 hunks)composer.json(2 hunks)infection.json5(1 hunks)phpunit.xml.dist(1 hunks)
🔇 Additional comments (7)
phpunit.xml.dist (1)
4-4: Verify PHPUnit schema pathGreat improvement—using the local PHPUnit schema makes your configuration self-contained and supports offline development. Please confirm that, after installing dependencies, the schema file exists at the specified path and matches your PHPUnit version:
composer install ls vendor/phpunit/phpunit/phpunit.xsd vendor/bin/phpunit --versionIf the file lives in a different location or the version differs, update
phpunit.xml.distaccordingly.composer.json (2)
18-18: LGTM: Infection dependency addition.The version constraint
^0.27|^0.29covers recent stable versions of Infection, which is appropriate for a mutation testing setup.
45-45: LGTM: Required plugin allowance for Infection.Adding
infection/extension-installerto allowed plugins is necessary for Infection to function properly with Composer.infection.json5 (2)
2-2: Good addition of schema reference.Adding the
$schemareference enables better IDE support and validation for the Infection configuration file.
9-13: LGTM: Source directory configuration.The source configuration correctly points to the
srcdirectory where the main code is located..github/workflows/mutation.yml (2)
1-17: LGTM: Appropriate trigger configuration.The workflow triggers are well-configured:
- Runs on both pull requests and pushes for comprehensive coverage
- Path ignores appropriately exclude documentation and configuration files that don't affect code quality
21-24: Verify reusable workflow source and secret configurationPlease manually confirm the following before merging:
- The
php-forge/actions/.github/workflows/infection.yml@mainworkflow exists in a trusted, well-maintained repository (verify the repo’s URL, ownership and activity).- The
STRYKER_DASHBOARD_API_KEYsecret is correctly defined under your repository’s Settings → Secrets.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/mutation.yml (1)
1-18: Consider addingworkflow_dispatchfor manual runs and restricting default token scopeA manual trigger is handy when you want to re-run mutation tests after tweaking only workflow files or secrets.
Additionally, GitHub now defaults theGITHUB_TOKENto write scope; explicitly limiting it reduces blast-radius.on: pull_request: paths-ignore: @@ push: paths-ignore: - '.gitattributes' + + # Allow maintainers to start the job ad-hoc + workflow_dispatch: +# Restrict default token permissions (adjust if the reused workflow needs more) +permissions: + contents: read + pull-requests: read
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/mutation.yml(1 hunks)composer.json(2 hunks)infection.json5(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infection.json5
- composer.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (1)
.github/workflows/mutation.yml (1)
20-28: Secret won’t be available on forked PRs – expect false negatives
STRYKER_DASHBOARD_API_KEYis injected for thepull_requestevent, but GitHub withholds repository secrets on workflows triggered from forks.
The job will therefore fail (or the upload step in the reusable workflow will) whenever a contributor opens a PR from a fork.Options:
- Remove the secret for
pull_requestruns and upload results only onpushto protected branches.- Switch to
pull_request_target(with great care for security).- Make the upload step conditional on the secret’s presence (
if: env.STRYKER_DASHBOARD_API_KEY != ''inside the reusable workflow).Please verify the intended behaviour and adjust accordingly.
…ity requirement from `composer.json`.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/mutation.yml (2)
1-17: Avoid duplicating identicalpaths-ignoresections.Both the
pull_requestandpushtriggers declare the samepaths-ignorelist. You can reduce duplication and future maintenance cost by defining the list once and re-using it via YAML anchors/aliases.Example refactor:
paths-ignore: &docs-ignore - 'docs/**' - 'README.md' - 'CHANGELOG.md' - '.gitignore' - '.gitattributes' on: pull_request: paths-ignore: *docs-ignore push: paths-ignore: *docs-ignore
20-27: Consider adding aconcurrencykey to cancel obsolete runs.Mutation testing can be slow; cancelling outdated runs (e.g. when a force-push occurs) saves CI minutes:
concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: truePlace this at the top level of the workflow file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/mutation.yml(1 hunks)composer.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- composer.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
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 (1)
tests/NestedSetsBehaviorTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/NestedSetsBehaviorTest.php (3)
tests/TestCase.php (1)
generateFixtureTree(168-202)tests/support/model/Tree.php (1)
Tree(17-64)src/NestedSetsBehavior.php (2)
isLeaf(719-723)deleteWithChildren(519-541)
🪛 PHPMD (2.15.0)
tests/NestedSetsBehaviorTest.php
1628-1628: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftsLeftRightAttributesCorrectly'. (Clean Code Rules)
(StaticAccess)
1635-1635: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftsLeftRightAttributesCorrectly'. (Clean Code Rules)
(StaticAccess)
1636-1636: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftsLeftRightAttributesCorrectly'. (Clean Code Rules)
(StaticAccess)
1637-1637: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftsLeftRightAttributesCorrectly'. (Clean Code Rules)
(StaticAccess)
1653-1653: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftsLeftRightAttributesCorrectly'. (Clean Code Rules)
(StaticAccess)
1673-1673: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftStartsAtCorrectPosition'. (Clean Code Rules)
(StaticAccess)
1679-1679: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftStartsAtCorrectPosition'. (Clean Code Rules)
(StaticAccess)
1680-1680: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftStartsAtCorrectPosition'. (Clean Code Rules)
(StaticAccess)
1681-1681: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftStartsAtCorrectPosition'. (Clean Code Rules)
(StaticAccess)
1696-1696: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testAfterDeleteShiftStartsAtCorrectPosition'. (Clean Code Rules)
(StaticAccess)
1724-1724: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testDeleteWithChildrenShiftsCorrectly'. (Clean Code Rules)
(StaticAccess)
1730-1730: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testDeleteWithChildrenShiftsCorrectly'. (Clean Code Rules)
(StaticAccess)
1739-1739: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testDeleteWithChildrenShiftsCorrectly'. (Clean Code Rules)
(StaticAccess)
1740-1740: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testDeleteWithChildrenShiftsCorrectly'. (Clean Code Rules)
(StaticAccess)
1741-1741: Avoid using static access to class '\yii2\extensions\nestedsets\tests\support\model\Tree' in method 'testDeleteWithChildrenShiftsCorrectly'. (Clean Code Rules)
(StaticAccess)
🪛 GitHub Actions: ecs
tests/NestedSetsBehaviorTest.php
[warning] 1708-1719: PhpCsFixer: TrailingCommaInMultilineFixer detected missing trailing commas in multiline function calls. Run 'vendor/bin/ecs check --fix' to apply fixes.
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit-compatibility / 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.1-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-ubuntu-latest
- GitHub Check: phpunit-compatibility / PHP 8.4-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.1-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.2-windows-latest
- GitHub Check: phpunit-compatibility / PHP 8.1-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (3)
tests/NestedSetsBehaviorTest.php (3)
1624-1667: Well-structured test for leaf node deletion!The test correctly verifies the nested set attribute adjustments after deleting a leaf node. The shift calculations are accurate according to the nested sets algorithm.
1669-1718: Excellent test coverage with tree integrity validation!The test not only verifies the correct shifting of left/right attributes but also includes containment assertions to ensure child nodes remain within their parent's bounds after deletion. This is a best practice for nested set testing.
1720-1747: Comprehensive test for subtree deletion!The test properly validates the
deleteWithChildren()method by confirming that all nodes in the subtree are removed and subsequent nodes shift by the correct amount (width of the deleted subtree).
… attribute shifts.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/NestedSetsBehaviorTest.php (1)
1722-1748: Fix missing trailing commas in multiline assertion calls.The pipeline detected missing trailing commas in the assertion calls. This is a code style requirement that needs to be fixed.
Apply this diff to fix the formatting:
if ($oldLft > $rightValueBeforeDelete) { self::assertEquals( $oldLft - 2, $node->lft, - "Node $i left should be shifted when > deleted right" + "Node $i left should be shifted when > deleted right", ); } else { self::assertEquals( $oldLft, $node->lft, - "Node $i left should NOT be shifted when <= deleted right" + "Node $i left should NOT be shifted when <= deleted right", ); } if ($oldRgt > $rightValueBeforeDelete) { self::assertEquals( $oldRgt - 2, $node->rgt, - "Node $i right should be shifted when > deleted right" + "Node $i right should be shifted when > deleted right", ); } else { self::assertEquals( $oldRgt, $node->rgt, - "Node $i right should NOT be shifted when <= deleted right" + "Node $i right should NOT be shifted when <= deleted right", ); }
🧹 Nitpick comments (1)
tests/NestedSetsBehaviorTest.php (1)
1721-1747: Consider simplifying by removing else expressions.The logic is correct, but you can simplify the code by eliminating the else clauses as suggested by PHPMD.
Here's a simplified version:
- if ($oldLft > $rightValueBeforeDelete) { - self::assertEquals( - $oldLft - 2, - $node->lft, - "Node $i left should be shifted when > deleted right", - ); - } else { - self::assertEquals( - $oldLft, - $node->lft, - "Node $i left should NOT be shifted when <= deleted right", - ); - } + $expectedLft = $oldLft > $rightValueBeforeDelete ? $oldLft - 2 : $oldLft; + $message = $oldLft > $rightValueBeforeDelete + ? "Node $i left should be shifted when > deleted right" + : "Node $i left should NOT be shifted when <= deleted right"; + self::assertEquals($expectedLft, $node->lft, $message); - if ($oldRgt > $rightValueBeforeDelete) { - self::assertEquals( - $oldRgt - 2, - $node->rgt, - "Node $i right should be shifted when > deleted right", - ); - } else { - self::assertEquals( - $oldRgt, - $node->rgt, - "Node $i right should NOT be shifted when <= deleted right", - ); - } + $expectedRgt = $oldRgt > $rightValueBeforeDelete ? $oldRgt - 2 : $oldRgt; + $message = $oldRgt > $rightValueBeforeDelete + ? "Node $i right should be shifted when > deleted right" + : "Node $i right should NOT be shifted when <= deleted right"; + self::assertEquals($expectedRgt, $node->rgt, $message);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/mutation.yml(1 hunks)tests/NestedSetsBehaviorTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/mutation.yml
🧰 Additional context used
🪛 PHPMD (2.15.0)
tests/NestedSetsBehaviorTest.php
1727-1733: The method testShiftBehaviorAfterDeleteWithPreciseBoundary uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (Clean Code Rules)
(ElseExpression)
1741-1747: The method testShiftBehaviorAfterDeleteWithPreciseBoundary uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (Clean Code Rules)
(ElseExpression)
🪛 GitHub Actions: ecs
tests/NestedSetsBehaviorTest.php
[error] 1722-1748: PhpCsFixer detected missing trailing commas in multiline arrays or function calls. Run 'vendor/bin/ecs check --fix' to automatically fix this issue.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (1)
tests/NestedSetsBehaviorTest.php (1)
1624-1672: LGTM! Excellent edge case test for nested set deletion behavior.This test effectively verifies that nodes whose left value equals the deleted node's right value remain unaffected while other nodes shift correctly. The test logic is sound and covers an important edge case in nested set implementations.
…Insert()` methods.
…update appendTo/insertAfter calls in tests.
…d test for appending to existing child node.
…y depth behavior in `NestedSetsBehavior`.
…and options in `mutation.yml`.
2 participants
Summary by CodeRabbit