Fix RunTestsInCoroutine for PHPUnit 10.5+#362
Fix RunTestsInCoroutine for PHPUnit 10.5+#362anabeto93 wants to merge 3 commits intohypervel:mainfrom
Conversation
|
Hi @anabeto93. The best option for Hypervel v0.3 is to pin PHPUnit to v10.5.45. It's not ideal because of the CVE but the vulnerability won't affect most people. You can take a look at #357 for more info and decide for yourself if you're comfortable doing that. Good news is v0.4 will use PHPUnit 13, which added a new hook for frameworks like ours to tap into. |
|
Only now seeing this just as I am linting and pushing some more 😄 . I can close this if v0.4 with PHPUnit 13 is the preferred path. |
@anabeto93 No problem! There are a lot failing tests with this PR: https://github.com/hypervel/components/actions/runs/23811413288/job/69399160165?pr=362. If you run the full PHPUnit test suite locally you'll see them. Unfortunately this isn't something that can be fixed easily in v0.3 because of Hyperf dependencies. |
|
@anabeto93 Thank you for the PR! In v0.3, since PHPUnit changed the While moving the execution of Because Hyperf's testing package also limits the PHPUnit version to v10, and we will be supporting newer versions of PHPUnit in v0.4, it is recommended that we keep the PHPUnit version locked at 10.5.45 for v0.3. |
Problem
PHPUnit 10.5 changed
runTest()fromprotectedtoprivate:The
RunTestsInCoroutinetrait overridesrunTest()asfinal protectedto swap the test method name and wrap execution in a Swoole coroutine viarun(). But since PHPUnit'srunTest()is nowprivate, PHP treats the trait's method as a completely separate method — PHPUnit'srunBare()calls its own privaterunTest(), never the trait's.The result: test methods execute outside a Swoole coroutine. Any operation that requires coroutine context (database connections, channels, HTTP client) crashes with:
This affects every Hypervel test that uses
RunTestsInCoroutine— including the defaultExampleTestin a freshcomposer create-project hypervel/hypervelapp.Reproduction
composer create-project hypervel/hypervel test-app cd test-app composer require pestphp/pest --dev -W ./vendor/bin/pestWhat is not important now is to wonder why I chose pest instead of phpunit. The default phpunit setup with a bare app leads to segmentation fault and that's a different matter as evidenced below
Unit tests pass, but the Feature
ExampleTest(which calls$this->get('/')) crashes withAPI must be called in the coroutine.Root Cause
PHPUnit's
runBare()calls its privaterunTest()at line ~689:The private
runTest()calls the test method by name:The trait's approach was to intercept
runTest(), rename$this->nametorunTestsInCoroutine, then callparent::runTest(). But since the trait'srunTest()is never called by PHPUnit (it's a different method due toprivatevisibility), the name swap never happens and the test runs without a coroutine.Fix
Move the name swap from the unreachable
runTest()override tosetUp(), which IS called before PHPUnit's privaterunTest()and IS overridable (protected).RunTestsInCoroutine.php
Replaced the broken
final protected function runTest()withsetUpCoroutineTest():When PHPUnit's private
runTest()reads$this->name, it findsrunTestsInCoroutineinstead of the real test name. It calls$this->runTestsInCoroutine()which wraps the real test method inHypervel\Coroutine\run().TestCase.php
Added the call at the end of
setUp():This runs after all trait setup (including
RefreshDatabase::refreshDatabase()) but before PHPUnit'srunTest().Files Changed
src/foundation/src/Testing/Concerns/RunTestsInCoroutine.php— removed brokenrunTest()override, addedsetUpCoroutineTest()src/foundation/src/Testing/TestCase.php— callssetUpCoroutineTest()at end ofsetUp()Backward Compatibility
RunTestsInCoroutineare unaffected (themethod_existscheck handles this)runTestsInCoroutine()method,invokeSetupInCoroutine(),invokeTearDownInCoroutine(), and the$enableCoroutine/$copyNonCoroutineContextproperties are unchangedrunTest()method was non-functional in PHPUnit 10.5+ anywayKnown Remaining Issue
After this fix, tests run inside coroutines correctly. However,
RefreshDatabase::beginDatabaseTransaction()has a separate issue: the transactionbeginTransaction()androllBack()execute in different coroutines (setUp's coroutine vs tearDown's coroutine). Hyperf's connection pooling binds PDO sockets to the coroutine that opened them, so the rollback coroutine getsSwoole\Error: Socket has already been bound to another coroutine.This is an architectural limitation of Hyperf's database connection pooling under Swoole's coroutine model, and is being addressed separately by PR #349 which replaces the Hyperf DB layer entirely.