Skip to content

Add concurrent coroutine test runner#3

Open
loks0n wants to merge 1 commit into
mainfrom
async-test-case
Open

Add concurrent coroutine test runner#3
loks0n wants to merge 1 commit into
mainfrom
async-test-case

Conversation

@loks0n

@loks0n loks0n commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

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.

vendor/bin/co-phpunit tests --concurrency=20
use Utopia\Tests\Async\Runner;

$runner = new Runner(concurrency: 20);
$runner->addDirectory(__DIR__ . '/tests');
exit($runner->run());

How

  • Async\Runner discovers *Test.php (or takes classes via addTestCase()), expands #[DataProvider] rows, and runs each case across a Channel-bounded coroutine pool with a WaitGroup. It classifies outcomes (pass / fail / error / skip) and returns a process exit code.
  • It drives each test's lifecycle directly (setUp/tearDown, setUpBeforeClass/tearDownAfterClass) and bootstraps PHPUnit's Configuration registry itself — because PHPUnit 12's runTest() is private and runBare() is final, and both route through process-global output buffering that corrupts when coroutines interleave.
  • Enables Swoole\Runtime::enableCoroutine(SWOOLE_HOOK_ALL) so sleep/sockets/file & DB I/O yield instead of blocking the scheduler.
  • Async\TestCase is a convenience base carrying the existing Async trait, so assertEventually() is available out of the box and its polling waits yield under hook-all.
  • bin/co-phpunit CLI 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 composer image, so:

  • phpstan uses stubs/swoole.stub (via scanFiles).
  • ext-swoole is a composer suggest, not a hard requirement.
  • RunnerTest skips when the extension is absent.

Testing

  • RunnerTest drives 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=1 vs --concurrency=10.
  • Local gate green: lint (PSR-12), phpstan level 8, phpunit.

🤖 Generated with Claude Code

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

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a Swoole-based concurrent coroutine test runner (Async\Runner) that runs PHPUnit TestCases in parallel coroutines bounded by a configurable pool, along with a bin/co-phpunit CLI entrypoint, supporting infrastructure (stubs, fixture, integration test), and README documentation.

  • Async\Runner discovers test classes via reflection, expands #[DataProvider] rows, and dispatches each case into a Channel-bounded coroutine pool with a WaitGroup, bypassing PHPUnit's sequential runner to enable genuine I/O overlap.
  • Three correctness issues in Runner.php need attention: tearDown() is not called when setUp() throws (resource-leak risk), test methods declared in abstract parent classes are silently skipped due to an overly broad isAbstract() guard, and setUpBeforeClass() is iterated without deduplication while tearDownAfterClass() uses array_unique.

Confidence Score: 3/5

The 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

Filename Overview
src/Tests/Async/Runner.php Core runner implementation; contains three correctness issues: tearDown skipped when setUp throws, inherited test methods silently dropped, and setUpBeforeClass deduplication asymmetry.
tests/RunnerTest.php Subprocess-based integration test that validates outcome counts and concurrency timing; solid approach to avoiding config bootstrap conflicts.
tests/fixtures/SampleTest.php Fixture covering pass/fail/error/skip/data-provider/assertEventually cases; accurately represents the expected 8 expanded test cases.
bin/co-phpunit CLI entrypoint; properly escapes shell arguments, but passes concurrency to Runner without validating it is > 0.
src/Tests/Async/Result.php Simple immutable value object for test outcomes; no issues.
src/Tests/Async/Status.php Backed enum representing test outcome states with symbol display; no issues.
src/Tests/Async/TestCase.php Thin convenience base class mixing in the Async trait; no issues.
stubs/swoole.stub PHPStan stub providing Swoole type stubs for CI environments without the extension; no issues.
composer.json Adds ext-swoole as a suggest dependency and registers the bin entrypoint; no issues.

Reviews (1): Last reviewed commit: "Add concurrent coroutine test runner" | Re-trigger Greptile

Comment on lines +203 to +210
$invoke = \Closure::bind(function () use ($method, $args) {
$this->setUp();
try {
$this->{$method}(...$args);
} finally {
$this->tearDown();
}
}, $test, $class);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Suggested change
$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);

Comment on lines +142 to +144
if ($method->isStatic() || $method->getDeclaringClass()->isAbstract()) {
continue;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Suggested change
if ($method->isStatic() || $method->getDeclaringClass()->isAbstract()) {
continue;
}
if ($method->isStatic()) {
continue;
}

Comment on lines +87 to +93
$cases = [];
foreach ($this->classes as $class) {
$class::setUpBeforeClass();
foreach ($this->cases($class) as $case) {
$cases[] = $case;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 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 FoosetUpBeforeClass() 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.

Suggested change
$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;
}
}

Comment on lines +180 to +188
$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;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

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.

1 participant