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

Use UTF-8 for SQLServer tests #5236

Closed
wants to merge 2 commits into from

Conversation

duncan3dc
Copy link
Contributor

Q A
Type improvement
BC Break no

Summary

While working on the fix for a bug around how UTF-8 data is handled I discovered that the default test connections to sqlserver are not using UTF-8. I couldn't find any tests that just configured this for them, so figured it should perhaps just be the default. If there's another preferred approach for this please let me know

@derrabus
Copy link
Member

derrabus commented Feb 2, 2022

Can you have a look at the AppVeyor failure?

That being said, I wonder if it's worth fixing the 2.13 testsuite for that matter. Bugfixes should target the 3.3.x branch currently.

@derrabus
Copy link
Member

derrabus commented Feb 2, 2022

Oh, I see why AppVeyor is broken. This is unrelated to your PR, nevermind.

@derrabus
Copy link
Member

derrabus commented Feb 2, 2022

Can you please rebase on the latest 2.13.x branch? This should fix the AppVeyor build.

@morozov
Copy link
Member

morozov commented Feb 3, 2022

While working on the fix for a bug around how UTF-8 data is handled I discovered that the default test connections to sqlserver are not using UTF-8.

Why is this a problem? There is a test that runs SQL against different Unicode strings and passes:

public static function expressionProvider(): iterable
{
yield '1-byte' => ['Hello, world!', 13, false];
yield '2-byte' => ['Привет, мир!', 12, true];
yield '3-byte' => ['你好,世界', 5, true];
yield '4-byte' => ['💩', 1, true];
}

@duncan3dc
Copy link
Contributor Author

Why is this a problem?

Without this, the test in #5237 fails:

1) Doctrine\Tests\DBAL\Functional\Platform\TextTypeTest::testAddColumnWithDefault
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
-    0 => 'Zażółć'
+    0 => 'Za?ó??'
 )

@duncan3dc duncan3dc force-pushed the sqlserver-utf-8-tests branch from b5dabf6 to b0564bc Compare February 3, 2022 09:28
@morozov
Copy link
Member

morozov commented Feb 3, 2022

In this case, it should be part of the change that requires it. Note the 2.13.x is approaching the end of life, so this patch should be targeted at 3.4.x.

Note that the support for charset configuration in the test suite is already implemented in #4855.

@duncan3dc duncan3dc closed this Feb 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants