[4.x] Testing tenants is slow #250
Comments
|
Yeah, it's on my roadmap to make a few traits like those provided by Laravel. (e.g. to only run migrations on phpunit setup rather than every single test setup). There could be a trait for tests that don't need to know about tenant creation (because they don't test the central app) and only test parts of the tenant app. That trait could create a tenant & DB on phpunit setup and then cleanup after all tests are run. |
|
@stancl I wrote a package the hijacks the Refresh Database trait. I'm not sure, but you may be able to find some useful code in there to make this happen. If I get some free time in the upcoming weeks I might just add it myself. This is a major pain point for us right now too. |
|
Hi @drfraker. Good idea, I'll take a look at your package alongside Laravel traits for some inspiration. |
|
@stancl I also like your idea of a trait that can be added onto test classes to bypass the tenant creation. Like a eg. |
Just don't do it. Don't create any tenants and just run the tests. The use for that would be limited, though. I think the opposite would be useful. A trait that creates & deletes the tenant before/after phpunit runs the tests. Any calls related to the current tenant (e.g. tenant-specific calls and inline And to test tenant creation using your signup form, you'd use no traits. So you really only need two things:
|
|
Actually I was thinking about it a bit differently. My thought was to have 1 or more databases for testing tenants. Maybe like This way we can avoid the slow part which is database creation and running migrations. Of course if a developer needed more than the pre-provisioned number of tenants for testing they could create a new one in the test. Since this isn't a requirement most of the time the tests should run really fast. Is there anything that would prevent us from going in this direction? |
|
Hmm, I see. I'll take a deeper look at SnipeMigrations. Skipping the central & tenant DB migration/seeding process even on phpunit setup could speed things up considerably. |
|
This might be related: #190 (comment) |
|
Hey @stancl I've been looking into how to make testing faster. I think a good start would be to have some flag that when set will tell the database manager that the database already exists and not to try to create one. One of the problems is that the database has to be created/migrated/deleted for every test case, which is slow. If the database does not have to be created we could presumably have a few databases set up for testing like tenant_1_test, tenant_2_test, ... and when creating test tenants simply set the _tenancy_database_name to tenant_1_test etc. If that existed, we could mimic the functionality of the RefreshDatabaseTrait on all tenant databases. Basically, that trait runs migrations and seeds once. Then sets the migrated state in RefreshDatabaseState::$migrated to true and starts a database transaction that rolls back once the test has run. On subsequent tests (when $migrated is true) it skips the migrations and just uses the transactions. If you could get a branch to the point where there was a setting to skip database creation I could probably fill in the rest of the pieces to make this work. Does this make sense? |
|
There are 3 types of tests:
The third category is what I expect to be the biggest group of tests for all apps that use this package, so let's focus on that. Assuming the DB storage driver is used, we can:
Later we can do something like Snipe migrations for even faster tests, but this is a good start. I will have to look into how the Laravel migration traits work, because Application state is not persisted between tests (for obvious reasons), but the transaction persists. My concern is that Application being destroyed between tests could also destroy the tenant connection, which would probably break the transaction. But that shouldn't be a big issue, if |
|
To add Laravel 7 support faster, I'm putting this off for 2.4.0. |
|
A technique that I'm going to try to adapt is this one that I used to really speed up tests with hyn/multitenant: https://medium.com/@sadnub/faster-multitenant-testing-in-laravel-4769eae4b603. It made a huge difference. Before that though I need to figure out how to make my existing test work now that I've converted to stancl/tenancy :) At the moment my problem is that when using a route from within a test e.g. I get a 404 error because the url being called is: The route works fine in the app but the test doesn't know it's in a tenant even though setup has: Any thoughts anyone? |
|
You can specify the domain that the tenant should be redirected to if you enable the return redirect()->route(...)->tenant($domain);Otherwise the app has no way of knowing what domain to use, since tenants can have multiple domains. |
|
Thanks for the response. Don't quite follow how that helps in the test. Certainly with that enabled I can get a redirect to a tenant page: That works. But I can't post to a route modified by tenant which is what the test needs: Returns an error: Call to a member function tenant() on string I would have thought the app/test could know which domain I had initialised from tenancy()->init('testing.host.test'); The app itself is fine because when it is live code the request is coming from the tenant domain. It's only the tests that fail, not the code. |
|
No, because |
|
Thanks - tenant_route() sorted it, I can store the domain in the TenantTestCase and just modify all my tests. Couldn't find anything in the documentation about the tenant_route() helper but got it in the helpers file. All the work converting from hyn definitely seems to be worth it though. Great package :) |
|
Looks like this has been added to V3 roadmap 👍🏻 In the meantime has anyone had relative success in setting this up on their own to speed up tests? My current test suite takes about 15 mins to run when using sqlite, and on GitHub actions on a mysql db it takes anywhere between 1-5 hours if it doesn't time out first. It's gotten to the point where I would like to try to solve it using @stancl comment from 15 jan as an initial approach. Just wanted to reach out and see if anyone has already had any luck with a solution so far. Cheers 😊 |
|
With v3 coming soon, I'd like to make this part of 3.0 or 3.1. |
|
Any updates on this one? |
|
I have had luck using mysql databases. This is a stripped-down version of my test case, the brand model is a wrapper around the tenant this is for V2. The second brand created is used for a couple of edge cases (like making sure commands run for multiple tenants). There are caveats with this approach but if there is interest in this solution I can go into a bit more detail. Running 2000+ tests takes about 5 minutes, was close to an 1+ hours before this change. abstract class TestCase extends BaseTestCase
{
use CreatesApplication, RefreshDatabase;
protected $brand;
protected bool $tenancy = false;
protected function setUp(): void
{
parent::setUp();
config(['tenancy.database.prefix' => 'tenant_test_']);
config(['tenancy.filesystem.suffix_base' => 'tenant_test']);
if ($this->tenancy) {
$this->initBrand();
DB::beginTransaction();
}
}
protected function tearDown(): void
{
if ($this->tenancy) {
DB::rollback();
}
parent::tearDown();
}
public function initBrand()
{
$this->brand = Brand::first();
Brand::init($this->brand->domain);
}
/**
* Override the refresh trait for the conventional test database.
*
* @return void
*/
protected function refreshTestDatabase()
{
config(['tenancy.storage_drivers.db.cache_ttl' => 60]);
config(['tenancy.database.prefix' => 'tenant_test_']);
config(['tenancy.filesystem.suffix_base' => 'tenant_test']);
if (!RefreshDatabaseState::$migrated) {
$this->artisan('migrate');
$this->artisan('tenants:migrate');
if (!Brand::first()) {
factory(Brand::class)->create([
'subdomain' => 'tenant-test',
]);
}
if (!Brand::skip(1)->first()) {
factory(Brand::class)->create([
'subdomain' => 'tenant-test-2',
]);
}
$this->app[Kernel::class]->setArtisan(null);
RefreshDatabaseState::$migrated = true;
}
$this->beginDatabaseTransaction();
}
} |
There's this: https://sponsors.tenancyforlaravel.com/frictionless-testing-setup And I'll be adding database transactions to it soon. |
|
any update ? and maybe migrate to using LazilyRefreshDatabase |
|
Nope still slow. Tried a range of things too but it needs to migrate each run. Using on a legacy project and has a lot of migrations. |
|
In my case, I was able to run tests much faster using schema:dump command. To create a tenant schema dump you can run - |
Can you elaborate? Perhaps share some code? |
|
@plakhin did you ever manage to get this to work with |
|
@francoisauclair911, I've managed to get database refreshing before each test, will post it here later, currently AFK. However it's not as fast as using transactions. |
trait RefreshDatabaseWithTenant
{
use RefreshDatabase {
beginDatabaseTransaction as parentBeginDatabaseTransaction;
}
...
protected static function addAfterClass()
{
app()->make('db')->connection()->disconnect();
tenant()->delete();
}
...abstract class TestCase extends BaseTestCase
{
...
public static function tearDownAfterClass(): void
{
if (method_exists(static::class, 'addAfterClass')) {
method_exists(static::class, 'addAfterAll')
? (new static(fn () => null, '', []))->setUp()
: (new static())->setUp();
static::addAfterClass();
}
parent::tearDownAfterClass();
}
... |
How long would you say it would reduce this Trait? |
|
It depends, just try in your project |
|
I'll reopen this since testing is something we want to focus on in v4 👍🏻 |
How are things going with V4? |
|
Here's what I have been using so far and it has been working perfectly trait WithTenancy
{
protected function setUpTraits(): array
{
$uses = parent::setUpTraits();
if (isset($uses[WithTenancy::class])) {
$this->initializeTenancy($uses);
}
return $uses;
}
protected function initializeTenancy(array $uses): void
{
$organization = Organization::firstOr(static fn () => Organization::factory()->create());
tenancy()->initialize($organization);
if (isset($uses[DatabaseTransactions::class]) || isset($uses[RefreshDatabase::class])) {
$this->beginTenantDatabaseTransaction();
}
if (isset($uses[DatabaseMigrations::class]) || isset($uses[RefreshDatabase::class])) {
$this->beforeApplicationDestroyed(function () use ($organization) {
$organization->delete();
});
}
}
public function beginTenantDatabaseTransaction(): void
{
$database = $this->app->make('db');
$connection = $database->connection('tenant');
$dispatcher = $connection->getEventDispatcher();
$connection->unsetEventDispatcher();
$connection->beginTransaction();
$connection->setEventDispatcher($dispatcher);
$this->beforeApplicationDestroyed(function () use ($database) {
$connection = $database->connection('tenant');
$dispatcher = $connection->getEventDispatcher();
$connection->unsetEventDispatcher();
$connection->rollBack();
$connection->setEventDispatcher($dispatcher);
$connection->disconnect();
});
}
}Then on the tests that need it I just have to reference it like so: PestPHPuse Illuminate\Foundation\Testing\DatabaseTransactions;
use Tests\WithTenancy;
uses(WithTenancy::class);
uses(DatabaseTransactions::class);
// ...PHPUnitnamespace Tests\Feature;
use Illuminate\Foundation\Testing\DatabaseTransactions;
use Tests\TestCase;
use Tests\WithTenancy;
class ExampleTest extends TestCase
{
use WithTenancy;
use DatabaseTransactions;
// ...
} |
|
@viicslen It looks very nice and elegant. But how is the speed? Did you have it in a different way before? |
|
When used with PS: I have also setup a custom version of the schema dump functionality for the tenant database |
For this to work, do I have to have the database already created and empty? |
|
Fixed in v4 |
When will it be available to the general public? |
|
There's no strict ETA yet. For now it's open in early access to sponsors. See #announcements on our Discord. |
|
@viicslen can you share a complete setup / config? Meaning Pest.php, TestCase, tenant aware test, and a central aware test? |
|
Here is what i have done with mine. I have heavy seeders that import large xmls so i cannot afford to keep migrating and seeding <?php
namespace Tests;
use App\Models\Tenant;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
use Illuminate\Support\Facades\Artisan;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\URL;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
protected $tenancy = false;
/**
* If true, setup has run at least once.
*
* @var boolean
*/
protected static $setUpRun = false;
/**
* Set up the test.
*/
public function setUp(): void
{
parent::setUp();
if (!static::$setUpRun) {
Artisan::call('migrate:refresh', ['--seed' => true]);
static::$setUpRun = true;
config(['tenancy.database.prefix' => 'yopractice_testing_' . \Str::random() . '_']);
//create test tenant
if (!Tenant::count()) {
$tenant = Tenant::create([
'name' => 'Test'
]);
$tenant->createDomain("test");
}
//drop our test databases
}
if ($this->tenancy) {
$this->initializeTenancy();
}
}
public function initializeTenancy(): void
{
$tenant = Tenant::find(1);
tenancy()->initialize($tenant);
URL::forceRootUrl('http://' . $tenant->domains->first()->domain . '.' . config('app.central_domain'));
}
} |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
In the event it helps someone, here is what I'm doing to test Central and Tenants. I'm using PEST, however it will work just fine with standard PHPUnit. Database transactions are working for tenants so things are really fast, parallel testing is working, pest is working, code coverage, etc all work. I'm using Laravel Sail, so you may need to adjust URL's, etc., for other environments. Of course, make sure you have MySQL setup properly first: https://tenancyforlaravel.com/docs/v3/integrations/sail Central Tests <?php
uses(\Illuminate\Foundation\Testing\RefreshDatabase::class);
test('the admin login page returns a successful response', function () {
$this->get(route('filament.admin.auth.login'))
->assertSuccessful();
});Tenant Tests <?php
uses(\Tests\Traits\RefreshDatabaseWithTenant::class);
test('the portal login page returns a successful response', function () {
$this->get(route('filament.portal.auth.login'))
->assertSuccessful();
});Here is my <?php
namespace Tests\Traits;
use App\Models\Tenant;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\ParallelTesting;
use Illuminate\Support\Facades\URL;
trait RefreshDatabaseWithTenant
{
use RefreshDatabase {
beginDatabaseTransaction as parentBeginDatabaseTransaction;
}
/**
* The database connections that should have transactions.
*
* `null` is the default landlord connection, used for system-wide operations.
* `tenant` is the tenant connection, specific to each tenant in the multi-tenant system.
*/
protected array $connectionsToTransact = [null, 'tenant'];
/**
* We need to hook initialize tenancy _before_ we start the database
* transaction, otherwise it cannot find the tenant connection.
* This function initializes the tenant setup before starting a transaction.
*/
public function beginDatabaseTransaction()
{
// Initialize tenant before beginning the database transaction.
$this->initializeTenant();
// Continue with the default database transaction setup.
$this->parentBeginDatabaseTransaction();
}
/**
* Initialize tenant for testing environment.
* This function sets up a specific tenant for testing purposes.
*/
public function initializeTenant()
{
// Hardcoded tenant ID for testing purposes.
$tenantId = 'foo';
// Retrieve or create the tenant with the given ID.
$tenant = Tenant::firstOr(function () use ($tenantId) {
/**
* Set the tenant prefix to the parallel testing token.
* This is necessary to avoid database collisions when running tests in parallel.
*/
config(['tenancy.database.prefix' => config('tenancy.database.prefix') . ParallelTesting::token() . '_']);
// Define the database name for the tenant.
$dbName = config('tenancy.database.prefix') . $tenantId;
// Drop the database if it already exists.
DB::unprepared("DROP DATABASE IF EXISTS `{$dbName}`");
// Create the tenant and associated domain if they don't exist.
$t = Tenant::create(['id' => $tenantId]);
if (!$t->domains()->count()) {
$t->domains()->create(['domain' => $tenantId]);
}
return $t;
});
// Initialize tenancy for the current test.
tenancy()->initialize($tenant);
// Set the root URL for the current tenant.
URL::forceRootUrl('http://foo.localhost');
}
} |
|
@citricguy How long does a test normally take to initialize the tenant? I used your code and it takes about 8 to 10 seconds to complete a basic test. |
|
Maybe 0.5 to 1s for the trait initialization and then 0.05s to 0.2s per test after that in the same class. Running tests in parallel cut the time down a bit more. It is very similar to running tests without tenancy with DatabaseTransactions given that it is exactly the same concept. I do a lot of TDD so slow tests freak me out. 🤣 This setup is nearly fully credited to @plakhin (#250 (comment)) Here are the results of a |
|
The solution @citricguy provided works perfectly. However, I am using SQLite so I had to make some modifications. For the people interested: public function initializeTenant()
{
// Hardcoded tenant ID for testing purposes.
$baseDomain = 'domain.test';
$tenantId = 'pest';
$tenantDomain = $tenantId.'.'.$baseDomain;
// Retrieve or create the tenant with the given ID.
$tenant = Tenant::firstOr(function () use ($tenantId, $tenantDomain) {
/**
* Set the tenant prefix to the parallel testing token.
* This is necessary to avoid database collisions when running tests in parallel.
*/
config(['tenancy.database.prefix' => config('tenancy.database.prefix').ParallelTesting::token().'_']);
// Define the database name for the tenant.
$dbName = config('tenancy.database.prefix').$tenantId;
// Drop the database if it already exists.
$dbPath = database_path("{$dbName}");
// Check if the file exists and delete it
if (File::exists($dbPath)) {
File::delete($dbPath);
}
// Create the tenant and associated domain if they don't exist.
$t = Tenant::create(['id' => $tenantId]);
if (! $t->domains()->count()) {
$t->domains()->create(['domain' => $tenantDomain]);
}
return $t;
});
// Initialize tenancy for the current test.
tenancy()->initialize($tenant);
// Set the root URL for the current tenant.
URL::forceRootUrl("http://$tenantDomain");
} |
and others
Accordingly to the documentation testing of tenants could be done this way:
But if there is a lot of tests - it will be very slow. In my case ~3 seconds for each test with more than 1k tests (~50 minutes).
The text was updated successfully, but these errors were encountered: