Skip to content

tests 継承するクラスを Common_TestCase に統一#1403

Open
seasoftjapan wants to merge 2 commits into
masterfrom
seasoft-1401-2
Open

tests 継承するクラスを Common_TestCase に統一#1403
seasoftjapan wants to merge 2 commits into
masterfrom
seasoft-1401-2

Conversation

@seasoftjapan
Copy link
Copy Markdown
Contributor

@seasoftjapan seasoftjapan commented May 27, 2026

fix #1401

#1402 (comment) の情報を基に、extends \PHPUnit\Framework\TestCaseextends Common_TestCase に置換した。

class Common_TestCase extends \PHPUnit\Framework\TestCase なので、おそらく問題ないと見込んでいる。

Summary by CodeRabbit

  • Tests
    • テスト基盤の継承先を統一し、テスト実行環境の一貫性を向上しました。
    • 一部テスト内の冗長なヘルパーを親側に移動・削除し、共通セットアップを活用するように整理しました。
    • これによりテストの保守性と実行の安定性が向上します。

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

複数のテストファイルで基底クラスを PHPUnit の TestCase 系から Common_TestCase に置き換え、tests/class/SC_Query_Test.php ではローカルの verify() メソッドを削除して親実装へ委譲しています。

Changes

テスト基底クラスの統一

Layer / File(s) Summary
テストクラスの継承先を Common_TestCase へ統一
tests/class/module/HTTP/HTTP_Request_CompatibilityTest.php, tests/class/module/HTTP/HTTP_Request_GuzzleTest.php, tests/class/module/Net/Net_URLTest.php, tests/class/module/Net/Net_URL_CompatibilityTest.php, tests/class/batch/SC_Batch_Update_parseDistInfoTest.php, tests/class/modifier/Modifier_ScriptEscapeTest.php
これらのテストクラスが \PHPUnit\Framework\TestCase または PHPUnit_Framework_TestCase から Common_TestCase に継承先を変更している。
SC_Query_Test の基底移行と verify() 削除
tests/class/SC_Query_Test.php
SC_Query_TestPHPUnit_Framework_TestCase から Common_TestCase へ変更され、同ファイル内の protected function verify() を削除して親クラス実装へ委譲している。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

bug:Low

Suggested reviewers

  • nanasess

Poem

🐇 テストの道 そっと揃えたら
みんなが共に 親を見つけたよ
古い verify は 空に消えて
共通の基盤で 走り出すテスト
小さな一歩 安らぐ朝 😊

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PRタイトルは「tests 継承するクラスを Common_TestCase に統一」であり、実際の変更内容(複数のテストクラスの基底クラスをPHPUnitのTestCaseからCommon_TestCaseに統一)と完全に一致している。
Linked Issues check ✅ Passed PR #1403はissue #1401の修正を目的とし、参照コメントに基づいて全テストクラスの継承元をCommon_TestCaseに統一することで、MDB2インスタンスエラーの原因となる継承構造の不整合を解消している。
Out of Scope Changes check ✅ Passed 変更内容は全てテストクラスの基底クラス統一に限定されており、issue #1401の解決に直結する修正となっている。機能コードやその他の要素への変更は含まれていない。

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch seasoft-1401-2

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 and usage tips.

@seasoftjapan
Copy link
Copy Markdown
Contributor Author

#1402 投稿時点では、もっと該当があると思い込んでいたが、実際には局所的な使用状況だった。

f3c520e940 (のぶ 2026-02-02 14:43:22 +0900  13) class HTTP_Request_CompatibilityTest extends \PHPUnit\Framework\TestCase
77a4d81f9f (のぶ 2026-02-24 14:42:14 +0900  11) class HTTP_Request_GuzzleTest extends \PHPUnit\Framework\TestCase
cb73d22b69 (のぶ 2026-02-02 14:30:54 +0900  11) class Net_URLTest extends \PHPUnit\Framework\TestCase
f3c520e940 (のぶ 2026-02-02 14:43:22 +0900  13) class Net_URL_CompatibilityTest extends \PHPUnit\Framework\TestCase

それぞれ、以下に属するコミットだった。
#1329
#1349
#1329
#1329

