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

TestCase::setLocale() resets LC_ALL locale without respecting format #5200

Closed
SiebelsTim opened this issue Feb 14, 2023 · 4 comments
Closed
Labels
type/bug Something is broken version/9 Something affects PHPUnit 9 version/10 Something affects PHPUnit 10

Comments

@SiebelsTim
Copy link

SiebelsTim commented Feb 14, 2023

Q A
PHPUnit version main, 9.6.3
PHP version 8.1
Installation Method Composer

Summary

When using TestCase::setLocale in combination with \LC_ALL, PHPUnit does not reset the locale properly.
In the worst case, it throws a warning which stops the test:

setlocale(): Specified locale name is too long

Current behavior

Here, PHPUnit saves the currently used locale with setlocale($category, 0).

$this->locale[$category] = setlocale($category, 0);

Then, it tries to reset the locale based on what was saved.

setlocale($category, $locale);

However, when we use \LC_ALL and it contains something other than C, setlocale(\LC_ALL, 0) returns a list of key-value pairs separated by semicolons.

https://3v4l.org/BNNQu

Therefore, PHPUnit tries to reset the locale for \LC_ALL with an invalid value.

How to reproduce

class MyTest extends TestCase {
  function testStuff(): void {
    setlocale(\LC_NUMERIC, 'en_US.utf8'); // To get a list of key-value pairs

    $this->setLocale(\LC_ALL, 'en_US.utf8');
  }
}

Expected behavior

No error.
Locale reset to the previous value.

@SiebelsTim SiebelsTim added the type/bug Something is broken label Feb 14, 2023
@sebastianbergmann sebastianbergmann added version/9 Something affects PHPUnit 9 version/10 Something affects PHPUnit 10 labels Feb 14, 2023
@sebastianbergmann
Copy link
Owner

When you master you mean main, I assume. So the problem exists in both PHPUnit 9.6 and PHPUnit 10.0. Correct?

@SiebelsTim
Copy link
Author

When you master you mean main

Yup, sorry!

So the problem exists in both PHPUnit 9.6 and PHPUnit 10.0. Correct?

Yes. I tested PHPUnit 9.6. Then I looked into the source code to confirm that it is not fixed. However, I did not run PHPUnit 10.

@sebastianbergmann
Copy link
Owner

Thank you for reporting this issue. However, I am afraid that I won't be able to dedicate time to debugging/fixing it. I will, of course, review and consider a pull request that addresses this.

Here is why I am reluctant to look into this: I am considering to deprecate functionality such as TestCase::setLocale() in a future release, and then remove it in the next major version after that. I consider this functionality to be "niche" as I have not seen used it being used so far.

@sebastianbergmann sebastianbergmann changed the title TestCase::setLocale resets LC_ALL locale without respecting format TestCase::setLocale() resets LC_ALL locale without respecting format Mar 6, 2023
@sebastianbergmann
Copy link
Owner

The TestCase::setLocale() method is deprecated and scheduled for removal, see #5216 for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is broken version/9 Something affects PHPUnit 9 version/10 Something affects PHPUnit 10
Projects
None yet
Development

No branches or pull requests

2 participants