Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create documentation about testing #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vpratfr
Copy link

@vpratfr vpratfr commented Nov 2, 2018

Getting started with testing. Related to tenancy/multi-tenant#627.

Getting started with testing. Related to tenancy/multi-tenant#627.
@luceos
Copy link
Contributor

luceos commented Nov 2, 2018

Looking extremely good, but next time don't post in the issues, you can link to them from here and the PR will be shown automatically.

As to the contents of the page, I see some code that we could potentially move over to the framework to make it widely available. Would you be up to attempt a PR at the hyn/multi-tenant codebase?

@vpratfr
Copy link
Author

vpratfr commented Nov 2, 2018

Sorry about the issues reference missing.

I am currently really overloaded so I don't think I get any time to PR something clean any time soon.

I guess that should more or less consist in making that driver + the base test class available and provide some ways to customize a few things such as the hostname, etc.

Regards

@PtyMatt
Copy link

PtyMatt commented Nov 19, 2018

It is good news!

There are still some points to check :

  • miss the following use statements in TestSQLiteDriver:

use Hyn\Tenancy\Contracts\Webserver\DatabaseGenerator;
use Hyn\Tenancy\Database\Connection;
use Hyn\Tenancy\Events\Websites\Created;
use Hyn\Tenancy\Events\Websites\Deleted;
use Hyn\Tenancy\Events\Websites\Updated;

  • When using <env name="TENANCY_DATABASE" value=":memory:"/> I get the following error Illuminate\Database\QueryException: SQLSTATE[HY000]: General error: 1 no such table: permissions (SQL: select * from "permissions") like the migrations weren't done. Do you know what I'm missing?

  • When using an sqlite file database (<env name="TENANCY_DATABASE" value="database/database.sqlite"/>), it works for my part but the system and tenant use the same sqlite file. Is it intentional?

Thanks.

@vpratfr
Copy link
Author

vpratfr commented Nov 19, 2018

Hi,

Feel free to amend the PR, this should get most people started with little work.

Just noticed the namespace could me made more standard: Tests\Infrastructure to Tests instead.

Make sure you get all the proper env variable names correct in your .env and phpunit.xml files.

This setup is intended to maximize speed. Hence we specifically target the :memory: database and thus I did not bother looking how we could have 2 distinct DB (because there can only be one in memory unfortunatly).

@vpratfr
Copy link
Author

vpratfr commented Dec 24, 2018

Small addition here for the record. I was struggling with testing jobs: even though everything was running properly in production, in our tests, the models could not be restored as the tables were not found.

After digging in the package, it appears that the SQLite driver clears the DB fully when asked to purge + reconnect.

This is happening every time we are switching tenants. And Jobs switch tenants when they get restored to be executed (even with the Sync queue driver).

The workaround is to replace the Connection class from the package with another one which skips purge/reconnect.

Basically we have to override the public function set($to, $connection = null): bool with one doing the same thing without reconnecting. And for safety, also override the purge method to make sure this does not happen.

Here is the class:

class TestConnection extends \Hyn\Tenancy\Database\Connection
{

    public function set($to, $connection = null): bool
    {
        $connection = $connection ?? $this->tenantName();
        $website = $this->convertWebsiteOrHostnameToWebsite($to);
        $existing = $this->configuration($connection);

        if ($website)
        {
            // Sets current connection settings.
            $this->config->set(
                sprintf('database.connections.%s', $connection),
                $this->generateConfigurationArray($website)
            );
        }

        if (Arr::get($existing, 'uuid') === optional($website)->uuid)
        {
            $this->emitEvent(
                new Events\Database\ConnectionSet($website, $connection, false)
            );

            return true;
        }

        // Purges the old connection.
//        $this->db->purge(
//            $connection
//        );
//
//        if ($website) {
//            $this->db->reconnect(
//                $connection
//            );
//        }

        $this->emitEvent(new Events\Database\ConnectionSet($website, $connection));

        return true;
    }

    public function purge($connection = null)
    {
    }
}

We then need to register this in the container before the package providers. Hence we cannot do that within the TestCase but inside one of our application service providers with that code:

        if ($this->app->environment()==='testing')
        {
            $this->app->bind(Hyn\Tenancy\Database\Connection::class, TestConnection::class);
        }

@jezzdk
Copy link

jezzdk commented Apr 5, 2019

Just what I needed, thank you very much! Now all my tests are green again :)

@kylekz kylekz mentioned this pull request Mar 12, 2020
@mattvb91
Copy link

Hi @vpratfr is there any chance of getting an updated version for this? Pitty this was never accepted into the documentation because im currently struggling testing with the tenantAware testcase

@vpratfr
Copy link
Author

vpratfr commented Mar 15, 2022

Hi,

sorry, I am not using this library anymore. Switched to the one from spatie which is lighter and fits my use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants