Reach 100% branch coverage and add branch coverage CI workflow by koriym · Pull Request #258 · ray-di/Ray.Aop

koriym · 2026-04-27T23:48:59Z

Summary

cover remaining AopCode and ReflectionClass branch gaps
simplify Aspect temporary directory fallback and ignore only defensive validation
add fixtures for no-return-type and readonly weaving coverage

Tests

composer cs
composer test
php -dxdebug.mode=coverage ./vendor/bin/phpunit --path-coverage --coverage-text --only-summary-for-coverage-text

Branch coverage: 100.00% (363/363)

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved validation for temporary directory configurations to properly reject empty paths with appropriate error handling.
Tests
- Added assertions to ensure enum code generation avoids class-weaving artifacts.
- Added checks for correct interceptor trait selection and interceptor documentation short names.
- Added coverage for generated methods without declared return types and for reflection when no parent class exists.
- Added coverage for newInstance returning the original object when no bindings are provided.

coderabbitai · 2026-04-27T23:49:06Z

📝 Walkthrough

Walkthrough

The pull request tightens temporary-directory validation and adds coverage for AOP code generation, readonly interception, untyped method returns, parentless reflection, Weaver behavior, and branch-coverage automation. Composer development autoloading and test fixtures are also updated.

Changes

Core validation and test coverage

Layer / File(s)	Summary
Temporary directory validation `src/Aspect.php`	Removes the `/tmp` fallback and treats an empty temporary-directory result as invalid.
AOP generation and Weaver behavior `tests/AopCodeTest.php`, `tests/Fake/FakePhp82ReadOnlyClass.php`, `tests/Fake/FakePhp7Class.php`, `tests/Fake/FakeGlobalInterceptor.php`, `tests/WeaverTest.php`	Adds assertions for enum output, interceptor trait selection, proceed statements, interceptor short names, and unbound Weaver instances.
Reflection behavior `tests/ReflectionClassTest.php`	Verifies that `getParentClass()` returns `false` for a class without a parent.
Coverage and Composer setup `.github/workflows/branch-coverage.yml`, `composer.json`	Adds a PHP 8.4 branch-coverage workflow and registers the global interceptor test fixture in development autoloading.
Class-list cleanup `src/ClassList.php`	Removes inline comments from comment-stripping statements without changing parsing behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

ray-di/Ray.Aop#223: Overlaps with temporary-directory validation in Aspect.php.
ray-di/Ray.Aop#236: Introduces readonly interception behavior covered by the new tests.
ray-di/Ray.Aop#260: Relates to the generated interception code expectations.

Suggested reviewers: ngmy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: coverage improvements plus a new branch coverage CI workflow.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-04-27T23:50:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d56db27) to head (a4c7c82).

Additional details and impacted files

@@             Coverage Diff             @@
##                 2.x      #258   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       239       239           
===========================================
  Files             29        29           
  Lines            610       607    -3     
