[4.x] Testing tenants is slow

[antonkomarev]

Accordingly to the documentation testing of tenants could be done this way:

<?php

namespace Tests;

use Illuminate\Foundation\Testing\TestCase as BaseTestCase;

abstract class TestCase extends BaseTestCase
{
    use CreatesApplication;

    protected $tenancy = false;

    public function setUp(): void
    {
        if ($this->tenancy) {
            $this->initializeTenancy();
        }
    }

    public function initializeTenancy($domain = 'test.localhost')
    {
        tenancy()->create($domain);
        tenancy()->init($domain);
    }

    public function tearDown(): void
    {
        config([
            'tenancy.queue_database_deletion' => false,
            'tenancy.delete_database_after_tenant_deletion' => true,
        ]);
        tenancy()->all()->each->delete();

        parent::tearDown();
    }
}

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

[stancl]

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.

[drfraker]

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

https://laravel-news.com/snipe-migrations-laravel-package

[stancl]

Hi @drfraker. Good idea, I'll take a look at your package alongside Laravel traits for some inspiration.

[drfraker]

@stancl I also like your idea of a trait that can be added onto test classes to bypass the tenant creation. Like a NoTenants trait. I've also seen where if you add certain keywords to a test name it will do the same thing for more granular control within a test class.

eg. test_it_can_do_something_no_tenants Within the test case this would be picked up and not add tenants to that particular test. It's helpful when some tests in a class use tenants and others do not. A single trait would not be usable in that case.

[stancl]

to bypass the tenant creation

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 tenant() calls) require a tenant to be created & identified.

And to test tenant creation using your signup form, you'd use no traits.

So you really only need two things:

• a base Laravel TestCase for unit testing classes and feature testing tenant creation
• a tenant-aware trait/TestCase that creates & identifies one tenant before the tests and deletes the tenant after the tests

[drfraker]

Actually I was thinking about it a bit differently. My thought was to have 1 or more databases for testing tenants. Maybe like tenant_1_db and tenant_2_db. These would be set up beforehand by the developer. Like SnipeMigrations, it will run the migrations once then take a snapshot of the databases in the "clean" state. After initial db intialization by importing the snapshot, the databases would use database transactions for each test so that no actual data gets saved to the test tenant databases. If a tenant migration file changes the app would know about it, re-run the tenant migrations and take a new mysql snapshot for future imports. The "central" database would be pre-seeded with the tenants belonging to the 2 pre-created databases, using custom db names. Snipe already allows a seeder to run before the snapshot is taken.

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?

[stancl]

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.

[stancl]

This might be related: https://github.com/stancl/tenancy/issues/190#issuecomment-559031731

[drfraker]

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?

[stancl]

There are 3 types of tests:

• central only - these test purely central functionality, completely separate from any tenancy things. These tests can use Laravel's testing traits.
• central & tenant - tests that interact with both parts of the application, e.g. a signup form that creates tenants. The tenant creation is up to the test, so that part can't be made more performant, but we could at least store the central DB empty post-migration state in a transaction and roll back any changes to central DB after each test (this would delete tenants, domains, but only assuming the DB storage driver is used)
• tenant only - these tests don't care about central anything, they run purely in the tenant app

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:

• hold the state of an empty database seeded with one tenant and one domain for the central database. Roll back to that after every test
• hold the state of an empty, post-migration tenant database in another transaction (on the tenant connection) and use that as the base for each test

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 RefreshDatabase works, we should be able to make a similar trait.

[stancl]

To add Laravel 7 support faster, I'm putting this off for 2.4.0.

[colinmackinlay]

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.

        $this->post(route('tenant.site.area.store'),
            ['name' => 'New Location'])
            ->assertRedirect(route('tenant.site.area.index'));

I get a 404 error because the url being called is: https://host.test/site/area where it should be: https://tenant.host.test/site/area

The route works fine in the app but the test doesn't know it's in a tenant even though setup has:

        tenancy()->init('testing.host.test');

Any thoughts anyone?

[stancl]

You can specify the domain that the tenant should be redirected to if you enable the TenantRedirect feature.

return redirect()->route(...)->tenant($domain);

Otherwise the app has no way of knowing what domain to use, since tenants can have multiple domains.

[colinmackinlay]

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:

redirect()->route('tenant.site.area.store')->tenant('testing.host.test')

That works. But I can't post to a route modified by tenant which is what the test needs:

post(route('tenant.site.area.store')->tenant('testing.host.test'),['name' => 'New Location']);

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.

[stancl]

redirect()->route() returns something different than route(). route() returns a string. You need tenant_route() for route strings w/ tenant domains swapped.

I would have thought the app/test could know which domain I had initialised from tenancy()->init('testing.host.test');

No, because testing.host.test is resolved to a tenant id, and each tenant can have an infinite amount of domains.

[colinmackinlay]

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 :)

[devoncmather]

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 😊

[stancl]

With v3 coming soon, I'd like to make this part of 3.0 or 3.1.

[okihus]

Any updates on this one?

[devonmather]

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();
    }
}

[stancl]

Any updates on this one?

There's this: https://sponsors.tenancyforlaravel.com/frictionless-testing-setup

And I'll be adding database transactions to it soon.

[lewiswharf]

I've run into this issue and wonder what's the best approach currently?

[stancl]

See https://github.com/stancl/tenancy/discussions/579

[lewiswharf]

@stancl WOW, I only needed to add the following to my testing MySQL Docker container

    tmpfs:
      - /var/lib/mysql

Thank you!

[hackeresq]

Is the best practice still reflected in the https://sponsors.tenancyforlaravel.com/frictionless-testing-setup?

It works well.... but it's super slow. By hooking into the setUp (and tearDown) function, it creates a new tenant for each test method. Since I create an S3 bucket and Stripe customer whenever a TenantCreated event is called, this obviously adds some overhead to each test method call. Assuming others have similar Jobs running on the creation of new tenants.

I can't seem to figure out how to create a test tenant once, and clean up (i.e. delete test S3 bucket and test Stripe customer) once all tests are complete. Is this possible?

[sinamiandashti]

@stancl did u add db transactions to it or not ?

[hackeresq]

Ended up writing my own static method that creates a test tenant once when the first tenant-related test is run. Importantly, this means only one tenant is created.

Additionally, to help with cleanup, this method also keeps track of how many tests need to run (phpunit --list-tests | grep '-' | wc -l), and what the current test is. Then on the last test, it deletes the test tenant (which also triggers the jobs for deleting S3 bucket and Stripe customer).

The trade-off of speed vs. testing "best practice" (i.e. creating a fresh environment/application for each test) is very clear here. The code I use is super specific to my use case, but happy to answer questions on how it works.

If there's enough interest, I might be able to package it up or create a PR to include as a testing helper trait.

[sinamiandashti]

@hackerESQ ur workaround wont work properly in : php artisan test --parallel

[hackeresq]

@hackerESQ ur workaround wont work properly in : php artisan test --parallel

Another tradeoff that is acceptable for my use case...

[erikgaal]

I was running into this issue as well. I've made a slightly adapted RefreshDatabase trait that works for me. Tests were previously running for more than a second each, with this trait it was reduced back to 300ms (as it was before using multi-database tenancy).

<?php

namespace Tests;

use App\Tenant\Models\Tenant;
use Illuminate\Support\Facades\URL;
use Illuminate\Foundation\Testing\RefreshDatabase;

trait RefreshDatabaseWithTenant
{
    use RefreshDatabase {
        beginDatabaseTransaction as parentBeginDatabaseTransaction;
    }

    /**
     * The database connections that should have transactions.
     *
     * `null` is the default landlord connection
     * `tenant` is the tenant connection
     */
    protected array $connectionsToTransact = [null, 'tenant'];

    /**
     * We need to hook initialize tenancy _before_ we start the database
     * transaction, otherwise it cannot find the tenant connection.
     */
    public function beginDatabaseTransaction()
    {
        $this->initializeTenant();

        $this->parentBeginDatabaseTransaction();
    }

    public function initializeTenant()
    {
        $tenant = Tenant::firstOr(fn () => Tenant::factory()->name('Acme')->create());

        tenancy()->initialize($tenant);

        URL::forceRootUrl('http://acme.localhost');
    }
}