@codecov
Copy link
Copy Markdown

codecov Bot commented May 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.20%. Comparing base (fbc45f7) to head (6795073).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1403      +/-   ##
==========================================
+ Coverage   55.88%   56.20%   +0.31%     
==========================================
  Files          87       87              
  Lines       11090    11090              
==========================================
+ Hits         6198     6233      +35     
+ Misses       4892     4857      -35     
Flag Coverage Δ
tests 56.20% <ø> (+0.31%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@seasoftjapan seasoftjapan changed the title fix: PHPUnit でテスト後半が could not find MDB2 instance #1401 tests 継承するクラスを Common_TestCase に統一 May 27, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/class/batch/SC_Batch_Update_parseDistInfoTest.php (1)

6-38: ⚡ Quick win

Common_TestCase への変更がこのクラスでは実質効いていません。

@backupGlobals disabled のまま、setUp() / tearDown() でも parent::setUp() / parent::tearDown() を呼んでいないので、Common_TestCase 側のトランザクション管理や MDB2 初期化はこのテストでは使われません。ここは親ライフサイクルを呼ぶか、DB 非依存テストとして PHPUnit\Framework\TestCase のままに戻したほうが変更意図と一致します。

Based on learnings, pure unit tests that don't require database access should extend PHPUnit\Framework\TestCase instead of Common_TestCase to avoid environment-dependent setup and skipped tests.

🤖 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 `@tests/class/batch/SC_Batch_Update_parseDistInfoTest.php` around lines 6 - 38,
The test class SC_Batch_Update_parseDistInfoTest either needs to call the parent
lifecycle to use Common_TestCase's DB setup/teardown or be reverted to a pure
unit test by changing its base class; update SC_Batch_Update_parseDistInfoTest
so that setUp() and tearDown() call parent::setUp() and parent::tearDown()
respectively (preserving the existing tmpDir logic), or if the test does not
require DB/MDB2/transaction support, change the class to extend
PHPUnit\Framework\TestCase instead of Common_TestCase to avoid relying on
environment-dependent initialization.
🤖 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.

Nitpick comments:
In `@tests/class/batch/SC_Batch_Update_parseDistInfoTest.php`:
- Around line 6-38: The test class SC_Batch_Update_parseDistInfoTest either
needs to call the parent lifecycle to use Common_TestCase's DB setup/teardown or
be reverted to a pure unit test by changing its base class; update
SC_Batch_Update_parseDistInfoTest so that setUp() and tearDown() call
parent::setUp() and parent::tearDown() respectively (preserving the existing
tmpDir logic), or if the test does not require DB/MDB2/transaction support,
change the class to extend PHPUnit\Framework\TestCase instead of Common_TestCase
to avoid relying on environment-dependent initialization.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e68a2088-1866-459a-b71e-ca3ce25a1893

📥 Commits

Reviewing files that changed from the base of the PR and between b2c5989 and 6795073.

📒 Files selected for processing (3)
  • tests/class/SC_Query_Test.php
  • tests/class/batch/SC_Batch_Update_parseDistInfoTest.php
  • tests/class/modifier/Modifier_ScriptEscapeTest.php
✅ Files skipped from review due to trivial changes (1)
  • tests/class/modifier/Modifier_ScriptEscapeTest.php

@seasoftjapan
Copy link
Copy Markdown
Contributor Author

6795073 で3クラスを追加対応。

class Modifier_ScriptEscapeTest extends PHPUnit_Framework_TestCase

commit 24ced1b
Author: daichi_otani <d**@**p>
Date: Thu Jul 27 00:28:34 2023 +0900

class SC_Batch_Update_parseDistInfoTest extends PHPUnit_Framework_TestCase

commit e4beafe #1341
Author: のぶ <n**@**m>
Date: Tue Feb 17 19:59:45 2026 +0900

class SC_Query_Test extends PHPUnit_Framework_TestCase

commit c13e750
Author: nanasess <n**@**8>
Date: Tue Aug 3 02:43:49 2010 +0000

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.

PHPUnit でテスト後半が could not find MDB2 instance

1 participant