code-distortion / adapt

A Laravel package that builds databases for your tests, improving their speed
MIT License
24 stars 3 forks source link

The database is not switched early enough #8

Open manu144x opened 1 year ago

manu144x commented 1 year ago

Description:

So my problem is very simple. I have a table called "user_permissions" where I store a list of user permissions that become gates.

They are defined in the AuthServiceProvider.php file as per the standard documentation, like this:

public function boot()
    {
        $this->registerPolicies();

        foreach ($this->getPermissions() as $permission) {
            \Gate::define($permission->name, function (User $user) use ($permission) {
                return $user->hasPermission($permission);
            });
        }
        \Log::debug('AuthServiceProvider Is Being Executed for: ' . config('app.env') . ' with ' . count(\Gate::abilities()) . ' gates from ' . count($this->getPermissions()) . ' permissions.');
    }

    public function getPermissions()
    {
        try {
            return UserPermission::all();
        } catch(\Exception $e) {
            return [];
        }
    }

The problem is that when this executes, it's not pointing to the adapt hashed database, it's still pointing to the standard testing database mentioned in my .env.testing file. Which has a totally different state and disconnected from the tests.

I was able to reproduce it this way:

  1. Database mentioned in the .env.testing to be totally empty.

  2. The query will be empty in the AuthServiceProvider.php

  3. Database mentioned in the .env.testing is migrated and seeded

  4. The query will get the results from that database and define the gates.

I don't know the innerworkings of the package, is it possible to override the database name in the in-memory config sooner so that it uses it, or not?

code-distortion commented 1 year ago

Hi @manu144x. This is an interesting one and I have come across it before, although it's a bit clearer now.

Adapt hooks into the test process by using PHPUnit's @before docblock annotation.

When using the database scenarios setting, Adapt chooses new database name/s to use, and updates Laravel's database config values in memory to refer to them.

However by this point, it appears that Laravel has already run its service-providers and connected to the database (with the original unaltered name).

Adapt tries to remedy this as far is it can at this stage by detecting whether a connection has already been made by the time it starts, and if so, it disconnects so it can connect to the desired database instead. The logs will show something like this:

[2022-12-30 17:13:00] testing.DEBUG: ADAPT: Disconnecting established database connection "testing"

But this doesn't help you here.

I don't have a solution for you, but I have two work-arounds…

1) Don't use Adapt's database scenarios. This will tell Adapt to leave the test database names as they are. This setting is designed to avoid thrashing if you set the database up differently for different tests (e.g. different seeders). But that might not be a problem for you.

2) This one is hacky: Leave a test database there with the default name from .env.testing. Populate it with the data you want, so AuthServiceProvider at least works as intended. Not nice as this behaviour isn't clear, and it relies on the environment being curated in a certain way. Also, this won't work if the permissions are what you'd actually like to test!

This is a problem that I'll keep my thinking cap on for a solution. However don't think there's going to be a quick fix.