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

Allow for application testing using SQLite #178

Closed
wants to merge 3 commits into from

Conversation

mralston
Copy link

@mralston mralston commented Dec 7, 2023

Issue #177

Laravel Pulse doesn't support SQLite.

It is common to use SQLite whilst running tests of an application built using Laravel.

Pulse's migration throws an error if the database driver is SQLite, causing application tests to fail.

This pull request adds an App::environment('testing') check which allows SQLite during testing. Running the migration in any other environment will throw the original exception.

Here is the modified code:

match ($driver = $connection->getDriverName()) {
    'mysql' => $table->char('key_hash', 16)->charset('binary')->virtualAs('unhex(md5(`key`))'),
    'pgsql' => $table->uuid('key_hash')->storedAs('md5("key")::uuid'),
    'sqlite' => App::environment('testing') ? $table->char('key_hash', 16) : throw new RuntimeException("Unsupported database driver [{$driver}]."),
    default => throw new RuntimeException("Unsupported database driver [{$driver}]."),
};

Allows application (userland) tests that use an SQLite database to run whilst Pulse installed. Previously, the Pulse migration would throw an error stating that SQLite is not supported.
@driesvints
Copy link
Member

Just disable Pulse during tests with PULSE_ENABLED?

@taylorotwell taylorotwell marked this pull request as draft December 7, 2023 19:25
@jessarcher
Copy link
Member

Thanks for the PR!

I'm not opposed to adding SQLite support so Pulse works locally, but I'd want to see it fully implemented (i.e. SQLite-specific expressions for the upsert* methods) along with CI tests.

Agree with @driesvints solution if you just want to disable Pulse locally.

@jessarcher jessarcher closed this Dec 8, 2023
@mralston
Copy link
Author

mralston commented Dec 8, 2023

Thanks for taking the time to look at the PR @jessarcher. Very much appreciated. :)

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

Successfully merging this pull request may close these issues.

3 participants