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