Bug #67092

Epic #65314: PHP7

PHP7 Warning DateViewHelperTest::setLocale() should be compatible with PHPUnit_Framework_TestCase::setLocale()

Added by Stephan Großberndt 2 months ago. Updated 5 days ago.

Status:Resolved Start date:2015-05-21
Priority:Should have Due date:
Assigned To:- % Done:

100%
Category:- Spent time: -
Target version:7 LTS
TYPO3 Version:7 Is Regression:No
PHP Version:7.0 Sprint Focus:
Complexity:

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?

Associated revisions

Revision c0f322ad
Added by Benjamin Mack 5 days ago

[BUGFIX] Rename setLocale() in DateTimeViewHelperTest

The method setLocale($locale) in the DateTimeViewHelperTest
was implemented to test against several locales. However,
as there is a setLocale() in the parent class from PHPUnit that
does not take any parameter, PHP7 throws a warning as the
subclass does not have the same method signature as the parent
class.

The patch changes the method to be named differently thus,
solving the method signature conflict inside the test.

Resolves: #67092
Releases: 6.2, master
Change-Id: Ia05d90c64a3a8e2fc599935d2435192392e5a667
Reviewed-on: http://review.typo3.org/42110
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Andreas Fernandez <>
Tested-by: Andreas Fernandez <>

Revision ce72378b
Added by Andreas Fernandez 5 days ago

[BUGFIX] Rename setLocale() in DateTimeViewHelperTest

The method setLocale($locale) in the DateTimeViewHelperTest
was implemented to test against several locales. However,
as there is a setLocale() in the parent class from PHPUnit that
does not take any parameter, PHP7 throws a warning as the
subclass does not have the same method signature as the parent
class.

The patch changes the method to be named differently thus,
solving the method signature conflict inside the test.

Resolves: #67092
Releases: 6.2, master
Change-Id: Ia05d90c64a3a8e2fc599935d2435192392e5a667
Reviewed-on: http://review.typo3.org/42113
Reviewed-by: Andreas Fernandez <>
Tested-by: Andreas Fernandez <>

History

#1 Updated by Stephan Großberndt 2 months 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()

#2 Updated by Markus Klein 2 months ago

  • Parent task set to #65314

#3 Updated by Alexander Opitz 2 months ago

Hmmm looking into neos/fluid doesn't help us here.

https://git.typo3.org/Packages/TYPO3.Fluid.git/blob/HEAD:/Tests/Unit/ViewHelpers/Format/DateViewHelperTest.php

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").

#4 Updated by Stephan Großberndt 2 months 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?

#5 Updated by Alexander Opitz 2 months 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', ... )

#6 Updated by Susanne Moog 18 days 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.

#7 Updated by Alexander Opitz 18 days ago

  • Status changed from Resolved to New

This ticket is about the warning not about a PHP7 error.

#8 Updated by Gerrit Code Review 5 days 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

#9 Updated by Benjamin Mack 5 days ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#10 Updated by Gerrit Code Review 5 days 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

#11 Updated by Andreas Fernandez 5 days ago

  • Status changed from Under Review to Resolved

Also available in: Atom PDF