Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.16k stars 75 forks source link

Figure out how to run the test suite in multiple locales #134

Closed Zegnat closed 1 year ago

Zegnat commented 4 years ago

See #131 and #133 for a problem with strtolower that never before showed up on tests.

While working on #133 I also discovered that testUriComponentsGetEncodedProperly is locale dependent and started failing with en_US.UTF-8:

Tests\Nyholm\Psr7\UriTest::testUriComponentsGetEncodedProperly with data set #1 ('/€?€#€', '/%E2%82%AC', '%E2%82%AC', '%E2%82%AC', '/%E2%82%AC?%E2%82%AC#%E2%82%AC')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'/%E2%82%AC'
+'/%E2_%AC'

/psr7/tests/UriTest.php:406

Though I am unsure why this goes wrong. rawurlencode seems pretty clear in what it should output. More research necessary to fix that one.

This seems to highlight that PSR7 was developed to run only for 1 locale, and deviations may break. How do we best detect this with our tests?

Nyholm commented 4 years ago

Is this still a problem after #143 was merged?

Zegnat commented 4 years ago

Just cloned latest master and I still do not pass tests locally. Actually failing two of them now:

$ locale
LANG=""
LC_COLLATE="C"
LC_CTYPE="UTF-8"
LC_MESSAGES="C"
LC_MONETARY="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_ALL=
$ composer test
> vendor/bin/phpunit
PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

...............................................................  63 / 388 ( 16%)
............................................................... 126 / 388 ( 32%)
............................................................... 189 / 388 ( 48%)
............................................................... 252 / 388 ( 64%)
............................................................... 315 / 388 ( 81%)
........F.............F........................................ 378 / 388 ( 97%)
..........                                                      388 / 388 (100%)

Time: 3.55 seconds, Memory: 8.00 MB

There were 2 failures:

1) Tests\Nyholm\Psr7\UriTest::testUriComponentsGetEncodedProperly with data set #1 ('/€?€#€', '/%E2%82%AC', '%E2%82%AC', '%E2%82%AC', '/%E2%82%AC?%E2%82%AC#%E2%82%AC')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'/%E2%82%AC'
+'/%E2_%AC'

/Users/martijn/Source/psr7/tests/UriTest.php:389

2) Tests\Nyholm\Psr7\UriTest::testUtf8Host
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'ουτοπία.δπθ.gr'
+'ο�_�_ο�_ία.δ�_θ.gr'

/Users/martijn/Source/psr7/tests/UriTest.php:478

FAILURES!
Tests: 388, Assertions: 881, Failures: 2.
Script vendor/bin/phpunit handling the test event returned with error code 1

However, after setting my LC_CTYPE to C (note I use fish as my shell):

$ set -x LC_CTYPE "C"

The tests all pass.

Zegnat commented 4 years ago

Would love to know what locale settings other people are running the tests on, to know if it is just something about me, or if PHP is way more locale sensitive than I previously realised.

Nyholm commented 4 years ago

Hm.

I know that I had this issue before. I didn’t do anything specifically to fix it. But since the I’ve updated PHP..

Zegnat commented 4 years ago

For completion, I should have probably included this in the previous run output:

php --version
PHP 7.4.4 (cli) (built: Mar 19 2020 20:12:27) ( NTS )

Installed through brew on macOS.

@Nyholm could you check your output of locale?

Nyholm commented 4 years ago

It test fails on the office computer. I'll see if there is any differences at my home machine.

PHPUnit: 2 failures. 

❯ php --version
PHP 7.4.4 (cli) (built: Mar 19 2020 20:12:27) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.4, Copyright (c), by Zend Technologies
    with blackfire v1.29.5~mac-x64-non_zts74, https://blackfire.io, by Blackfire

❯ locale
LANG=""
LC_COLLATE="C"
LC_CTYPE="UTF-8"
LC_MESSAGES="C"
LC_MONETARY="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_ALL=
Nyholm commented 4 years ago

Hm... At home it is the same thing.

PHPUnit: 2 failures

❯ php -v
PHP 7.4.4 (cli) (built: Mar 19 2020 20:12:27) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.4, Copyright (c), by Zend Technologies
    with blackfire v1.29.5~mac-x64-non_zts74, https://blackfire.io, by Blackfire

❯ locale
LANG=""
LC_COLLATE="C"
LC_CTYPE="UTF-8"
LC_MESSAGES="C"
LC_MONETARY="C"
LC_NUMERIC="C"
LC_TIME="C
LC_ALL=

However, when I use PHPStorm to run the test, they all pass...

PHPStrom runs the tests with --teamcity argument.

Zegnat commented 4 years ago

PHPStorm could be running in an entirely different environent. Maybe it isn’t spinning up a login shell, maybe there is no shell at all, or maybe it is configuring its own locale settings. I wouldn’t even be able to tell you with much certainty where the LC_CTYPE is coming from on my machine. Could be a macOS default for the environment, or maybe it is something that iTerm sets.

Nevertheless I feel like the tests should be independable and portable enough to run no matter the locale that is currently configured on the host machine. Because if they can’t, the code may also not be as portable as we expect.

nicolas-grekas commented 1 year ago

Closing as nobody else reported the issue and this stalled.