Skip to content
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

Creating new static DateTime objects when frozen doesn't work with IANA timezones #37

Closed
larbear opened this issue Apr 17, 2023 · 1 comment · Fixed by #39
Closed

Creating new static DateTime objects when frozen doesn't work with IANA timezones #37

larbear opened this issue Apr 17, 2023 · 1 comment · Fixed by #39

Comments

@larbear
Copy link

larbear commented Apr 17, 2023

The following code will produce an error due to the use of an IANA timezone:

<?php
require "vendor/autoload.php";

SlopeIt\ClockMock\ClockMock::freeze(new DateTime("2023-01-01 05:00:00"));

$dt = "2022-12-25 01:00:00";
$dt = new DateTime($dt, new DateTimeZone("US/Eastern"));
echo $dt->format('Y-m-d H:i:s e') . "\n";

The culprit seems to be the constructor, with the following code:

if ($timezone !== null && !$isDateTimeStringRelative) {
    $this->setTimestamp(
        strtotime(
            "$datetime {$timezone->getName()}",
            ClockMock::getFrozenDateTime()->getTimestamp()
        )
    );
}

Specifically, strtotime will not work with IANA timezones such as US/Eastern.

Looking at the code, it seems like you could return right away if the passed in string isn't relative instead of trying to regenerate the time with strtotime:

parent::__construct($datetime, $timezone);

$isDateTimeStringRelative = $this->isRelativeDateString($datetime);

// New code here
if (!$isDateTimeStringRelative) {
    return $this;
}

I didn't make a PR as I don't know the system well enough to ensure this works and I couldn't get the PHPUnit tests to work without any changes. Hoping you can take a look at this and check if it's correct or not.

@asprega
Copy link
Contributor

asprega commented May 19, 2023

Thanks for your report! Just merged a PR that fixes this, fix was released in version 0.3.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants