mysqli_set_charset now throws an mysqli_sql_exception when incorrect charset is provided #6142
Conversation
…charset is provided
If there is an existing test for set_charset functionality, it's better to change it. |
|
I wasn't sure if I should add a test case for the error from the database. e.g. "Variable 'character_set_client' can't be set to the value of 'ucs2'". |
Probably wouldn't hurt? Or is that error condition not reliable? |
It's unclear if the support for ucs2 will be added in the future but at the moment it looks like this is impossible. An exception should be thrown if you try to set charset to ucs2
| $link->set_charset('invalid'); | ||
| } catch (\mysqli_sql_exception $exception) { | ||
| echo $exception->getMessage() . "\n"; | ||
| } |
There was a problem hiding this comment.
I fixed it but it is difficult to keep the indentation consistent when half of the file is using tabs and the other half is using 4 spaces. My IDE ignores this differences and the file doesn't follow any formatting rules. If I execute CS-fixer it actually fixes formatting but I believe I should leave the formatting as it is. If you want me to execute CS-fixer when updating test cases I can do that, but then I would probably require the CS config file.
There was a problem hiding this comment.
Are you referring to the tabs in the SKIPIF section? I've opened #6161 to fix that.
Previously mysqli didn't throw an exception when report mode was set to exception and a wrong charset was provided.
Additionally if an unsupported charset was provided and the pre-check didn't catch it then there would be a confusing warning "Error executing query". Instead let mysqli throw an exception if it is set to exception mode.
This results in 2 new behaviours.
I was unable to find a suitable test case for this. I am new to writing PHP test cases and I don't know if I should create a new one or change an existing one. I would appreciate some guidance.