UserTimeZoneTest fails on PHP 7.0.x-dev and 7.1.x-dev

Is this constantly failing, or intermittent?

I think we need to do two things:

1. Either on admin/reports/status or admin/config/regional/settings, we need to output the results of `timezone_version_get()`. I suspect these dev versions of PHP are using the newer timezonedb that have the Chile change. This should also be logged in a test (UserTimeZoneTest may be best here).

2. Short term we should change UserTimeZoneTest to use a different TZ that observes DST (to fix the critical). Long term, we should standardize the zone we use throughout tests, unless we are explicitly testing something and have picked the zones for particular reasons (eg, the TZ loop in the datatime field tests). Not to be US centric, but I think we should pick America/New_York, which maps to the North American Eastern Time Zone (I can't recall the official letter designation), and that book is at home). It observes DST, is far enough from UTC to show problems where there is a glitch, and I think these is less chance of another local change (which is my fear with choosing something outside the US).

For reference this is the database, https://github.com/eggert/tz, but work/changes are discussed on the tz mailing list (patches are posted to the list, they don't do pull requests).

https://www.drupal.org/pift-ci-job/624542

Just something so we have test runs on this issue.

This is in the notes for 2017a:

  Changes to past and future time zone abbreviations

    Switch to numeric time zone abbreviations for South America, as
    part of the ongoing project of removing invented abbreviations.
    This avoids the need to invent an abbreviation for the new Chilean
    new zone.  Similarly, switch from invented to numeric time zone
    abbreviations for Afghanistan, American Samoa, the Azores,
    Bangladesh, Bhutan, the British Indian Ocean Territory, Brunei,
    Cape Verde, Chatham Is, Christmas I, Cocos (Keeling) Is, Cook Is,
    Dubai, East Timor, Eucla, Fiji, French Polynesia, Greenland,
    Indochina, Iran, Iraq, Kiribati, Lord Howe, Macquarie, Malaysia,
    the Maldives, Marshall Is, Mauritius, Micronesia, Mongolia,
    Myanmar, Nauru, Nepal, New Caledonia, Niue, Norfolk I, Palau,
    Papua New Guinea, the Philippines, Pitcairn, Qatar, Réunion, St
    Pierre & Miquelon, Samoa, Saudi Arabia, Seychelles, Singapore,
    Solomon Is, Tokelau, Tuvalu, Wake, Vanuatu, Wallis & Futuna, and
    Xinjiang; for 20-minute daylight saving time in Ghana before 1943;
    for half-hour daylight saving time in Belize before 1944 and in
    the Dominican Republic before 1975; and for Canary Islands before
    1946, for Guinea-Bissau before 1975, for Iceland before 1969, for
    Indian Summer Time before 1942, for Indonesia before around 1964,
    for Kenya before 1960, for Liberia before 1973, for Madeira before
    1967, for Namibia before 1943, for the Netherlands in 1937-9, for
    Pakistan before 1971, for Western Sahara before 1977, and for
    Zaporozhye in 1880-1924.

I think this is the problem, but I need to figure out a way to arbiltrarilly swap the PHP zoneinfo locally to test this.

Still haven't gotten the 2017a zoneinfo working locally. Hopefully this will confirm my suspicions.

Here is an idea that we can keep the locations, but test the same things. Once I see the results from #11 and #12, I'll post a writeup.

Ok, here is essentially what happened.

In the 2017a zoneinfo file which the 7-dev PHP branches are using, local zone names started to be replaced with numeric designations, because of a change in one of the local time zone rules.

So

2007-03-10 02:00 CLST
2007-03-11 05:00 CLT
2007-03-21 00:00 CLT

became

2007-03-10 02:00 -03
2007-03-11 05:00 -04
2007-03-21 00:00 -04

This is what caused the test fail: the 'T' placeholder is generating a different value for 2017a.

So, if we want to keep these test cases, but effectively test the same thing, we can use the 'I' placeholder, which is 1 when DST is being observed, and 0 if not (see #12). That will keep the spirit of the test; in fact you could argue that this is better because we are explicitly testing DST not implicitly testing it through what the abbreviations mean.

I'll try to find the thread about this change on the TZ mailing list for reference.

If you want to see what zoneinfo is used in which version, you can checkout https://3v4l.org/3sEMa, but that doesn't have the dev tips.

@alexpott, @catch, @cottser, @xjm and I considered this issue and agree this is suitably a critical priority bug test failures in HEAD interrupt the work of project contributors.

I can confirm that the patch in #12 applies, passes locally on MAMP with PHP 7.0 (not dev), and PHP 5.6.

Just one thing:

+++ b/core/modules/user/src/Tests/UserTimeZoneTest.php
@@ -29,7 +29,7 @@ public function testUserTimeZone() {
     DateFormat::load('medium')
-      ->setPattern('Y-m-d H:i T')
+      ->setPattern('Y-m-d H:i I')
       ->save();

There should be a comment explaining why we chose this pattern. Basically explaining #13.

Also out of scope here:

• This should be a BTB test.
• It shouldn't be in the user module. It should probably be in system, since it needs system_test module.

Here is an attempt at a comment.

Diggit. LGTM. Unfortunately I don't have a way to repro locally, but the testbot looks happy.

Committed and pushed fc3d289 to 8.4.x and be0f298 to 8.3.x. Thanks!

Nice fix that preserves the existing coverage - mpdonadio++

This is causing test failures in Drupal 7 too, so here's an issue to backport it: #2880700: [D7] UserTimeZoneFunctionalTest fails on recent versions of PHP 7 and 7.1