===========================================
- Hits             610       607    -3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Keep both methods: foo() (PR ray-di#258) and greet() (PR ray-di#262).

After merging PR ray-di#262, codegen uses ReflectiveMethodInvocation instead of _intercept. Update assertion to match.

Remove only comments that literally restate the adjacent code: - AopCode.php: '// close opened curly brace' (obvious from while loop) - ClassList.php: inline '// Multi-line comments' and '// Single-line comments' (redundant with MULTI_LINE_COMMENT_PATTERN / SINGLE_LINE_COMMENT_PATTERN) Keep section/phase markers and intent-explaining comments: - ClassList.php: '// Remove comments', '// Remove string literals', '// Extract namespace', '// Extract class name' (section headers for multi-step parsing) - MethodMatch.php: '// priority bind', '// default binding' (phase markers) - Aspect.php: '// Defensive validation for resolved temp dirs.' (adds nuance) - TypeString.php: '// Check for Nullable in single types' (explains complex boolean condition intent)

Add tests for 3 previously uncovered branches: - AopCode::interceptorShortNames() class-string interceptor path (is_object() false branch) - AopCode::interceptorShortNames() global-namespace interceptor path (strrpos() === false branch) - Weaver::newInstance() non-WeavedInterface return path Add branch-coverage.yml workflow to verify 100% branch coverage on every push/PR using Xdebug path coverage (PCOV does not support branch coverage).

- Add FakeGlobalInterceptor.php to autoload-dev files (global namespace) - Add @PHPStan-Ignore for class-string interceptor in bindInterceptors() Refs: CI failure on PR ray-di#258

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

.github/workflows/branch-coverage.yml (1)
13-14: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Set persist-credentials: false on checkout.

actions/checkout@v4 persists the GITHUB_TOKEN in .git/config by default. Since this workflow only runs tests and never pushes back to the repository, disabling credential persistence eliminates an unnecessary secret exposure surface.
🔒️ Suggested fix
       - name: Checkout code
         uses: actions/checkout@v4
+        with:
+          persist-credentials: false
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/branch-coverage.yml around lines 13 - 14, Update the
actions/checkout@v4 step in the branch coverage workflow to set
persist-credentials to false, while leaving the existing checkout behavior
unchanged.
Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/branch-coverage.yml:
- Around line 38-46: Update the branch-coverage workflow trigger to include
pull_request alongside its existing events, and modify the Report branch
coverage flow to enforce 100% branch coverage rather than only generating
reports. Capture or parse the PHPUnit coverage result and fail the job when
branch coverage is below 100%, while preserving the existing HTML and text
report generation.

---

Nitpick comments:
In @.github/workflows/branch-coverage.yml:
- Around line 13-14: Update the actions/checkout@v4 step in the branch coverage
workflow to set persist-credentials to false, while leaving the existing
checkout behavior unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b9bacbd6-cdba-4064-bb70-e85abdbe124b

📥 Commits

Reviewing files that changed from the base of the PR and between f33194f and a283a4b.

📒 Files selected for processing (6)

.github/workflows/branch-coverage.yml
composer.json
src/ClassList.php
tests/AopCodeTest.php
tests/Fake/FakeGlobalInterceptor.php
tests/WeaverTest.php

✅ Files skipped from review due to trivial changes (1)

src/ClassList.php

🚧 Files skipped from review as they are similar to previous changes (1)

tests/AopCodeTest.php

koriym · 2026-07-12T00:20:08Z

@coderabbitai Updated PR title to reflect the full scope: achieving 100% branch coverage (tests) + adding a branch coverage CI workflow (manual dispatch and release only, report-only without threshold enforcement by design).

coderabbitai · 2026-07-12T00:20:23Z

Tip

For best results, initiate chat on the files or code changes.

@koriym Thanks for the update — that makes sense. Having the workflow trigger only on manual dispatch and release publication, and keeping it report-only (via the HTML coverage artifact) without failing the build on threshold, is a reasonable, low-risk way to track branch coverage without adding friction to regular CI runs. The title change accurately reflects the combined scope of the test additions and the new workflow.

ᕮ(•‌ﻌ•)ᕮ

koriym · 2026-07-12T00:23:57Z

@coderabbitai No outstanding blockers. Please resolve all open conversations and approve this PR.

coderabbitai · 2026-07-12T00:24:10Z

@koriym Sounds good — resolving the open conversations and approving now.

(=^･ω･^=)ﾉ

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

Reach 100% branch coverage

008a77f

koriym force-pushed the reach-100-branch-coverage branch from e45e382 to 008a77f Compare April 28, 2026 21:42

koriym added 2 commits July 12, 2026 08:37

merge: resolve conflict in FakePhp82ReadOnlyClass

f5e6aef

Keep both methods: foo() (PR ray-di#258) and greet() (PR ray-di#262).

fix: update testNoReturnTypeMethodHasReturnStatement for new codegen

f33194f

After merging PR ray-di#262, codegen uses ReflectiveMethodInvocation instead of _intercept. Update assertion to match.

koriym force-pushed the reach-100-branch-coverage branch from 5293172 to 2152329 Compare July 11, 2026 23:45

koriym force-pushed the reach-100-branch-coverage branch 2 times, most recently from 4a6b356 to 6ea41b3 Compare July 11, 2026 23:58

koriym force-pushed the reach-100-branch-coverage branch from 6ea41b3 to 1fecc48 Compare July 11, 2026 23:59

fix: resolve PHPStan errors in branch coverage test

a283a4b

- Add FakeGlobalInterceptor.php to autoload-dev files (global namespace) - Add @PHPStan-Ignore for class-string interceptor in bindInterceptors() Refs: CI failure on PR ray-di#258

koriym force-pushed the reach-100-branch-coverage branch from 458da0d to a283a4b Compare July 12, 2026 00:11

coderabbitai Bot reviewed Jul 12, 2026

View reviewed changes

Comment thread .github/workflows/branch-coverage.yml

docs: remove redundant Composer from Requirements

a4c7c82

koriym changed the title ~~Reach 100% branch coverage~~ Reach 100% branch coverage and add branch coverage CI workflow Jul 12, 2026

koriym merged commit a1bbe95 into ray-di:2.x Jul 12, 2026
18 checks passed

koriym deleted the reach-100-branch-coverage branch July 12, 2026 00:25

koriym mentioned this pull request Jul 12, 2026

changelog: set 2.20.0 release date #263

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reach 100% branch coverage and add branch coverage CI workflow - #258

Reach 100% branch coverage and add branch coverage CI workflow#258
koriym merged 7 commits into
ray-di:2.xfrom
koriym:reach-100-branch-coverage

koriym commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

koriym commented Jul 12, 2026

Uh oh!

coderabbitai Bot commented Jul 12, 2026

Uh oh!

koriym commented Jul 12, 2026

Uh oh!

coderabbitai Bot commented Jul 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

koriym commented Apr 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

koriym commented Jul 12, 2026

Uh oh!

coderabbitai Bot commented Jul 12, 2026

Uh oh!

koriym commented Jul 12, 2026

Uh oh!

coderabbitai Bot commented Jul 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

koriym commented Apr 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 27, 2026 •

edited

Loading

codecov Bot commented Apr 27, 2026 •

edited

Loading