-
Notifications
You must be signed in to change notification settings - Fork 441
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
chore: upgrade PHPUnit 9 => 10 #780
Conversation
changes for sebastian/comparator
Tests are failing, because of |
replace setLocale() calls with setlocale()
I replaced it with direct calls to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. Only with regard to setlocale
I would solve that differently
@@ -13,6 +15,6 @@ public function setUp(): void | |||
{ | |||
parent::setUp(); | |||
|
|||
$this->setLocale(LC_ALL, 'ru_RU.UTF-8'); | |||
setlocale(LC_ALL, 'ru_RU.UTF-8'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a file called tests/LocaleTest.php
, with the following content.
declare(strict_types=1);
namespace Tests\Money;
trait LocaleTest
{
public static function setLocale(int $category, string $locale, \Closure $callback)
{
$currentLocale= \setlocale($category, 0);
try {
\setlocale($category, $locale);
$callback();
} finally {
\setlocale($category, $currentLocale);
}
}
}
And then remove setlocale
from setUp
but simply use it in the test file as follows
use Tests\Money\LocaleTest;
public static function the_test()
{
LocaleTest::setLocale(LC_ALL, 'ru_RU.UTF-8', function () {
// test content here
});
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed your suggestion. But i did need to make some changes to make it work.
The name setLocale is already taken with other parameters.
Filename changed from LocaleTest to Locale
I am not sure if i did everythink correct. Please have a look.
changes for sebastian/comparator
#746 rebased against master.
Migrate Config PHPUnit 9 => 10
Only Allow PHPUnit 10.
I did not have a closer look to the comperator changes.