Skip to content

Fix RunTestsInCoroutine for PHPUnit 10.5+#362

Open
anabeto93 wants to merge 3 commits intohypervel:mainfrom
anabeto93:fix/run-tests-in-coroutine-phpunit-10
Open

Fix RunTestsInCoroutine for PHPUnit 10.5+#362
anabeto93 wants to merge 3 commits intohypervel:mainfrom
anabeto93:fix/run-tests-in-coroutine-phpunit-10

Conversation

@anabeto93
Copy link
Copy Markdown
Contributor

Problem

PHPUnit 10.5 changed runTest() from protected to private:

// PHPUnit < 10.5
protected function runTest(): mixed { ... }

// PHPUnit 10.5+
private function runTest(): mixed { ... }

The RunTestsInCoroutine trait overrides runTest() as final protected to swap the test method name and wrap execution in a Swoole coroutine via run(). But since PHPUnit's runTest() is now private, PHP treats the trait's method as a completely separate method — PHPUnit's runBare() calls its own private runTest(), 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:

Swoole\Error: API must be called in the coroutine

This affects every Hypervel test that uses RunTestsInCoroutine — including the default ExampleTest in a fresh composer create-project hypervel/hypervel app.

Reproduction

composer create-project hypervel/hypervel test-app
cd test-app
composer require pestphp/pest --dev -W
./vendor/bin/pest

What 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

./vendor/bin/phpunit 
PHPUnit 10.5.45 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.4.18
Configuration: /home/richard/Codes/Tutorials/Laravel/hyper-app/phpunit.xml.dist

..Segmentation fault (core dumped)

Unit tests pass, but the Feature ExampleTest (which calls $this->get('/')) crashes with API must be called in the coroutine.

Root Cause

PHPUnit's runBare() calls its private runTest() at line ~689:

// PHPUnit\Framework\TestCase::runBare()
$this->testResult = $this->runTest();

The private runTest() calls the test method by name:

// PHPUnit\Framework\TestCase::runTest() [private]
$testResult = $this->{$this->name}(...array_values($testArguments));

The trait's approach was to intercept runTest(), rename $this->name to runTestsInCoroutine, then call parent::runTest(). But since the trait's runTest() is never called by PHPUnit (it's a different method due to private visibility), the name swap never happens and the test runs without a coroutine.

Fix

Move the name swap from the unreachable runTest() override to setUp(), which IS called before PHPUnit's private runTest() and IS overridable (protected).

RunTestsInCoroutine.php

Replaced the broken final protected function runTest() with setUpCoroutineTest():

protected function setUpCoroutineTest(): void
{
    if (Coroutine::getCid() === -1 && $this->enableCoroutine) {
        $this->realTestName = $this->name();
        $this->setName('runTestsInCoroutine');
    }
}

When PHPUnit's private runTest() reads $this->name, it finds runTestsInCoroutine instead of the real test name. It calls $this->runTestsInCoroutine() which wraps the real test method in Hypervel\Coroutine\run().

TestCase.php

Added the call at the end of setUp():

if (method_exists($this, 'setUpCoroutineTest')) {
    $this->setUpCoroutineTest();
}

This runs after all trait setup (including RefreshDatabase::refreshDatabase()) but before PHPUnit's runTest().

Files Changed

  • src/foundation/src/Testing/Concerns/RunTestsInCoroutine.php — removed broken runTest() override, added setUpCoroutineTest()
  • src/foundation/src/Testing/TestCase.php — calls setUpCoroutineTest() at end of setUp()

Backward Compatibility

  • Users who don't use RunTestsInCoroutine are unaffected (the method_exists check handles this)
  • Users who DO use the trait get working coroutine wrapping again
  • The runTestsInCoroutine() method, invokeSetupInCoroutine(), invokeTearDownInCoroutine(), and the $enableCoroutine / $copyNonCoroutineContext properties are unchanged
  • The removed runTest() method was non-functional in PHPUnit 10.5+ anyway

Known Remaining Issue

After this fix, tests run inside coroutines correctly. However, RefreshDatabase::beginDatabaseTransaction() has a separate issue: the transaction beginTransaction() and rollBack() 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 gets Swoole\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.

@binaryfire
Copy link
Copy Markdown
Collaborator

binaryfire commented Mar 31, 2026

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.

@anabeto93
Copy link
Copy Markdown
Contributor Author

#357

Only now seeing this just as I am linting and pushing some more 😄 .
For what it's worth, this PR offers a fix that works with PHPUnit 10.5.46+ without pinning, moving the test name swap from the now-private runTest() to setUp(), which runs before PHPUnit's runTest() reads $this->name. v0.3 users who can't pin (or don't want the CVE exposure) have an option.

I can close this if v0.4 with PHPUnit 13 is the preferred path.

@binaryfire
Copy link
Copy Markdown
Collaborator

binaryfire commented Mar 31, 2026

#357

Only now seeing this just as I am linting and pushing some more 😄 . For what it's worth, this PR offers a fix that works with PHPUnit 10.5.46+ without pinning, moving the test name swap from the now-private runTest() to setUp(), which runs before PHPUnit's runTest() reads $this->name. v0.3 users who can't pin (or don't want the CVE exposure) have an option.

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.

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