Fix MariaDB support in FactoryTest
#194
Conversation
Apparently MariaDB does not contain the ``` (using password: YES) ``` substring in the authentication error message. By making that substring optional in the corresponding regex check, the tests are green for me (11.1.2-MariaDB @ Macbook Pro M1)
There was a problem hiding this comment.
@bwaidelich Thanks for opening this pull request 👍
This seems like a fairly simple change and brings up the bigger question, how are we making sure this works as expected across different relational database management systems? For now I can only take your word for it, as we don't have proof that this test failed with MariaDB, so I think we need to improve our test suite to cover these cases.
I'm not sure if this is something to discuss for this pull request, as answering this question takes up much more time and I don't want to leave this laying around for too long.
The test suite is currently green and was green before, so we could argue that this doesn't break anything we're currently testing against and merge it as is. I personally can't really "approve" this PR as it's hard to proof what's currently getting fixed, so maybe postpone until we have tests in place?
Interested in your and @WyriHaximus's and @clue's opinion on this.
I was wondering the same. |
This change set introduces a `rdbms` matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions. Note that due to MySQL's versioning there are no `v6` and `v7`. Refs: friends-of-reactphp#194
This change set introduces a `rdbms` matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions. Note that due to MySQL's versioning there are no `v6` and `v7`. Refs: friends-of-reactphp#194
This change set introduces a `rdbms` matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions. Note that due to MySQL's versioning there are no `v6` and `v7`. Refs: friends-of-reactphp#194
TL;DR: here is my opinion on this: #196
We should increase our compatibility matrix. I've just put up #196 as to how we will achieve that. It's rough on the edges but it is the way forward IMHO. The question is should |
This change set introduces a `rdbms` matrix with the purpose of increasing insurance this package works with up to the latest MySQL/MariaDB versions. Note that due to MySQL's versioning there are no `v6` and `v7`. Refs: friends-of-reactphp#194
|
Closing to remove this one from my list of pending PRs |
Apparently MariaDB does not contain the
substring in the authentication error message.
By making that substring optional in the corresponding regex check, the tests are green for me (11.1.2-MariaDB @ Macbook Pro M1)