-
Notifications
You must be signed in to change notification settings - Fork 3
pgsql driver #1
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
Comments
Hi Bruno. Thanks for your message! This is the first one. Adding pgSQL support is something I would like to do. But it isn't high on my priority list as I don't have a need for it, or much pgSQL experience at this stage. |
Hi Bruno. I took it up as a challenge and added support for PostgreSQL. If you try the package, I'm interested in feedback on how your experience was. |
Thanks for the response. Adapt can log a bunch of details about how it builds and reuses the database for each test. Would you mind adding The And the |
The idea is that When you run a test, and it needs a database for the first time, Adapt builds the database and the output looks a bit like this:
And then the next time the test runs, the database is re-used and output looks a bit like this (the bottom section shows when the database is re-used):
|
migrations and build_databases with [2022-05-21 16:01:51] testing.DEBUG: ADAPT: Looking for stale things to remove [2022-05-21 16:02:52] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ────────────────────────────────────────────┐ [2022-05-21 16:02:54] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────┐ [2022-05-21 16:02:59] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────────────────────┐ |
Just the first test passed, after more than 1 minute, the other fails, i stopped the execution after like 5 minutes, since all tests should be finished in ~ 140s |
Thanks. Got it. There's two things happening:
Postgres added the option to force it to close existing connections before dropping a database recently in version 13. From my experience I didn't find that I needed it, but I can see it might be important here. I'll have a look at adding it in. Do you use any other tools that might also connect to the database? There are several reasons why it might decide that the database can't be reused. Ultimately it's checking to make sure that the database was left in a clean state, ready for the next test. I'll also add some extra logging to see why in this case it didn't think the database could be reused. I'll reply back when I've made the update. |
Thanks for your help. I've released version 0.10.2 with those changes. Would you mind updating to this, and perform the same steps. Then copy and paste from the logs again? And in case it helps, would you mind letting me know which version you use of:
|
Logs: [2022-05-25 10:58:51] testing.DEBUG: ADAPT: Looking for stale things to remove [2022-05-25 10:59:52] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ────────────────────────────────────────────┐ [2022-05-25 10:59:55] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────┐ [2022-05-25 10:59:57] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────────────────────┐ [2022-05-25 10:59:59] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ────────────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:02] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────────────────┐ [2022-05-25 11:00:04] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ─────────────────────────────────────────────────────┐ [2022-05-25 11:00:06] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ───────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:09] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ───────────────────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:11] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ────────────────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:14] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ─────────────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:16] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ───────────────────────────────────────────────────────────────────────────────────────────────────┐ [2022-05-25 11:00:18] testing.DEBUG: ADAPT: ┌── ADAPT - Preparing a Test-Database ──────────────────────────────────────────────────────────────────────────────┐ Slow "boot" before testing, slow between each test and with errors. |
Thanks Bruno, I'll have a look at these tonight |
Hi Bruno. Thanks again for giving this a test. It looks like it's dropping the database now when it tries to, which is a step forward.
This section here is run once at the beginning of the test-run. It's purpose is to clean up old databases when your any of your migrations, seeders or factories change. It does this by first generating a "build-hash" based on these directories. If any files change, this hash changes. Then it connects to your database servers (based on your config's available I can see this is taking a long time to search for databases (1 minute!). This indicates to me that you have lots of databases to search through, but 1 minute seems excessive. It might be because of a time-out of some sort. I'd like to try and add extra logging to show the steps it's taking here in more detail. In the time being, you can disable this clean-up step by adding (You can always run The second part of the problem is more confusing. After building a database, Adapt creates the table This is the step where it creates this meta-table:
However every time the next test runs, the table doesn't exist! I've checked that this package can work against PostgreSQL 8.4 - 14, however perhaps the CREATE TABLE query is failing for some reason in your circumstance... I'd like to add better checking around creation of this table, to try and understand what's happening better. I'll get back to you when I've had a good think and look through the code, and added in extra checking + logging. |
Hi Bruno, thanks for your patience. I just wanted to let you know that I haven't forgotten about this. |
That's ok, I'm glad to help. |
Hi Bruno. I reproduced the error you found!
I found this happens when using one of Laravel's own database-building traits: All you need to do is remove Laravel's traits from your tests. They kind of clash when running at the same time. I've added a check, Adapt now generates an exception if they're present. I've released a new version 0.11.0, which includes lots of additional logging. I'm still very interested to see where that whole minute is spent at the beginning, when it looks for stale test-databases to remove. Would you mind updating to this version, and having another try? I renamed a number of config settings in this release (I don't want to do this often). So if you published the config file before, please re-publish it again: php artisan vendor:publish --provider="CodeDistortion\Adapt\AdaptLaravelServiceProvider" --tag="config" Previously, I asked you to set Also, I added a new log-verbosity setting, with 0 being the lowest and 2 currently being the highest.
|
Hi, my Pest.php <?php
uses(Tests\TestCase::class)->in(__DIR__); My TestCase.php <?php
namespace Tests;
use CodeDistortion\Adapt\AdaptDatabase;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
use AdaptDatabase;
} Config (everything directly in the file, no ENV variables) https://gist.github.com/ibrunotome/da13d4c181f2c8b1cf4a8cff12320564 Result: tests stars only after 1 minute and all them fail. Log: [2022-06-13 12:16:37] testing.DEBUG: ADAPT: Looking for stale things to remove [2022-06-13 12:17:37] testing.DEBUG: ADAPT: ┌── ADAPT (v0.11.0) ──────────────────────────────────────────────────────────────┐ [2022-06-13 12:17:37] testing.DEBUG: ADAPT: ┌── ADAPT (v0.11.0) ────────────────────────────────────────────────────────────────┐ |
Thanks very much. Sorry, I was wrong, I was really confident I'd found the cause of the issue. I'm glad I added the check for Laravel's traits, but there's more looking I need to do into Pest. Regarding the start-up delay, Adapt connects to each connection listed in I'm pretty sure you don't have a MySQL server set up so this particular step is unnecessary. You can skip this by commenting out unused connections from I will have a look to see if I can reproduce the delay, and if there's a way to mitigate it. |
Hi Bruno, I haven't found any problems when running Pest and PostgreSQL together, however there is something else I've looked in to that might cause the problem you've found. I was aware of an issue, but it occurred in more obscure circumstances: Adapt chooses the names of its databases dynamically. If Laravel connects to the database before Adapt runs, that connection and database name is maintained, even though Adapt thinks it's picking and choosing a new database to use. When this happens, Adapt won't work correctly. I've found a few packages that cause this to happen, but not when running tests from the command-line. These packages connect to the database before Adapt runs when browser test requests come in:
I'm sure there's many more. It's possible that something like this is causing Laravel to connect to the database for you, before your tests run. To counter this, I've added a check when Adapt boots up, to see if any database connections have already been established. If so, they're disconnected from before proceeding. After updating to the latest version of Adapt, if you see a line like this in your logs, then we'll know this has been happening.
Regarding the start-up delay, I've reproduced the 1 minute connection time-out. When the PDO MySQL client tries to connect to a PostgreSQL server, it seems to cause this large delay. I'm still thinking about how to tackle this problem. This clean-up feature could be disabled as I definitely want users to have a quick first experience when using the package. However, having Adapt clean up old databases is a useful part of the system. For right now, my previous suggestions are still relevant. You can avoid the problem by commenting out unused connections in return [
// …
'connections' => [
// 'sqlite' => [
// 'driver' => 'sqlite',
// 'url' => env('DATABASE_URL'),
// 'database' => env('DB_DATABASE', database_path('database.sqlite')),
// 'prefix' => '',
// 'foreign_key_constraints' => env('DB_FOREIGN_KEYS', true),
// ],
//
// 'mysql' => [
// 'driver' => 'mysql',
// 'url' => env('DATABASE_URL'),
// 'host' => env('DB_HOST', '127.0.0.1'),
// 'port' => env('DB_PORT', '3306'),
// 'database' => env('DB_DATABASE', 'forge'),
// 'username' => env('DB_USERNAME', 'forge'),
// 'password' => env('DB_PASSWORD', ''),
// 'unix_socket' => env('DB_SOCKET', ''),
// 'charset' => 'utf8mb4',
// 'collation' => 'utf8mb4_unicode_ci',
// 'prefix' => '',
// 'prefix_indexes' => true,
// 'strict' => true,
// 'engine' => null,
// 'options' => extension_loaded('pdo_mysql') ? array_filter([
// PDO::MYSQL_ATTR_SSL_CA => env('MYSQL_ATTR_SSL_CA'),
// ]) : [],
// ],
'pgsql' => [
'driver' => 'pgsql',
'url' => env('DATABASE_URL'),
'host' => env('DB_HOST', '127.0.0.1'),
'port' => env('DB_PORT', '5432'),
'database' => env('DB_DATABASE', 'forge'),
'username' => env('DB_USERNAME', 'forge'),
'password' => env('DB_PASSWORD', ''),
'charset' => 'utf8',
'prefix' => '',
'prefix_indexes' => true,
'search_path' => 'public',
'sslmode' => 'prefer',
],
// 'sqlsrv' => [
// 'driver' => 'sqlsrv',
// 'url' => env('DATABASE_URL'),
// 'host' => env('DB_HOST', 'localhost'),
// 'port' => env('DB_PORT', '1433'),
// 'database' => env('DB_DATABASE', 'forge'),
// 'username' => env('DB_USERNAME', 'forge'),
// 'password' => env('DB_PASSWORD', ''),
// 'charset' => 'utf8',
// 'prefix' => '',
// 'prefix_indexes' => true,
// // 'encrypt' => env('DB_ENCRYPT', 'yes'),
// // 'trust_server_certificate' => env('DB_TRUST_SERVER_CERTIFICATE', 'false'),
// ],
],
// …
]; Or you can disable the clean-up feature by adding this to your
|
Ok, the package starts to work after comment the other database connections, but:
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
use AdaptDatabase;
protected function setUp(): void
{
parent::setUp();
DB::transactionLevel() && DB::rollBack();
}
} because of: bavix/laravel-wallet#463 (comment) With this setting, my tests run with 268 seconds with debug enabled, 188 seconds with debug disabled, all tests passed successfully (just to remember, ~154s without your pacakge) If I remove the Tests: 58 failed, 1 skipped, 280 passed (because of that linked issue above) Anyway, I don't know if this info is relevant 😆 |
Running with --parallel flag:
Anyway, i never had good results with parallel test, mostly because almost all of my tests use database. |
Hi Bruno, Great. It looks like things are progressing. There's a few things here to unpack (I'm sorry, this response is quite long): First: I've spun the issue of timing-out when looking for stale databases to remove into a separate issue. I think it's important to look into, but unrelated to the main things happening here. Secondly: Regarding the message Because Adapt is showing the log message, this confirms that this is what was causing the error you found earlier:
This now means that Adapt should be able to create and use the database it intends to, which is a great step forward. What I can't be sure of is what the other thing that's connecting to the database is doing, and if this disconnection + connection to a new database will cause a problem for that thing. debugbar and telescope are just two that I'm aware of that can connect in some circumstances (not when tests run, but when browser-tests make their requests back to the application), but I'm sure there are other things that connect before tests run. If anybody has some info I'm interested to hear. Thirdly: (I think that the tests that use Now that Adapt can create and connect to the correct database properly, I'd like to isolate Adapt away from laravel-wallet for a second, and try a simple test. Would you mind trying this test, and copy the logs here?
<?php
/*
|--------------------------------------------------------------------------
| Test Case
|--------------------------------------------------------------------------
|
| The closure you provide to your test functions is always bound to a specific PHPUnit test
| case class. By default, that class is "PHPUnit\Framework\TestCase". Of course, you may
| need to change it using the "uses()" function to bind a different classes or traits.
|
*/
uses(Tests\TestCase::class)->in('Feature');
<?php
namespace Tests;
use Illuminate\Foundation\Testing\TestCase as BaseTestCase;
abstract class TestCase extends BaseTestCase
{
use CreatesApplication;
// i.e. no RefreshDatabase, LazilyRefreshDatabase or DB::rollBack();
}
<?php
use App\Models\User;
use CodeDistortion\Adapt\AdaptDatabase;
uses(AdaptDatabase::class);
it('creates a user 1', function () {
$user = User::factory()->create();
$this->assertTrue(User::query()->latest('id')->first()->is($user));
});
it('creates a user 2', function () {
$user = User::factory()->create();
$this->assertTrue(User::query()->latest('id')->first()->is($user));
});
Then run: Fourthly: I've read the bavix/laravel-wallet#463 thread and here are my observations:
I think that looking at the speed of your tests would be the wrong thing to start with here. The crux of the problem is that when testing laravel-wallet, you can't use transactions to restore the database back to a clean state. This happens in other situations too, like when running browser tests, for example. The only ways to approach this is to use an alternative method to restore the database to the clean state, or to build the database again for each test. As an alternative method, you could try to return it to a clean state yourself. Like you mentioned, you tried by truncating tables. This gives you the responsibility of making sure the database is clean. It might work well, but I can't really comment or help you with this. If you want to build the database for each test to be sure it's clean, you have a few options:
Lastly: Parallel testing - I'm interested to see the logs from a test where Adapt couldn't create the database. (Getting logs from parallel tests is trickier because multiple processes write to it at the same time. You could reduce your test-suite down to a single test, and run that in parallel to make it clearer) |
Interestingly, bavix/laravel-wallet's use of transactions is the intended use case for using transactions. It's very literally the classic example of some code that wants to debit or credit an account, and ensure that some other things happen as well. I think it's understandable that they'd want to use transactions. |
His package forces to use his own transaction implementation (with cache) https://bavix.github.io/laravel-wallet/#/transaction (didn't tested your suggestions in the last comment yet) |
Hi Bruno. I just wanted to thank you. Because of your queries, I've been able to address these problems, which make Adapt more robust:
I've released a new version of Adapt - 0.12.0. And along with this release I've also created a dedicated site for Adapt's documentation. I think this format has helped me explain the concepts better that a README.md could. (I'm sorry but the config file's structure has changed again. I wanted to improve the structure, and as far as configs go, doing so sooner is better because less people will be affected. I think this format is easier to understand). I'm still interested to find out what your experience like is when using Adapt with a Postgres database, when running tests that don't use bavix/laravel-wallet (i.e. tests that don't need transactions themselves). And I'm still thinking of ways to isolate the problem you found when parallel testing. Having lots of tests that use databases shouldn't really be a reason for tests to fail.
|
Hi Bruno. Great, I'm glad it's working more out-of-the-box for you now. Don't worry about other use-cases. The feedback you've been able to give from your existing tests has been very valuable. Thanks for letting me know about the read / write configuration issue. Adapt uses your database details to create new databases etc. But it doesn't yet know about the config structure when using Laravel's read / write option. I'll have a look in to this. And, my name is Tim. |
Hi, I found your article about fast database tests and then this package, pgsql driver is in your plans?
The text was updated successfully, but these errors were encountered: