php / php-src

The PHP Interpreter
https://www.php.net
Other
38.01k stars 7.73k forks source link

`IntlDateFormatter` output changes based on operating system used #16127

Closed GuySartorelli closed 2 hours ago

GuySartorelli commented 3 hours ago

Description

The following code:

<?php

$locale = 'en_NZ';

var_dump(
    IntlDateFormatter::create(
        $locale,
        IntlDateFormatter::MEDIUM,
        IntlDateFormatter::NONE
    )->format(1727655409)
);

Resulted in this output in Ubuntu 22.04 and Debian 12:

string(10) "30/09/2024"

And resulted in this output in Ubuntu 24.04:

string(12) "30 Sept 2024"

I'm not sure which of those is correct, but I expected the two results to be the same.

PHP Version

PHP 8.3

Operating System

See description

GuySartorelli commented 3 hours ago

Interestingly, if $locale is set to 'en_US' the output is the same regardless of which OS I use:

string(12) "Sep 30, 2024"

This leads me to believe that the correct output is the one from Ubuntu 24.04 - but regardless of which is correct, they should still be identical results from PHP without concern for which OS is being used.

damianwadley commented 2 hours ago

they should still be identical results from PHP without concern for which OS is being used.

Not quite.

IntlDateFormatter is powered by the ICU project, and PHP is going to use the version of that library that's installed on your system. Their data includes things like date patterns, currency formatting, and timezone data. Apparently the version installed with Ubuntu 24.04 made some changes to en_NZ's date formatting since the version installed in Ubuntu 22.04.

You can update the older system's ICU data/library, or you can not worry about the exact output and just trust that it's going to be formatted in the way that seems (seemed) best at the time. I suggest the latter because that's the whole point of using libraries like ICU.

GuySartorelli commented 2 hours ago

You can update the older system's ICU data/library, or you can not worry about the exact output and just trust that it's going to be formatted in the way that seems (seemed) best at the time

Neither of those is particularly helpful when we've got failing unit tests which expect a specific value, which is drastically different based on which OS (or I guess which ICU) version is used.

Given the values are so vastly different I would have expected PHP to normalise it... but I can accept your position that since PHP doesn't do the formatting, it's not responsible for the result of the formatting.

damianwadley commented 2 hours ago

Neither of those is particularly helpful when we've got failing unit tests which expect a specific value, which is drastically different based on which OS (or I guess which ICU) version is used.

Don't do that.

If you want to use a library (be that PHP or ICU, doesn't matter) to handle things like formatting dates then you should be offloading the date formatting to that library. If the library changes the date format over time then that's not your responsibility, and your tests shouldn't break because of that.

In other words, as the saying goes, "don't test the framework".

The real truth is that the library makes no guarantees about the exact output. There's nothing that says "when IntlDateFormatter is given the en_NZ locale and asked for 'medium'-length output, the date should be formatted as DD/MM/YYYY". Creating tests to ensure that it does exactly that is expecting the functionality to adhere to a contract that does not exist.

If your application actually needs DD/MM/YYYY format then use PHP's own date functionality with its manual format-ing instead.

GuySartorelli commented 2 hours ago

Don't do that.

The tests already exist - and are testing an abstraction around IntlDateFormatter. To test that the abstraction is working we need to test the result. Or I guess rewrite all the tests to use mocks and just check the right args are passed to the right methods on IntlDateFormatter but that's way too much work right now.

damianwadley commented 1 hour ago

Well, if you want the least amount of work, could you change the locale used to one that hasn't had its date formatting patterns change recently?...

Or, simply update the test so that it allows both outputs.

Personally, I'm seeing parallels here to browser version detection with JS/CSS - and how that's fallen out of favor in the last decade or so and been replaced with feature detection. For this, the equivalent would be like having the test run IntlDateFormatter manually, identifying the format it's using, and having the test expect the abstraction's output accordingly.