Skip to content

Run the test suite on Orchestra Testbench - #1055

Open
dazza-dev wants to merge 1 commit into
tenancy:5.xfrom
dazza-dev:contrib/testbench-harness
Open

Run the test suite on Orchestra Testbench#1055
dazza-dev wants to merge 1 commit into
tenancy:5.xfrom
dazza-dev:contrib/testbench-harness

Conversation

@dazza-dev

@dazza-dev dazza-dev commented Aug 29, 2026

Copy link
Copy Markdown

Swaps the test bootstrap for Orchestra Testbench. It touches only require-dev and the test suite — no src/ changes, and nothing that reaches an application installing this package.

Why

tests/Test.php searches three candidate paths for a laravel/laravel checkout and requires its bootstrap/app.php:

$appPaths[] = realpath(__DIR__ . '/../vendor/laravel/laravel');
// ...
$app = require "$path/bootstrap/app.php";
$app->make(Kernel::class)->bootstrap();

That file is rewritten every major release, so the harness has to be rewritten alongside it. Laravel 11 is where it stops working entirely: the skeleton drops app/Http/Kernel.php and app/Console/Kernel.php, and bootstrap/app.php returns the result of Application::configure(...)->create(). Whoever takes on Laravel 11 support has to solve this first, whatever else they change.

Testbench ships the skeleton instead and tracks it release by release, which is what makes that future upgrade a version bump rather than another rewrite. It is also what a contributor to any other Laravel package expects to find, and laravel/laravel leaves require-dev.

What changed

tests/Test.php extends Orchestra\Testbench\TestCase. The hooks the suite already had are preserved — beforeSetUp(), duringSetUp(), $loadProviders — so no test had to change on account of the base class. The one rename is pathIdentified(string $path)prepareSkeleton(string $path), since there is no longer a path to identify; it is now called from defineEnvironment(), which still runs before the providers boot, which is what that hook is for.

Three tests reached into the skeleton directly:

Test Dependency Now
RunCommandTest Registered commands through App\Console\Kernel Resolves Illuminate\Contracts\Console\Kernel, which is what it always wanted
TenantAwareJobTest Notified an App\Models\User The assertion is about the website_id in the queue payload, so the notifiable no longer has to be a database model, and the users table stops being a dependency
RouteProviderTest Assumed the skeleton ships exactly one web route and one api route Declares the two global routes it needs

That last one is worth spelling out. The assertion was assertEquals(2, $router->getRoutes()->count()), and the 2 came from routes/web.php plus routes/api.php. The tenant route does not add to the global one, it replaces it — RouteCollection keys by method and URI, so the second GET / overwrites the first. The test was checking that without saying so. It now declares one global route the tenant file takes over and one that has to survive it, and the 2 means something.

Verification

Same tests, same assertions, same skips, before and after — 5.x as it stands versus this branch, on the same database containers:

before after
MySQL 8 128 tests, 260 assertions, 4 skipped 128 tests, 260 assertions, 4 skipped
MariaDB 11 128 tests, 260 assertions, 4 skipped 128 tests, 260 assertions, 4 skipped
PostgreSQL 128 tests, 209 assertions, 19 errors 128 tests, 209 assertions, 19 errors

The PostgreSQL errors are pre-existing and unrelated to this change: since PostgreSQL 15 a grant at the database level no longer allows creating tables in public, so tenant provisioning fails. That one is fixed separately in #1054.

Both halves of the ^9.0|^10.0 range the package declares are covered, since orchestra/testbench ^7.0|^8.0 follows the same split — Testbench 7 is Laravel 9, Testbench 8 is Laravel 10:

before after
laravel/framework v9.52.22 128 tests, 260 assertions, 4 skipped 128 tests, 260 assertions, 4 skipped
laravel/framework v10.50.3 128 tests, 260 assertions, 4 skipped 128 tests, 260 assertions, 4 skipped

The "before" column there is 5.x with laravel/laravel v9.5.2 and v10.3.3 respectively.

One thing you may want to know

composer update on 5.x currently fails on a recent Composer, before any change of mine:

Root composer.json requires laravel/framework ^9.0|^10.0, found
laravel/framework[v9.0.0, ..., v10.50.3] but these were not loaded,
because they are affected by security advisories

Composer now refuses advisory-affected versions by default, and every Laravel 9 and 10 release carries at least one. symfony/dom-crawler ~3.1 is blocked the same way. I worked around it locally with composer config --global policy.advisories.block false rather than touch composer.json here, since it is a separate concern from this PR. A config.policy.advisories.ignore-id list in composer.json would fix it properly if you want one.


Happy to rebase or split this if you would rather take it in pieces.

tests/Test.php searches three candidate paths for a laravel/laravel
checkout and requires its bootstrap/app.php. That skeleton is rewritten
every major release, so the harness has to be rewritten alongside it:
Laravel 11 drops app/Http/Kernel.php and app/Console/Kernel.php
entirely, and the current bootstrap cannot boot it at all.

Testbench ships the skeleton instead and tracks it release by release.
It is also what a contributor to any other Laravel package expects to
find, and it drops laravel/laravel from require-dev.

Three tests reached into the skeleton and had to be pointed elsewhere:

- RunCommandTest registered console commands through App\Console\Kernel.
  It now resolves the kernel contract, which is what it always wanted.
- TenantAwareJobTest notified an App\Models\User. The assertion is about
  the website_id in the queue payload, so the notifiable does not need
  to be a database model, and the users table stops being a dependency.
- RouteProviderTest counted on laravel/laravel shipping exactly one web
  route and one api route -- the tenant route replaces the global one
  rather than adding to it, since RouteCollection keys by method and
  URI, and the assertion of 2 came from routes/api.php. It now declares
  the two global routes it needs, which states out loud what overriding
  the global route means.

Nothing else in the suite changed. Same tests, same assertions, same
skips, before and after, on MySQL 8, MariaDB 11 and PostgreSQL:

    mysql     128 tests, 260 assertions, 4 skipped   (both)
    mariadb   128 tests, 260 assertions, 4 skipped   (both)
    pgsql     128 tests, 209 assertions, 19 errors   (both)

The PostgreSQL errors are pre-existing and unrelated.
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