e107inc / e107-test

For developmental use with e107 v2.
2 stars 2 forks source link

Updated testStrptime() to match the result of strptime on linux. #5

Closed SimSync closed 6 years ago

SimSync commented 6 years ago

Should also run fine on windows with the updated e_date::strptime() function.

SimSync commented 6 years ago

@Deltik Do the tests pass now?

Deltik commented 6 years ago

@SimSync: Not all of them, but that's because the test failing is unrelated to what you were fixing:

There was 1 failure:

---------
1) e_dateTest: Convert_date
 Test  tests/unit/e_dateTest.php:testConvert_date
Failed asserting that two strings are equal.
- Expected | + Actual
@@ @@
-'12:45 PM'
+'12:45 pm'
#1  /home/deltikdev/repos/e107-test/tests/unit/e_dateTest.php:198
SimSync commented 6 years ago

@Deltik Looks like you have a different mask defined in the prefs. This the mask that is return for "inputtime" on my machine: "%I:%M %p" date_handler.php line 175

Deltik commented 6 years ago

@SimSync: That is the same mask configured for all unit tests. This is the database dump used.

SimSync commented 6 years ago

@Deltik Yes, and the mask is the same. The lowercase %p returns an uppercase AM or PM. And that's how it should be according to the manual: https://secure.php.net/manual/en/function.strftime.php Don't know why it is a lowercase am / pm on your machine ... Any chance you can verify that the mask is really what it should be on your machine?

Deltik commented 6 years ago

@SimSync: Frustratingly, 3v4l.org shows that the output is 01:45 PM instead of the expected 12:45 PM.

I also get the expected output when I run the function directly in PHP:

$ php -r 'echo strftime("%I:%M %p", "1517834703");'
12:45 PM

I did verify that the mask is correct in the test.

Deltik commented 6 years ago

A difference in locale as set by setlocale() does explain some of the behavior. The test runs with the locale en_GB.UTF-8 set while the command line outputs use C as the locale.

How to reproduce:

$ php -r 'echo setlocale(LC_TIME, "en_GB.UTF-8") . PHP_EOL . strftime("%I:%M %p", "1517834703") . PHP_EOL;'
en_GB.UTF-8
12:45 pm
$ php -r 'echo setlocale(LC_TIME, 0) . PHP_EOL . strftime("%I:%M %p", "1517834703") . PHP_EOL;'
C
12:45 PM

This does not explain the 3v4l.org output of 01:45 PM, though.

SimSync commented 6 years ago

@Deltik To be hosest... This looks like a PHP bug, or not? The manual says "%p" will become "AM" or "PM" and not will become an uppercase AM/PA or lowercase am/pm depending on your locale settings. Or is an uppercase AM/PM required in england and a lowercase am/pm would be a gramatical error?

See: https://secure.php.net/manual/en/function.strftime.php

Return values

Returns a string formatted according format using the given timestamp or the current local time if no timestamp is given. Month and weekday names and other_ language-dependent strings respect the current locale set with setlocale().

Nevertheless, i'm unsure how to handle this in the tests as it is some kind of inconstant...?!?

Deltik commented 6 years ago

@SimSync: Well, I pushed a stopgap in c0cc29d830e326c3fe1c15e816546d794f2c2744, and now, all the tests pass.

Deltik commented 6 years ago

@SimSync: I did some research on the en_GB.UTF-8 locale and found that the uppercase format for 12-hour times is actually "am" and "pm".

Relevant pages:

If I had the ability, I would change the wording of the PHP documentation from:

UPPER-CASE 'AM' or 'PM' based on the given time

to:

Either "AM" or "PM" according to the given time value, or the corresponding strings for the current locale. Noon is treated as "PM" and midnight as "AM".

Deltik commented 6 years ago

PHP documentation bug reported: https://bugs.php.net/bug.php?id=76378