Bug #67092
closedEpic #65314: PHP7
PHP7 Warning DateViewHelperTest::setLocale() should be compatible with PHPUnit_Framework_TestCase::setLocale()
100%
Description
PHP Warning: Declaration of TYPO3\CMS\Fluid\Tests\Unit\ViewHelpers\Format\DateViewHelperTest::setLocale() should be compatible with PHPUnit_Framework_TestCase::setLocale() in /home/travis/build/TYPO3/TYPO3.CMS/typo3/sysext/fluid/Tests/Unit/ViewHelpers/Format/DateViewHelperTest.php on line 264
This happens because PHP7 reads the type hint in PHPUnit_Framework_TestCase:
/** * This method is a wrapper for the setlocale() function that automatically * resets the locale to its original value after the test is run. * * @param integer $category * @param string $locale * @throws PHPUnit_Framework_Exception * @since Method available since Release 3.1.0 */ protected function setLocale() {
while DateViewHelperTest has its own definition for setLocale which is not compatible
/** * @param string $locale */ protected function setLocale($locale) { setlocale(LC_CTYPE, $locale); setlocale(LC_MONETARY, $locale); setlocale(LC_TIME, $locale); }
first param int vs string.
How to deal with that? Rename the function in DateViewHelperTest ? Edit phpunit on github to remove the type hinting?
Updated by Stephan Großberndt over 9 years ago
- Subject changed from PHP Warning DateViewHelperTest::setLocale() should be compatible with PHPUnit_Framework_TestCase::setLocale() to PHP7 Warning DateViewHelperTest::setLocale() should be compatible with PHPUnit_Framework_TestCase::setLocale()
Updated by Alexander Opitz over 9 years ago
Hmmm looking into neos/fluid doesn't help us here.
I would use a new function name like "setOwnLocale()" as (beside PHPUnits setLocale) we don't restore the locale afterwards and using PHPUnits setLocale will throw an exception instead of skipping if locale not found on system.
But I've 2 other small nitpicks while reading:
1.) False locale de_DE.UTF-8 inside Provider
- IMHO this should be de_DE.utf8 (without the "-") as "locale a" on command line tells. Tested both variants yet, it seams UTF-8 and utf8 are handled same on my system.
2.) Do not work on Windows?
- I don't have windows, but as I read last days much about setlocale() for de_DE.utf8 this needs to be "German_Germany.65001" (or "deu_deu.65001").
Updated by Stephan Großberndt over 9 years ago
On debian wheezy I have
C C.UTF-8 de_DE de_DE@euro de_DE.iso88591 de_DE.iso885915@euro de_DE.utf8 deutsch dutch français french fr_FR fr_FR@euro fr_FR.iso88591 fr_FR.iso885915@euro fr_FR.utf8 german nl_NL nl_NL@euro nl_NL.iso88591 nl_NL.iso885915@euro nl_NL.utf8 POSIX
there are all kinds of variations. I would rather add another locale de_DE.utf8 . Not that familiar with tests, does skipping the test mean it won't be executed for second and third set from dataprovider if the first one fails?
Updated by Alexander Opitz over 9 years ago
No, only skip the test once, the other tests generated through dataProvider will be run.
My Ubuntu 15.04 returns:
C C.UTF-8 de_AT.utf8 de_BE.utf8 de_CH.utf8 de_DE.utf8 de_LI.utf8 de_LU.utf8 en_AG en_AG.utf8 en_AU.utf8 en_BW.utf8 en_CA.utf8 en_DK.utf8 en_GB.utf8 en_HK.utf8 en_IE.utf8 en_IN en_IN.utf8 en_NG en_NG.utf8 en_NZ.utf8 en_PH.utf8 en_SG.utf8 en_US.utf8 en_ZA.utf8 en_ZM en_ZM.utf8 en_ZW.utf8 POSIX
But we don't need to add this as new runs in dataProvider, we can also add this as locale choose like array('de_DE.utf8', 'de_DE@euro', ... )
Updated by Susanne Moog over 9 years ago
- Status changed from New to Resolved
- Target version set to 7 LTS
The PHP7 error is resolved in current master (only the declaration thing).
Whether or not this should be done differently has nothing to do with 7, so if you still consider that a problem, please open a new ticket :)
Thanks.
Updated by Alexander Opitz over 9 years ago
- Status changed from Resolved to New
This ticket is about the warning not about a PHP7 error.
Updated by Gerrit Code Review over 9 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42110
Updated by Benni Mack over 9 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset c0f322ad1d4588aed5640006fe9a237c5c795b4a.
Updated by Gerrit Code Review over 9 years ago
- Status changed from Resolved to Under Review
Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/42113
Updated by Andreas Fernandez over 9 years ago
- Status changed from Under Review to Resolved
Applied in changeset ce72378b5fd55743ecd1f00167887e5a12d72f08.
Updated by Riccardo De Contardi about 7 years ago
- Status changed from Resolved to Closed