![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 6 years ago
Closed 6 years ago
#49367 closed defect (bug) (fixed)
MariaDB Galera Cluster fails tests due to MyISAM being forced
| Reported by: | xkon | Owned by: | SergeyBiryukov |
|---|---|---|---|
| Priority: | normal | Milestone: | 5.4 |
| Component: | Build/Test Tools | Version: | |
| Severity: | normal | Keywords: | dev-feedback has-patch |
| Cc: | Focuses: |
Description
I'm opening this ticket after the weekly hosting-community meeting on Slack.
The original report can be found on the phpunit-test-runner Github issues at https://github.com/WordPress/phpunit-test-runner/issues/89 .
It seems that we're forcing the test setup here to use MyISAM , https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/dbdelta.php#L49 which isn't supported in some environments.
Specifically in this case the MariaDB Galera Cluster only supports InnoDB/XtraDB so the tests are failing.
As WordPress doesn't require MyISAM in general & if possible we should be adding a check here to see what engine is supported and change from MyISAM to InnoDB respectively to avoid issues like this.
Attachments (1)
Change History (8)
This ticket was mentioned in Slack in #hosting-community by amykamala. View the logs.
6 years ago
#4
follow-up:
↓ 5
@
6 years ago
@SergeyBiryukov after checking out the tests, in general, I see plenty of explicitly stated MyISAM i.e.
https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/dbdelta.php#L453
https://github.com/WordPress/wordpress-develop/blob/master/tests/phpunit/tests/dbdelta.php#L478
And the list goes on ( all instances are within the dbdelta.php though so it makes it kinda easier ), should those be updated also in a similar manner? I'm guessing that the assertSame would be failing there if the tables are on InnoDB, correct? Sorry, I missed this before I didn't think of making a full search 😅.
Not sure at all on this btw so I'm just asking, tests pass on my end but trying to think if there's something that I'm missing :D
#5
in reply to: ↑ 4
@
6 years ago
- Resolution fixed
- Status closed → reopened
Replying to xkon:
after checking out the tests, in general, I see plenty of explicitly stated MyISAM
Indeed, thanks for catching that! Yes, those should be updated in a similar manner.
#6
@
6 years ago
- Keywords has-patch added; needs-patch removed
49367.diff creates a new static function to force the engine if needed and changes all MyISAM statements to dynamically be populated by the function.
I believe this should do the trick on all of them.
In 47193: