Speed up test suite via parallel runner and lighter git fixtures - #56
Conversation
`composer test` now drives Pest with `--parallel`, dropping the Core suite from ~17s to ~6s on a 4-core box (3x). Reliability fixes: - `Tests\TestCase` rebinds `livewire.compiler` to a per-worker cache dir; the upstream singleton at `framework/views/livewire/` raced across workers and produced random "filemtime/stat failed" errors. - It also propagates `VIEW_COMPILED_PATH` so child PHP processes (notably `BenchmarkPerformanceCommand --child`) target the same isolated blade dir as the parent test. Per-test git overhead is also cut: `initTestRepo()` copies a per-process `.git` template instead of running `git init` plus three `git config` invocations, and committer/author/`commit.gpgsign` are set globally via `GIT_*` env vars on first call. Saves ~1s on a serial run. `composer test:serial` is preserved as a single-process fallback for debugging worker-isolation issues. https://claude.ai/code/session_01MB9KobbSHQkuGovkT5fbEj
…outes Laravel 12.38+ ships two traits that build the application config and route table once, then reuse them for every test in the worker. They cut another ~10% off both serial (15.5s → 13.8s) and parallel (5.7s → 5.1s) runs in our suite, with no impact on tests that mutate config at runtime — the traits only short-circuit boot-time loading. Together with the prior parallel + git-fixture work this brings the default `composer test` from ~17s to ~5s (3.3x). https://claude.ai/code/session_01MB9KobbSHQkuGovkT5fbEj
- Move git env vars (GIT_AUTHOR_*, GIT_CONFIG_*) into phpunit.xml's <php><env> block so PHPUnit sets them once at boot. Drops the ensureGitTestEnvironment() helper and its per-call guard. - Use ParallelTesting::token() instead of a 3-source TEST_TOKEN fallback chain. - Use File::ensureDirectoryExists() in TestCase (project convention) instead of raw `if (!is_dir) @mkdir`. - Keep only putenv() for VIEW_COMPILED_PATH; the parallel \$_ENV / \$_SERVER writes were redundant for child-process inheritance. - Replace the custom RecursiveIteratorIterator deletion with a pre-resolved `Illuminate\Filesystem\Filesystem` instance captured by the shutdown closure (avoids container-at-shutdown issue without re-implementing recursion). - Extract a single execOrThrow() helper; the three exec/throw blocks in initTestRepo / runTestRepoCommand / ensureRepoTemplate were near-duplicates. - Collapse the `\$isolatedLivewireCachePath` + `\$isolatedPathsResolved` pair into a single `string|false|null` static (false = unresolved). - Trim narrating comments and caller references to keep only WHY notes (per CLAUDE.md). Suite still ~5s parallel, ~13.5s serial; 802/802 pass; lint + types clean. https://claude.ai/code/session_01MB9KobbSHQkuGovkT5fbEj
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes implement parallel test execution with worker isolation. Composer test scripts are updated to use Pest with Changes
Sequence DiagramsequenceDiagram
participant Pest as Pest/Paratest
participant Worker as Test Worker Process
participant Config as Config Cache
participant Livewire as Livewire Compiler
participant Git as Git Repository
participant SQLite as SQLite Database
Pest->>Pest: config:clear
Pest->>Pest: Launch parallel workers with token
activate Worker
Worker->>Config: Load cached config
Worker->>Livewire: Initialize with isolated<br/>cache path (token-based)
Livewire->>Livewire: Use per-token<br/>compiled dir
Worker->>Git: Copy cached .git template<br/>to test repo
Worker->>SQLite: Create in-memory<br/>database
Worker->>Worker: Run test suite
deactivate Worker
Pest->>Pest: Collect results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/TestCase.php (1)
21-21: Worker-isolation logic looks correct; tri-state sentinel is the only thing I'd note.The
string|false|nullsentinel works but is a little subtle (false=unresolved,null=no token,string=path). A small readability win is to memoize "resolved" with a separateboolflag and keep the value as?string, e.g.:- /** False = unresolved, null = no token, string = path. */ - private static string|false|null $isolatedLivewireCachePath = false; + private static bool $livewireCachePathResolved = false; + private static ?string $isolatedLivewireCachePath = null; @@ - if (self::$isolatedLivewireCachePath !== false) { - return self::$isolatedLivewireCachePath; - } + if (self::$livewireCachePathResolved) { + return self::$isolatedLivewireCachePath; + } + self::$livewireCachePathResolved = true;The
putenv(VIEW_COMPILED_PATH=…)propagation strategy is sound — Symfony Process andproc_openinherit the parent env by default, so child PHP processes (e.g.BenchmarkPerformanceCommand --child) will see the worker's compiled blade dir. Good comment block at lines 34-39 explaining why the rebind is needed.Also applies to: 40-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/TestCase.php` at line 21, Replace the tri-state sentinel property private static string|false|null $isolatedLivewireCachePath = false with a clearer pair: a nullable string for the path (e.g. private static ?string $isolatedLivewireCachePath) and a separate boolean flag (e.g. private static bool $isolatedLivewireCacheResolved) to indicate resolution; update any code that checks for false/null/string to instead check the resolved flag then use the nullable path, and keep the existing putenv propagation logic intact (leave comments around the rebind behavior as-is).tests/Helpers/InteractsWithTestRepositories.php (1)
11-11: Trait-levelstaticproperty is per-using-class, not per-process —$cachedRepoTemplatereinitializes for each test class.Static properties declared in traits are bound to each class that uses the trait, not shared globally. Each test class using
InteractsWithTestRepositoriesgets its own independent$cachedRepoTemplateslot, soensureRepoTemplate()runsgit initonce per consuming class per worker — not once per process as the PR description andtests/CLAUDE.md(line 48: "per-process.gittemplate") imply.To realize the documented per-process amortization, move the cache into a dedicated small singleton class that the trait delegates to:
♻️ Suggested refactor
New file
tests/Helpers/RepoTemplate.php:<?php namespace Tests\Helpers; use Illuminate\Filesystem\Filesystem; use Illuminate\Support\Facades\File; final class RepoTemplate { private static ?string $path = null; public static function path(callable $execOrThrow): string { if (self::$path !== null) { return self::$path; } $base = sys_get_temp_dir().'/rfa_repo_tpl_'.getmypid().'_'.bin2hex(random_bytes(4)); File::makeDirectory($base, 0755, true); try { $execOrThrow( 'git init -b main -q '.escapeshellarg($base), 'Failed to initialize git repo template', ); } catch (\Throwable $e) { File::deleteDirectory($base); throw $e; } $filesystem = new Filesystem; register_shutdown_function(static function () use ($filesystem, $base): void { $filesystem->deleteDirectory($base); }); return self::$path = $base.'/.git'; } }In
InteractsWithTestRepositories, replace the trait-level cache with:private function ensureRepoTemplate(): string { return RepoTemplate::path(fn (string $cmd, string $err) => $this->execOrThrow($cmd, $err)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Helpers/InteractsWithTestRepositories.php` at line 11, The trait-level static $cachedRepoTemplate is per-consuming-class, so move the cache into a singleton helper and delegate to it: create a final Tests\Helpers\RepoTemplate with a private static $path and a public static method path(callable $execOrThrow) that initializes a single temp repo template (runs git init, registers shutdown cleanup, and stores the .git path) and returns it; then update the InteractsWithTestRepositories::ensureRepoTemplate to call RepoTemplate::path(fn($cmd,$err) => $this->execOrThrow($cmd,$err)) and remove the trait-level static $cachedRepoTemplate to ensure the template is shared per process.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpunit.xml`:
- Around line 49-55: Add force="true" to each PHPUnit <env> entry so the test
git-related variables always override any existing process environment; update
the <env> elements for GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME,
GIT_COMMITTER_EMAIL, GIT_CONFIG_COUNT, GIT_CONFIG_KEY_0 and GIT_CONFIG_VALUE_0
in phpunit.xml to include force="true" to ensure deterministic values when tests
invoke git via exec().
---
Nitpick comments:
In `@tests/Helpers/InteractsWithTestRepositories.php`:
- Line 11: The trait-level static $cachedRepoTemplate is per-consuming-class, so
move the cache into a singleton helper and delegate to it: create a final
Tests\Helpers\RepoTemplate with a private static $path and a public static
method path(callable $execOrThrow) that initializes a single temp repo template
(runs git init, registers shutdown cleanup, and stores the .git path) and
returns it; then update the InteractsWithTestRepositories::ensureRepoTemplate to
call RepoTemplate::path(fn($cmd,$err) => $this->execOrThrow($cmd,$err)) and
remove the trait-level static $cachedRepoTemplate to ensure the template is
shared per process.
In `@tests/TestCase.php`:
- Line 21: Replace the tri-state sentinel property private static
string|false|null $isolatedLivewireCachePath = false with a clearer pair: a
nullable string for the path (e.g. private static ?string
$isolatedLivewireCachePath) and a separate boolean flag (e.g. private static
bool $isolatedLivewireCacheResolved) to indicate resolution; update any code
that checks for false/null/string to instead check the resolved flag then use
the nullable path, and keep the existing putenv propagation logic intact (leave
comments around the rebind behavior as-is).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0eca9f56-704f-493e-914c-50c7f8ce9409
📒 Files selected for processing (5)
composer.jsonphpunit.xmltests/CLAUDE.mdtests/Helpers/InteractsWithTestRepositories.phptests/TestCase.php
1. Force git env vars in phpunit.xml: contributor/CI shells with GIT_AUTHOR_EMAIL etc. already set would otherwise silently override the test identity (PHPUnit's <env> only sets if not already in env unless force="true"). 2. Move the cached `.git` template out of the trait into a dedicated `RepoTemplate` class. Trait-level `static` is bound to each consuming class, so `$cachedRepoTemplate` was actually re-initializing once per test class instead of once per worker process — the docs and PR description both claim per-process. Drops the serial run from ~13.5s to ~12.8s by collapsing ~30 redundant `git init` invocations into one. https://claude.ai/code/session_01MB9KobbSHQkuGovkT5fbEj
2 participants
composer testnow drives Pest with--parallel, dropping the Coresuite from ~17s to ~6s on a 4-core box (3x). Reliability fixes:
Tests\TestCaserebindslivewire.compilerto a per-worker cachedir; the upstream singleton at
framework/views/livewire/racedacross workers and produced random "filemtime/stat failed" errors.
VIEW_COMPILED_PATHso child PHP processes(notably
BenchmarkPerformanceCommand --child) target the sameisolated blade dir as the parent test.
Per-test git overhead is also cut:
initTestRepo()copies aper-process
.gittemplate instead of runninggit initplus threegit configinvocations, and committer/author/commit.gpgsignareset globally via
GIT_*env vars on first call. Saves ~1s on aserial run.
composer test:serialis preserved as a single-process fallback fordebugging worker-isolation issues.
https://claude.ai/code/session_01MB9KobbSHQkuGovkT5fbEj
Summary by CodeRabbit
New Features
composer test:serialcommand for single-process test execution, useful for debugging scenarios.Tests
Documentation