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

[slim-skeleton-11.x] Test Improvements #49111

Merged
merged 6 commits into from
Nov 24, 2023

Conversation

crynobone
Copy link
Member

Using $this->artisan() with Command::$aliases doesn't work properly at the moment. I believe this is due to how Symfony registers commands vs how Laravel resolves it with $signature.

Signed-off-by: Mior Muhammad Zaki <[email protected]>
@crynobone crynobone changed the title Test Improvements [slim-skeleton-11.x] Test Improvements Nov 24, 2023
@crynobone
Copy link
Member Author

@Jubeki There seems to be some collation-related issue with MySQL 5.7 tests and MariaDB tests

@crynobone crynobone marked this pull request as draft November 24, 2023 03:35
@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

@crynobone That kind of was to be expected, because MySQL 5.7 doesn't support the new collation. Based on https://laravel.com/docs/master/database#introduction I thought MySQL 8.0+ is only supported, which was one of the reasons for the change. Another reason was laravel/laravel#6239 which was reverted by laravel/laravel#6240

Now there are also discussions about reverting the Drop of MySQL 5.7 Support laravel/docs#9113

Possible solutions:

  • Creating a new config entry mysql57 in Testbench-Core with the old collation and testing against that.
  • In the Tests check for mysql57 and replace the collation on the fly.
  • Comment out mysql57 tests. (not recommended, if MySQL 5.7 Support will be retained)

The default for new Laravel 11 Applications should be the newer collation. Even if MySQL 5.7 is supported / deprecated in Laravel 11, we can give a meaningful error solution via Ignition (See laravel/docs#9113 (comment))

@crynobone
Copy link
Member Author

Is there any good approach to detect MySQL 5.7 vs 8 as well as MariaDB versions?

@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

My only idea for now: Is the collation available or not. Will research a little bit.

@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

The following should work (needs to be tested)

SHOW VARIABLES LIKE 'version';

# or
SELECT VERSION();

Jubeki and others added 3 commits November 24, 2023 09:26
@crynobone
Copy link
Member Author

@Jubeki Found a better solution, allow setting MYSQL_COLLATION and handled it via Testbench.

@crynobone crynobone marked this pull request as ready for review November 24, 2023 09:02
@Jubeki
Copy link
Contributor

Jubeki commented Nov 24, 2023

@crynobone cool 👍

Do you want to merge my PR #49113 into this one?

@crynobone
Copy link
Member Author

Do you want to merge my PR #49113 into this one?

Yes please

[slim-skeleton-11.x] Fix MariaDB Tests with changed query
@taylorotwell taylorotwell merged commit f0111bf into slim-skeleton-11.x Nov 24, 2023
17 checks passed
@taylorotwell taylorotwell deleted the slim-skeleton-tests branch November 24, 2023 14:33
taylorotwell pushed a commit that referenced this pull request Nov 28, 2023
* Test Improvements

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* Fix MariaDB Tests with changed query

* Fix Testbench version

* Allow to override collation using DB_COLLATION

Signed-off-by: Mior Muhammad Zaki <[email protected]>

* wip

Signed-off-by: Mior Muhammad Zaki <[email protected]>

---------

Signed-off-by: Mior Muhammad Zaki <[email protected]>
Co-authored-by: Julius Kiekbusch <[email protected]>
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