Project

General

Profile

Actions

Bug #67092

closed

Epic #65314: PHP7

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

Added by Stephan Großberndt over 9 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
Start date:
2015-05-21
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
7
PHP Version:
7.0
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

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?

Actions #1

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()
Actions #2

Updated by Markus Klein over 9 years ago

  • Parent task set to #65314
Actions #3

Updated by Alexander Opitz over 9 years 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").

Actions #4

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?

Actions #5

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', ... )

Actions #6

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.

Actions #7

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.

Actions #8

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

Actions #9

Updated by Benni Mack over 9 years ago

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

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

Actions #11

Updated by Andreas Fernandez over 9 years ago

  • Status changed from Under Review to Resolved
Actions #12

Updated by Riccardo De Contardi about 7 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF