Daylight saving and PHP8.1

[nyamsprod]

In PHP8.1 some improvements around daylight saving did land and they are messing with how the next run is being calculated. The DaylightSavingsTest test suite is having many issues.

Excerpt from this blog post since I fail to find the correct commit/entry in the php changelog

Improved management of DateTime and daylight saving time transitions Just a little section to tell you about some improvements about transitions to Daylight saving time. When you’re trying to create February 29th on a non-leap year, PHP will “round” your date so it’s a valid and existing one. This RFC propose to apply this behaviour to hours that don’t exist because of Daylight saving time transitions: it will “round” the unexisting time to a valid one. This will result in a way much simpler management of dates and times in PHP. Here is a concrete example: in France, the next time change to daylight saving time will happen on March 27th 2022. We’ll go from 2:00 am to 3:00 am directly, which means 2:30 am won’t exist. If you try to create a Datetime for March 27th, 2:30 am, PHP will automatically “round” it to March 27th, 3:30 am. Note that this RFC has been created… 10 years ago, back in 2011!

see https://medium.com/geekculture/php-8-1-is-coming-2nd-round-of-upcoming-features-e1c5ddd14b38

I believe this makes the following tests fail.

2) Cron\Tests\DaylightSavingsTest::testOffsetIncrementsNextRunDate
Failed asserting that two DateTime objects are equal.
--- Expected
+++ Actual
@@ @@
-2021-03-28T02:00:00.000000+0100
+2021-04-04T01:00:00.000000+0100

There are other similar error since I do not want to change the tests maybe something needs to happen here in the code to selectively prevent any changes in PHP8.1 🤔 .

Also some other tests just end up in an infinite loop

Cron\Tests\DaylightSavingsTest::testOffsetIncrementsPreviousRunDate
2) Cron\Tests\DaylightSavingsTest::testOffsetIncrementsPreviousRunDate
RuntimeException: Impossible CRON expression

/Users/i.nyamagana.butera/dev/alt-cron-expression/src/Cron/CronExpression.php:461
/Users/i.nyamagana.butera/dev/alt-cron-expression/src/Cron/CronExpression.php:226
/Users/i.nyamagana.butera/dev/alt-cron-expression/tests/Cron/DaylightSavingsTest.php:95

[nyamsprod]

OK I read the PHP 8.1 changelog and indeed seems the whole DateInterval and DateTimeZone calculation where re-implemented and moreso fixed. Hence the reason why most of the Timezone issue this package has do not show up in my fork which is PHP8.1+ only .

For complete reference look at https://www.php.net/ChangeLog-8.php#PHP_8_1 the Date part. 🤔

[dragonmantank]

Currently as of v3.2.4 all of the existing tests seem to work with the DST patch that was applied (and subsequently refined) in v3.2.1, and currently work from PHP 7.2 to 8.1.

Can you take a look at the newest release and see if you are still having issues on PHP 8.1?

[rgson]

I'm seeing test failures in DaylightSavingsTest with v3.3.1 and PHP 8.1. Tests pass on PHP 7.4.

> php --version
PHP 8.1.7 (cli) (built: Jun 25 2022 07:57:04) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.7, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.7, Copyright (c), by Zend Technologies

> git status +short
HEAD detached at v3.3.1
nothing to commit, working tree clean

> vendor/bin/phpunit
PHPUnit 9.5.21 #StandWithUkraine

...............................................................  63 / 167 ( 37%)
............................................................... 126 / 167 ( 75%)
.......FF..F.F...........................                       167 / 167 (100%)

Time: 00:00.029, Memory: 8.00 MB

There were 4 failures:

1) Cron\Tests\DaylightSavingsTest::testOffsetDecrementsNextRunDateAllowCurrent
Failed asserting that two DateTime objects are equal.
--- Expected
+++ Actual
@@ @@
-2020-10-25T01:00:00.000000+0000
+2020-10-25T01:00:00.000000+0100

[...]/tests/Cron/DaylightSavingsTest.php:125

2) Cron\Tests\DaylightSavingsTest::testOffsetDecrementsNextRunDateDisallowCurrent
Failed asserting that two DateTime objects are equal.
--- Expected
+++ Actual
@@ @@
-2020-10-25T01:00:00.000000+0000
+2020-10-25T01:00:00.000000+0100

[...]/tests/Cron/DaylightSavingsTest.php:159

3) Cron\Tests\DaylightSavingsTest::testOffsetDecrementsMultipleRunDates
Failed asserting that an array contains DateTimeImmutable Object &00000000000000fd0000000000000000 (
    'date' => '2020-11-08 01:00:00.000000'
    'timezone_type' => 3
    'timezone' => 'Europe/London'
).

[...]/tests/Cron/DaylightSavingsTest.php:248

4) Cron\Tests\DaylightSavingsTest::testOffsetDecrementsEveryOtherHour
Failed asserting that an array contains DateTimeImmutable Object &00000000000000fe0000000000000000 (
    'date' => '2020-10-25 07:00:00.000000'
    'timezone_type' => 3
    'timezone' => 'Europe/London'
).

[...]/tests/Cron/DaylightSavingsTest.php:367

FAILURES!
Tests: 167, Assertions: 775, Failures: 4.

[athos-ribeiro]

Here is a reproducer:

#!/bin/bash

docker_build() {
  docker build - <<EOF
FROM composer:latest as composer
FROM php:${1}
COPY --from=composer /usr/bin/composer /usr/bin/composer
RUN apt update
RUN apt install -y git unzip
RUN git clone https://github.com/dragonmantank/cron-expression
workdir /cron-expression
RUN composer --no-plugins install
RUN composer --no-plugins test
EOF

}

for TAG in 7.4 8.0.22 8.1.0 8.1.5 8.1.6 8.1.7 8.1.8 8.1.9 8.2-rc-buster 8.1; do
  if docker_build ${TAG} > /dev/null 2>&1; then
    echo "${TAG}: SUCCESS"
  else
    echo "${TAG}: FAILURE"
  fi
done

Which outputs:

7.4: SUCCESS
8.0.22: SUCCESS
8.1.0: FAILURE
8.1.5: FAILURE
8.1.6: SUCCESS
8.1.7: FAILURE
8.1.8: FAILURE
8.1.9: FAILURE
8.2-rc-buster: FAILURE
8.1: FAILURE

By going through https://www.php.net/ChangeLog-8.php, I see several changes to the date implementation throughout the patch releases.

This seems to be the commit that fixed 8.1.6: https://github.com/php/php-src/commit/e6c498818717217c0af7699d9b9ce5fc8b903011

There is a related php upstream issue at https://github.com/php/php-src/issues/9165

[dragonmantank]

I believe I have a fix for this, it is due to how time math was being handled. Before I release a fix, could I get your thoughts on https://github.com/dragonmantank/cron-expression/discussions/181 as it will impact the overall solution?