Skip to content

Reach full branch coverage#372

Merged
koriym merged 2 commits into
bearsunday:1.xfrom
koriym:100
Jul 15, 2026
Merged

Reach full branch coverage#372
koriym merged 2 commits into
bearsunday:1.xfrom
koriym:100

Conversation

@koriym

@koriym koriym commented Jun 7, 2026

Copy link
Copy Markdown
Member

What changed

  • Added tests for remaining JSON Schema error-mapping and resolver branch paths.
  • Covered falsey/non-string/non-schema cases that were previously only line-covered.
  • Adjusted JsonSchemaErrors::getIterator() so branch/path instrumentation records the generator path fully.

Why

The suite already had full line coverage, but branch coverage was slightly below 100%. These changes make the branch coverage target explicit and verifiable.

Validation

  • Fetched latest upstream/1.x before creating this PR.
  • Ran composer coverage.
  • Result: OK (409 tests, 709 assertions).
  • Coverage summary: Branches: 100.00% (1150/1150), Lines: 100.00% (1453/1453), Methods: 100.00% (319/319), Classes: 100.00% (96/96).

@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 48e0c7c8-1d61-4702-81d2-6c781872da93

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

JsonSchemaErrors::getIterator() の実装をクロージャで Generator を生成する方式に変更した。あわせて JsonSchemaErrorsJsonSchemaInterceptorJsonSchemaErrorMapperJsonSchemaErrorSchemaErrorMessageResolver の各エッジケースをカバーするテストを追加・更新した。

Changes

JsonSchema getIterator 修正とエッジケーステスト追加

Layer / File(s) Summary
getIterator() クロージャ方式への変更とその直接消費テスト
src/JsonSchema/JsonSchemaErrors.php, tests/JsonSchema/JsonSchemaErrorsTest.php
getIterator() 内で yield from $this->errors を直接行う代わりにクロージャで Generator を生成する形に変更し、iterator_to_array で直接消費したときに単一エラー配列が返ることを検証するテストを追加した。
各コンポーネントのエッジケーステスト追加
tests/JsonSchema/Interceptor/JsonSchemaInterceptorTest.php, tests/JsonSchema/JsonSchemaErrorMapperTest.php, tests/JsonSchema/JsonSchemaErrorTest.php, tests/JsonSchema/SchemaErrorMessageResolverTest.php
非オブジェクト JSON スキーマ (true) に対して schema メソッドが null を返すテスト、required 制約の params.property が空の場合に行の property へフォールバックするテスト、disabled => false のレンダリングを含む期待値更新、非文字列・非スキーマ値でのメッセージ解決が null を返す 3 件のテストを追加した。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • bearsunday/BEAR.Resource#367: JsonSchemaErrors::getIterator() の挙動変更および JsonSchemaErrorMapper の構造化エラー処理に関するテスト追加が、このPRで取り扱われたDTO基盤の構造化エラー実装と直接関連している。
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% 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
Title check ✅ Passed プルリクエストのタイトル「Reach full branch coverage」は、変更セットの主要な目的である分岐カバレッジを100%に達成することを明確に示しており、変更セットと完全に関連している。
Description check ✅ Passed 説明は変更セットに関連しており、追加されたテストと JsonSchemaErrors::getIterator() の変更について言及し、適切なレベルの詳細を提供している。
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (51e56dd) to head (fa865bd).
⚠️ Report is 3 commits behind head on 1.x.

Additional details and impacted files
@@             Coverage Diff             @@
##                 1.x      #372   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       687       687           
===========================================
  Files             96        96           
  Lines           1720      1722    +2     
===========================================
+ Hits            1720      1722    +2     

☔ 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.

@koriym
koriym marked this pull request as ready for review June 20, 2026 17:06
@koriym

koriym commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@tests/JsonSchema/Interceptor/JsonSchemaInterceptorTest.php`:
- Around line 143-149: The test method uses a hardcoded file path to write
schema test data, which causes race conditions and cleanup issues when tests run
in parallel. Replace the fixed path for the schema file with a temporary file
created using PHP's temporary file functions, and ensure the temporary file is
properly deleted after the test completes using a finally block or similar
cleanup mechanism. This will prevent test pollution and race conditions during
parallel test execution.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b6d7fb90-5a97-46e4-bb4a-fabd9955fe64

📥 Commits

Reviewing files that changed from the base of the PR and between 9cbee48 and d3c5495.

📒 Files selected for processing (6)
  • src/JsonSchema/JsonSchemaErrors.php
  • tests/JsonSchema/Interceptor/JsonSchemaInterceptorTest.php
  • tests/JsonSchema/JsonSchemaErrorMapperTest.php
  • tests/JsonSchema/JsonSchemaErrorTest.php
  • tests/JsonSchema/JsonSchemaErrorsTest.php
  • tests/JsonSchema/SchemaErrorMessageResolverTest.php

Comment thread tests/JsonSchema/Interceptor/JsonSchemaInterceptorTest.php Outdated
koriym added 2 commits July 16, 2026 03:12
Fix CodeRabbit review: replace hardcoded path with tempnam + finally cleanup
to prevent parallel test collisions and leftover files.
@koriym

koriym commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai 確認ありがとうございました。ご指摘の通り、固定パスではなく tempnam() で一時ファイルを作成し、finally でクリーンアップするように修正しました(commit fa865bd)。並列テスト時の競合と残留ファイルの問題を解消しています。

@koriym

koriym commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai Thanks for the review. As suggested, I replaced the hardcoded path with tempnam() for a temporary file and added a finally block for cleanup (commit fa865bd). This prevents race conditions during parallel test runs and leftover files.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tip

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

@koriym ご対応ありがとうございます。tempnam() による一時ファイル生成と finally でのクリーンアップは、並列テスト実行時の競合や残留ファイルの問題を解消する適切な修正です。確認しました。

(๑˃̵ᴗ˂̵)و

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tip

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

@koriym Understood, thanks for confirming! The tempnam() + finally cleanup approach is already noted as a solid fix for the temp file handling. No further action needed on my end here. 🐰✨

@koriym
koriym merged commit f708627 into bearsunday:1.x Jul 15, 2026
17 of 18 checks passed
@koriym
koriym deleted the 100 branch July 15, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant