Add concurrent coroutine test runner#3
Conversation
Runs PHPUnit test cases concurrently, each in its own Swoole coroutine, bounded by a user-defined pool of N. Tests that wait on coroutine I/O overlap each other instead of running serially. - Runner discovers *Test.php (or takes classes), expands #[DataProvider] rows, and drives each test's lifecycle directly (PHPUnit's runTest is private and runBare is final, and both share process-global output buffering that corrupts under concurrent coroutines). - Enables Swoole hook-all so sleep/socket/DB I/O yield instead of block. - Convenience Async\TestCase base carries the Async trait, so assertEventually() works out of the box and yields under hook-all. - bin/co-phpunit CLI entrypoint. - Swoole phpstan stubs (CI image has no ext-swoole); ext-swoole as a composer suggest; RunnerTest skips when the extension is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR introduces a Swoole-based concurrent coroutine test runner (
Confidence Score: 3/5The core runner logic in Runner.php has multiple behavioral deviations from PHPUnit that will silently affect real test suites; the peripheral files are clean. Three independent correctness bugs are concentrated in Runner.php: tearDown is skipped when setUp throws (leaving resources open), inherited test methods from abstract parent classes are silently never run, and setUpBeforeClass can fire multiple times against a single tearDownAfterClass due to missing deduplication. Any one of these could cause subtle misbehavior in a real integration suite, and all three are on the central code path exercised every time the runner is used. src/Tests/Async/Runner.php warrants a careful second pass before merging. Important Files Changed
Reviews (1): Last reviewed commit: "Add concurrent coroutine test runner" | Re-trigger Greptile |
| $invoke = \Closure::bind(function () use ($method, $args) { | ||
| $this->setUp(); | ||
| try { | ||
| $this->{$method}(...$args); | ||
| } finally { | ||
| $this->tearDown(); | ||
| } | ||
| }, $test, $class); |
There was a problem hiding this comment.
tearDown() is never called when setUp() throws an exception. If setUp() acquires a resource (opens a DB connection, writes a temp file) and then throws before returning, that resource is abandoned. PHPUnit's runBare() always calls tearDown() in a finally block regardless of whether setUp() succeeded.
| $invoke = \Closure::bind(function () use ($method, $args) { | |
| $this->setUp(); | |
| try { | |
| $this->{$method}(...$args); | |
| } finally { | |
| $this->tearDown(); | |
| } | |
| }, $test, $class); | |
| $invoke = \Closure::bind(function () use ($method, $args) { | |
| try { | |
| $this->setUp(); | |
| $this->{$method}(...$args); | |
| } finally { | |
| $this->tearDown(); | |
| } | |
| }, $test, $class); |
| if ($method->isStatic() || $method->getDeclaringClass()->isAbstract()) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The
$method->getDeclaringClass()->isAbstract() guard silently drops any test method inherited from an abstract base class. A project that shares common test logic via abstract class BaseSuiteTest extends TestCase { public function testCommonBehavior(): void { … } } would find that testCommonBehavior is never run in any concrete subclass. The abstract-class exclusion is already handled upstream by addDirectory(); the guard here is only needed to avoid picking up non-test methods from the PHPUnit TestCase hierarchy, but those are already filtered by the str_starts_with('test') / #[Test] attribute check.
| if ($method->isStatic() || $method->getDeclaringClass()->isAbstract()) { | |
| continue; | |
| } | |
| if ($method->isStatic()) { | |
| continue; | |
| } |
| $cases = []; | ||
| foreach ($this->classes as $class) { | ||
| $class::setUpBeforeClass(); | ||
| foreach ($this->cases($class) as $case) { | ||
| $cases[] = $case; | ||
| } | ||
| } |
There was a problem hiding this comment.
setUpBeforeClass() is iterated over $this->classes without deduplication, while tearDownAfterClass() uses array_unique. If the same class is registered twice — e.g. via addTestCase(Foo::class) followed by addDirectory() scanning a directory that already loaded Foo — setUpBeforeClass() fires twice but tearDownAfterClass() fires once. Depending on what setUpBeforeClass does (e.g. creating a database schema, inserting seed data), calling it twice is likely to throw or corrupt state.
| $cases = []; | |
| foreach ($this->classes as $class) { | |
| $class::setUpBeforeClass(); | |
| foreach ($this->cases($class) as $case) { | |
| $cases[] = $case; | |
| } | |
| } | |
| $cases = []; | |
| foreach (\array_unique($this->classes) as $class) { | |
| $class::setUpBeforeClass(); | |
| foreach ($this->cases($class) as $case) { | |
| $cases[] = $case; | |
| } | |
| } |
| $rows = []; | ||
| foreach ($attributes as $attribute) { | ||
| $provider = $attribute->newInstance()->methodName(); | ||
| /** @var iterable<array-key, array<mixed>> $yielded */ | ||
| $yielded = $class::$provider(); | ||
| foreach ($yielded as $key => $row) { | ||
| $rows[$key] = $row; | ||
| } | ||
| } |
There was a problem hiding this comment.
DataProvider key collision across multiple providers
When a method carries two #[DataProvider] attributes whose providers both yield the same key, the later provider silently overwrites the earlier one's row: $rows[$key] = $row is a last-writer-wins assignment. PHPUnit avoids this by prefixing each row's label with the provider method name (e.g. providerA#0 vs providerB#0). Without that, a user who stacks two providers and accidentally uses matching keys will silently lose test cases with no error or warning.
What
Adds a concurrent coroutine test runner for PHPUnit: each test runs in its own Swoole coroutine, bounded by a user-defined pool of N. While one test waits on coroutine I/O (a channel, a hooked socket,
Coroutine::sleep), another runs — so a suite of slow integration tests finishes in roughly the time of its slowest test rather than their sum.How
Async\Runnerdiscovers*Test.php(or takes classes viaaddTestCase()), expands#[DataProvider]rows, and runs each case across aChannel-bounded coroutine pool with aWaitGroup. It classifies outcomes (pass / fail / error / skip) and returns a process exit code.setUp/tearDown,setUpBeforeClass/tearDownAfterClass) and bootstraps PHPUnit'sConfigurationregistry itself — because PHPUnit 12'srunTest()is private andrunBare()is final, and both route through process-global output buffering that corrupts when coroutines interleave.Swoole\Runtime::enableCoroutine(SWOOLE_HOOK_ALL)sosleep/sockets/file & DB I/O yield instead of blocking the scheduler.Async\TestCaseis a convenience base carrying the existingAsynctrait, soassertEventually()is available out of the box and its polling waits yield under hook-all.bin/co-phpunitCLI entrypoint.Scope
Built for coroutine-friendly integration tests that assert and skip. Process-global PHPUnit features (output-buffering assertions, global-state isolation, separate-process tests) are out of scope, since the runner bypasses PHPUnit's sequential runner.
CI / Swoole
CI runs in a Swoole-less
composerimage, so:stubs/swoole.stub(viascanFiles).ext-swooleis a composersuggest, not a hard requirement.RunnerTestskips when the extension is absent.Testing
RunnerTestdrives the runner as a subprocess against a fixture (tests/fixtures/SampleTest.php, excluded from the normal suite), asserting the outcome breakdown + exit code, and proving concurrency by comparing wall-clock at--concurrency=1vs--concurrency=10.🤖 Generated with Claude Code