Skip to content

fix: PHP 8.2/8.3 の unit-test で polyfill-php84 の bcround が未定義になる問題を修正#6885

Open
nanasess wants to merge 1 commit into
EC-CUBE:4.4from
nanasess:fix/polyfill-php84-bcround-preload
Open

fix: PHP 8.2/8.3 の unit-test で polyfill-php84 の bcround が未定義になる問題を修正#6885
nanasess wants to merge 1 commit into
EC-CUBE:4.4from
nanasess:fix/polyfill-php84-bcround-preload

Conversation

@nanasess

@nanasess nanasess commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

概要(Overview)

PHP 8.2 / 8.3 の unit-test で、symfony/polyfill-php84Symfony\Polyfill\Php84\Php84::bcround()
実行時に「Call to undefined method」となり、税計算(TaxRuleService::roundByRoundingType()bcround())を
通す全テストが 600 件超エラーになる問題を修正します。

症状

  • 発生条件: PHP 8.2 / 8.3(8.4 以降はネイティブ bcround のため無関係)。
  • 現象: 新規に composer install した環境(=新規 PR)の unit-test で以下が多発。
    Error: Call to undefined method Symfony\Polyfill\Php84\Php84::bcround()
    vendor/symfony/polyfill-php84/bootstrap80.php:47
    src/Eccube/Service/TaxRuleService.php:130
    
  • 4.4 ブランチの push CI が緑なのは、化石化した working な vendor/ キャッシュを
    Nothing to install で復元しているためで、fresh install する新規 PR は本問題を踏みます。

原因

テスト環境(APP_ENV=test)では Symfony の DebugClassLoader が有効です。この状態で、
グローバル関数 bcround()polyfill-php84bootstrap80.php が定義)から
Symfony\Polyfill\Php84\Php84 クラスを kernel 起動後に遅延 autoload すると、
Php84::bcround() が解決できなくなります。

  • bootstrap 時点や standalone(php -r)では Php84::bcround は正しく存在し bcround() も動作する。
  • kernel 起動後(DebugClassLoader 有効)に初めて Php84 を autoload するケースだけ失敗する。

切り分けの結果、composer キャッシュ・opcache・polyfill のバージョン(1.37 / 1.38 とも)は無関係で、
「遅延 autoload のタイミング」が要因であることを確認しています。

修正

tests/bootstrap.php で kernel 起動前に Php84 クラスを class_exists() で事前ロードします(8 行)。
これにより PHP 8.2 / 8.3 の unit-test が安定して通ります。本番(APP_ENV=prod, debug 無効)は
DebugClassLoader が非活性のため影響を受けません。

検証

  • 修正前: fresh install(キャッシュ完全無効)・opcache 無効いずれの条件でも 8.2/8.3 が失敗。
  • 修正後: 8.2 / 8.3 を含む unit-test マトリクスが全て success。

Summary by CodeRabbit

  • Bug Fixes
    • テスト実行時に一部の環境で発生していた失敗を防ぎ、より安定してテストを実行できるようにしました。
    • 依存ライブラリの読み込み順による不具合を回避し、既存のテストが正しく開始されるように改善しました。

テスト env では Symfony DebugClassLoader が有効で、kernel 起動後にグローバル関数
bcround() から Symfony\Polyfill\Php84\Php84 を遅延 autoload すると
Php84::bcround() が解決できず、PHP 8.2/8.3 で税計算(TaxRuleService::bcround)を通す
全テストが「Call to undefined method Php84::bcround()」で失敗していた
(bootstrap 時点や standalone では bcround は正常に動作する)。
tests/bootstrap.php で kernel 起動前に Php84 クラスを class_exists で事前ロードし回避する。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e6ee0dc8-c59a-4cb5-b409-910424173480

📥 Commits

Reviewing files that changed from the base of the PR and between a387a99 and c2b1025.

📒 Files selected for processing (1)
  • tests/bootstrap.php

📝 Walkthrough

Walkthrough

tests/bootstrap.php にて、Symfony Polyfill の Php84 クラスをテスト実行前に class_exists で明示的にロードする処理とコメントが追加された。vendor/autoload.php 読み込み後の遅延解決による問題を回避する目的の変更。

Changes

テストブートストラップの変更

Layer / File(s) Summary
Php84ポリフィルの事前ロード
tests/bootstrap.php
class_exists(Php84::class); の呼び出しと説明コメントを追加し、Php84クラスを事前にロードする処理を導入。

Estimated code review effort: 1 (Trivial) | ~3 minutes

Poem

ぴょんと跳ねて autoload
Php84 呼び出しておくよ
テストの前に そっと確認
遅延解決 もう怖くない
うさぎも安心 ぴょんぴょん🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PHP 8.2/8.3 の unit-test で polyfill-php84 の bcround 問題を修正する内容と一致しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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 Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.27%. Comparing base (a387a99) to head (c2b1025).

Additional details and impacted files
@@            Coverage Diff             @@
##              4.4    #6885      +/-   ##
==========================================
- Coverage   75.32%   75.27%   -0.06%     
==========================================
  Files         519      519              
  Lines       25483    25483              
==========================================
- Hits        19196    19182      -14     
- Misses       6287     6301      +14     
Flag Coverage Δ
Unit 75.27% <ø> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants