Everything

php

Home » Project » Newsgroups » php.internals » [Discussion] Add date_test_set_now() function »

[Discussion] Add date_test_set_now() function

php.internals

zeriyoshi

2 years ago

Hi, Internals.

I've been absent for a long time due to poor health. I'm finally back.

I maintain a legacy application written in PHP, and occasionally need to
fake the current time for testing purposes. However, PHP doesn't provide a
standard way to fake the current time, so I've been changing the OS's
current time to do this, which is quite painful.

This can be avoided by using third-party libraries (such as
Carbon::setTestNow()). However, it's almost impossible to modify all parts
of a legacy application that depend on the current time.

Another option is to use libfaketime (https://github.com/wolfcw/libfaketime),
but this is also quite painful to use.

Since this was absolutely necessary for my work, I implemented this
functionality as a PHP Extension. (Please ignore the dirty implementation
related to PDO. I'm not planning to propose it this time.)

https://github.com/colopl/php-colopl_timeshifter

However, this Extension has some problems.

The first is that there's no way to determine whether the format passed to
the ext-date parser is relative or absolute time, resulting in a dirty hack
using usleep. The second is that it depends on timelib, so it breaks when
upstream changes related to timelib are made.

So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)`
function to ext-date?

This function would treat the current time as shifted by the passed
DateInterval. Since it's implemented on the ext-date side, there's no need
for dirty hacks using usleep.

I'd like to hear your opinions. Thank you.

Best Regards,
Go Kudo

Larry Garfield

2 years ago

On Mon, Jul 1, 2024, at 1:07 PM, Go Kudo wrote:

> Hi, Internals.
>
> I've been absent for a long time due to poor health. I'm finally back.
>
> I maintain a legacy application written in PHP, and occasionally need
> to fake the current time for testing purposes. However, PHP doesn't
> provide a standard way to fake the current time, so I've been changing
> the OS's current time to do this, which is quite painful.
>
> This can be avoided by using third-party libraries (such as
> Carbon::setTestNow()). However, it's almost impossible to modify all
> parts of a legacy application that depend on the current time.
>
> Another option is to use libfaketime
> (https://github.com/wolfcw/libfaketime), but this is also quite painful
> to use.
>
> Since this was absolutely necessary for my work, I implemented this
> functionality as a PHP Extension. (Please ignore the dirty
> implementation related to PDO. I'm not planning to propose it this
> time.)
>
> https://github.com/colopl/php-colopl_timeshifter
>
> However, this Extension has some problems.
>
> The first is that there's no way to determine whether the format passed
> to the ext-date parser is relative or absolute time, resulting in a
> dirty hack using usleep. The second is that it depends on timelib, so
> it breaks when upstream changes related to timelib are made.
>
> So, how about adding a `date_set_test_now(\DateInterval
> $shiftInterval)` function to ext-date?
>
> This function would treat the current time as shifted by the passed
> DateInterval. Since it's implemented on the ext-date side, there's no
> need for dirty hacks using usleep.
>
> I'd like to hear your opinions. Thank you.
>
> Best Regards,
> Go Kudo

We don't generally add features just to support very-old legacy code.  These days, the correct answer would be to use PSR-20.  (Implementations are trivially easy, many available, and it's dead easy to write your own.)  That doesn't much help legacy code, but adding global features just to support legacy code that should get refactored anyway doesn't seem like a great idea.

--Larry Garfield

zeriyoshi

2 years ago

2024年7月1日(月) 22:07 Go Kudo <zeriyoshi@gmail.com>:

> Hi, Internals.
>
> I've been absent for a long time due to poor health. I'm finally back.
>
> I maintain a legacy application written in PHP, and occasionally need to
> fake the current time for testing purposes. However, PHP doesn't provide a
> standard way to fake the current time, so I've been changing the OS's
> current time to do this, which is quite painful.
>
> This can be avoided by using third-party libraries (such as
> Carbon::setTestNow()). However, it's almost impossible to modify all parts
> of a legacy application that depend on the current time.
>
> Another option is to use libfaketime (
> https://github.com/wolfcw/libfaketime), but this is also quite painful to
> use.
>
> Since this was absolutely necessary for my work, I implemented this
> functionality as a PHP Extension. (Please ignore the dirty implementation
> related to PDO. I'm not planning to propose it this time.)
>
> https://github.com/colopl/php-colopl_timeshifter
>
> However, this Extension has some problems.
>
> The first is that there's no way to determine whether the format passed to
> the ext-date parser is relative or absolute time, resulting in a dirty hack
> using usleep. The second is that it depends on timelib, so it breaks when
> upstream changes related to timelib are made.
>
> So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)`
> function to ext-date?
>
> This function would treat the current time as shifted by the passed
> DateInterval. Since it's implemented on the ext-date side, there's no need
> for dirty hacks using usleep.
>
> I'd like to hear your opinions. Thank you.
>
> Best Regards,
>

Go Kudo

>

I apologize, the main point of my explanation was off.

To put it simply, I'm suggesting that it might be good to implement
convenient and commonly used features from Carbon at the ext-date level.

I'm proposing the date_test_set_now() function for the following reasons:

User-land libraries like Carbon / Chronos have a setTestNow method,
indicating a potential demand for this feature.
It's impossible to determine whether a value is relative or absolute time
from either user-land or Extension.
While this is convenient for maintaining legacy systems, that's not the
essence of this proposal.

As you pointed out, this is an issue that should ideally be solved in
user-land. I deeply understand that.

However, in reality, PHP's time-related processing is diverse, and to
comprehensively handle all of these, it seems necessary to address this at
the ext-date level.

https://www.php.net/manual/en/ref.datetime.php

For example, you might want to use the date() function to display the
current time on a whim. However, doing this ruins everything.

Even if other parts of your code use Carbon or comply with PSR-20, using
the date() function is problematic because the current time it references
cannot be modified.

`date_test_now(\DateInterval $shiftInterval)` can solve this problem.

Of course, there might be various side effects. However, seeing
`Carbon::setTestNow()` being used in various places, I think this feature
might be necessary.

I would appreciate your thoughts on this.

Best Regards,
Go Kudo

Larry Garfield

2 years ago

On Mon, Jul 1, 2024, at 3:56 PM, Go Kudo wrote:

> I apologize, the main point of my explanation was off.
>
> To put it simply, I'm suggesting that it might be good to implement
> convenient and commonly used features from Carbon at the ext-date level.
>
> I'm proposing the date_test_set_now() function for the following reasons:
>
> User-land libraries like Carbon / Chronos have a setTestNow method,
> indicating a potential demand for this feature.
> It's impossible to determine whether a value is relative or absolute
> time from either user-land or Extension.
> While this is convenient for maintaining legacy systems, that's not the
> essence of this proposal.
>
> As you pointed out, this is an issue that should ideally be solved in
> user-land. I deeply understand that.
>
> However, in reality, PHP's time-related processing is diverse, and to
> comprehensively handle all of these, it seems necessary to address this
> at the ext-date level.
>
> https://www.php.net/manual/en/ref.datetime.php
>
> For example, you might want to use the date() function to display the
> current time on a whim. However, doing this ruins everything.
>
> Even if other parts of your code use Carbon or comply with PSR-20,
> using the date() function is problematic because the current time it
> references cannot be modified.
>
> `date_test_now(\DateInterval $shiftInterval)` can solve this problem.
>
> Of course, there might be various side effects. However, seeing
> `Carbon::setTestNow()` being used in various places, I think this
> feature might be necessary.
>
> I would appreciate your thoughts on this.
>
> Best Regards,
> Go Kudo

They are unchanged.

> For example, you might want to use the date() function to display the
> current time on a whim.

So don't do that.  Relying on global mutable state is a bug.  That older parts of the stdlib made that mistake doesn't mean we should continue it.  (See also the Random extension, which also works to avoid global mutable state.)

> Of course, there might be various side effects.

Exactly.  This is not something to just brush aside with a comment.

The way Carbon does it is wrong, for the same reason: It just sets a global state.  The correct answer is to use PSR-20 and inject a fixed-time instance of the Clock.

--Larry Garfield

Lanre

2 years ago

Larry,

Your absolutes and comments like "Relying on global mutable state is a bug"
are incredibly frustrating. Are the authors of Carbon and Chronos, the two
biggest userland time libraries, somehow wrong because they aren’t you? Do
you think they didn’t put any thought into their decisions and just relied
on global mutable state for no reason?

There are multiple internal functions that rely on global state, such as
date_default_timezone_set(), date_default_timezone_get(), mktime(),
gmmktime(), setlocale(), localeconv(), set_error_handler(),
restore_error_handler(), set_exception_handler(),
restore_exception_handler(), session_start(), session_destroy(),
session_set_save_handler(), session_id(), srand(), rand(), mt_srand(),
mt_rand(), stream_context_create(), stream_context_set_option(),
stream_context_set_params(), ob_start(), ob_get_contents(), ob_get_clean(),
ob_flush(), ob_end_flush(), import_request_variables(), ini_set(), ini_get(),
ini_restore(), timezone_identifiers_list(), and timezone_name_from_abbr(),
are those also to be considered bugs?

Isn’t PSR a recommendation, not a mandate? How can it be "the correct
answer"? (we get it, you're a member of PHP-FIG)

You don’t make any effort to help or provide constructive criticism; you
just love to (incorrectly) tell everyone what they're doing wrong and how
they should be doing it, without adding anything constructive to the
conversation.

Cheers,

Lanre

On Mon, Jul 1, 2024 at 12:13 PM Larry Garfield <larry@garfieldtech.com>
wrote:

Lanre

2 years ago

On Mon, Jul 1, 2024 at 12:42 PM Lanre <lnearwaju@gmail.com> wrote:

> Larry,
>
> Your absolutes and comments like "Relying on global mutable state is a
> bug" are incredibly frustrating. Are the authors of Carbon and Chronos, the
> two biggest userland time libraries, somehow wrong because they aren’t you?
> Do you think they didn’t put any thought into their decisions and just
> relied on global mutable state for no reason?
>
> There are multiple internal functions that rely on global state, such as
> date_default_timezone_set(), date_default_timezone_get(), mktime(),
> gmmktime(), setlocale(), localeconv(), set_error_handler(),
> restore_error_handler(), set_exception_handler(),
> restore_exception_handler(), session_start(), session_destroy(),
> session_set_save_handler(), session_id(), srand(), rand(), mt_srand(),
> mt_rand(), stream_context_create(), stream_context_set_option(),
> stream_context_set_params(), ob_start(), ob_get_contents(), ob_get_clean(),
> ob_flush(), ob_end_flush(), import_request_variables(), ini_set(),
> ini_get(), ini_restore(), timezone_identifiers_list(), and timezone_name_from_abbr(),
> are those also to be considered bugs?
>
> Isn’t PSR a recommendation, not a mandate? How can it be "the correct
> answer"? (we get it, you're a member of PHP-FIG)
>
> You don’t make any effort to help or provide constructive criticism; you
> just love to (incorrectly) tell everyone what they're doing wrong and how
> they should be doing it, without adding anything constructive to the
> conversation.
>
> Cheers,
>
> Lanre
>
> On Mon, Jul 1, 2024 at 12:13 PM Larry Garfield <larry@garfieldtech.com>
> wrote:
>
>> On Mon, Jul 1, 2024, at 3:56 PM, Go Kudo wrote:
>>
>> > I apologize, the main point of my explanation was off.
>> >
>> > To put it simply, I'm suggesting that it might be good to implement
>> > convenient and commonly used features from Carbon at the ext-date level.
>> >
>> > I'm proposing the date_test_set_now() function for the following
>> reasons:
>> >
>> > User-land libraries like Carbon / Chronos have a setTestNow method,
>> > indicating a potential demand for this feature.
>> > It's impossible to determine whether a value is relative or absolute
>> > time from either user-land or Extension.
>> > While this is convenient for maintaining legacy systems, that's not the
>> > essence of this proposal.
>> >
>> > As you pointed out, this is an issue that should ideally be solved in
>> > user-land. I deeply understand that.
>> >
>> > However, in reality, PHP's time-related processing is diverse, and to
>> > comprehensively handle all of these, it seems necessary to address this
>> > at the ext-date level.
>> >
>> > https://www.php.net/manual/en/ref.datetime.php
>> >
>> > For example, you might want to use the date() function to display the
>> > current time on a whim. However, doing this ruins everything.
>> >
>> > Even if other parts of your code use Carbon or comply with PSR-20,
>> > using the date() function is problematic because the current time it
>> > references cannot be modified.
>> >
>> > `date_test_now(\DateInterval $shiftInterval)` can solve this problem.
>> >
>> > Of course, there might be various side effects. However, seeing
>> > `Carbon::setTestNow()` being used in various places, I think this
>> > feature might be necessary.
>> >
>> > I would appreciate your thoughts on this.
>> >
>> > Best Regards,
>> > Go Kudo
>>
>> They are unchanged.
>>
>> > For example, you might want to use the date() function to display the
>> > current time on a whim.
>>
>> So don't do that.  Relying on global mutable state is a bug.  That older
>> parts of the stdlib made that mistake doesn't mean we should continue it.
>> (See also the Random extension, which also works to avoid global mutable
>> state.)
>>
>> > Of course, there might be various side effects.
>>
>> Exactly.  This is not something to just brush aside with a comment.
>>
>> The way Carbon does it is wrong, for the same reason: It just sets a
>> global state.  The correct answer is to use PSR-20 and inject a fixed-time
>> instance of the Clock.
>>
>> --Larry Garfield
>>
>

My bad for the top post, still getting used to it. Also i think this RFC is
a great idea and will be one less reason to use a third party userland
library to handle time.

Cheers,

Lanre

Joerg Sowa

2 years ago

> You don’t make any effort to help or provide constructive criticism; you

just love to (incorrectly) tell everyone what they're doing wrong and how
they should be doing it, without adding anything constructive to the
conversation.

Please limit to the arguments on the topic, not a person.

The argument "Relying on the global mutable state is a bug" is completely
valid and in most of the cases is the signal of bad design. And it has been
already addressed in some of the functions, look at
https://wiki.php.net/rfc/deprecations_php_8_3 for deprecations on rand
functions. However, it's not so trivial to fix everything introducing
compatibility-breaking changes. So it shouldn't be advised to add more of
such functions.

Kind regards,
Jorg

Lanre

2 years ago

On Mon, Jul 1, 2024 at 2:21 PM Jorg Sowa <jorg.sowa@gmail.com> wrote:

> > You don’t make any effort to help or provide constructive criticism; you
> just love to (incorrectly) tell everyone what they're doing wrong and how
> they should be doing it, without adding anything constructive to the
> conversation.
>
> Please limit to the arguments on the topic, not a person.
>
> The argument "Relying on the global mutable state is a bug" is completely
> valid and in most of the cases is the signal of bad design. And it has been
> already addressed in some of the functions, look at
> https://wiki.php.net/rfc/deprecations_php_8_3 for deprecations on rand
> functions. However, it's not so trivial to fix everything introducing
> compatibility-breaking changes. So it shouldn't be advised to add more of
> such functions.
>
> Kind regards,
> Jorg
>

It still doesn't make sense. Even if we removed all the functions from my
list, the entire engine remains filled with global state. Internal
zend_class_entry instances, arginfo macros, the interned string table,
executor/compiler/scanner/each-extension globals, and so on—all contribute
to global state -- or bugs according to you and Larry, unless your stupid
argument applies only to languages under your control. Do I think it always
makes sense to use global state? Absolutely not. But I won't go as far as
to imply that relying on 'global mutable state' inevitably leads to bugs.

Cheers,
Lanre

Derick Rethans

2 years ago

On Mon, 1 Jul 2024, Lanre wrote:

> On Mon, Jul 1, 2024 at 2:21 PM Jorg Sowa <jorg.sowa@gmail.com> wrote:
>
> > > You don’t make any effort to help or provide constructive criticism; you
> > just love to (incorrectly) tell everyone what they're doing wrong and how
> > they should be doing it, without adding anything constructive to the
> > conversation.
> >
> > Please limit to the arguments on the topic, not a person.
> >
> > The argument "Relying on the global mutable state is a bug" is completely
> > valid and in most of the cases is the signal of bad design. And it has been
> > already addressed in some of the functions, look at
> > https://wiki.php.net/rfc/deprecations_php_8_3 for deprecations on rand
> > functions. However, it's not so trivial to fix everything introducing
> > compatibility-breaking changes. So it shouldn't be advised to add more of
> > such functions.
> >
>
> It still doesn't make sense. Even if we removed all the functions from my
> list, the entire engine remains filled with global state. Internal
> zend_class_entry instances, arginfo macros, the interned string table,
> executor/compiler/scanner/each-extension globals, and so on—all contribute
> to global state -- or bugs according to you and Larry, unless your stupid
> argument applies only to languages under your control.

You really need to tone this langauge down. It's not the first time that
this has been requested of you. Ad-hominems are not welcome here.

I will point out the mailing list rules once more (only):
https://github.com/php/php-src/blob/master/docs/mailinglist-rules.md

cheers,
Derick

zeriyoshi

2 years ago

Hi.

> So don't do that.  Relying on global mutable state is a bug.  That older

parts of the stdlib made that mistake doesn't mean we should continue it.
(See also the Random extension, which also works to avoid global mutable
state.)

I generally agree. However, I have some questions.

It's true that relying on global state is generally undesirable. I dislike
stateful approaches so much that I proposed and implemented ext-random in
PHP 8.2.

However, the concept of date and time is stateful in the real world we live
in. For example, time varies due to various factors such as time zones and
daylight saving time.
Even now, if you execute `date_default_timezone_set()`, the current time
can potentially rewind.

https://3v4l.org/FJn4e

This behavior completely depends on the internal state of ext-date, but I
don't think it should be abolished because changing the time zone to
consider during execution is also a realistic scenario.
Similarly, I don't think the proposed `date_test_set_now()` will create new
flaws.

There's also the issue of convenience. It's possible to take an approach
similar to ext-random for ext-date, but that would result in a significant
increase in the amount of code to write.
Additionally, there's the problem that the current `\DateTimeInterface`
doesn't cover all the functionality provided by ext-date functions, making
a drop-in replacement impossible. (ext-random is completely replaceable)

As an aside, I once proposed deprecating the `mt_rand()`, `mt_srand()`,
`rand()`, and `srand()` functions.
However, it was rejected on the grounds that it was unnecessary and
excessive for many workloads. Couldn't the same be said for this case?

https://wiki.php.net/rfc/deprecations_php_8_3

I look forward to your reply.

Best Regards
Go Kudo

2024年7月2日(火) 1:41 Larry Garfield <larry@garfieldtech.com>:

Alexandru Pătrănescu

2 years ago

>
>
> https://github.com/colopl/php-colopl_timeshifter
>
> However, this Extension has some problems.
>
> The first is that there's no way to determine whether the format passed to
> the ext-date parser is relative or absolute time, resulting in a dirty hack
> using usleep. The second is that it depends on timelib, so it breaks when
> upstream changes related to timelib are made.
>
> So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)`
> function to ext-date?
>
> This function would treat the current time as shifted by the passed
> DateInterval. Since it's implemented on the ext-date side, there's no need
> for dirty hacks using usleep.
>
>

I can see how this would be useful on legacy applications, not forcing many
changes everywhere.

However, I think that we should not encourage this and keep it out of php
core offering.

Having it as a separate php extension sounds good to me, and as long as
that php extension is loaded and used only while running tests, it helps
with not possibly impacting production in any way.
Indeed, it mighty have some limitations but I think we can work around them.

An alternative solution would be to have namespaced functions and classes
and use rector to automate the replacement of native ones just before
running tests and package everything as a testing php library.

Alex

Andreas Heigl

2 years ago

Hey

Am 01.07.24 um 15:07 schrieb Go Kudo:

> Hi, Internals.
>
> I've been absent for a long time due to poor health. I'm finally back.
>
> I maintain a legacy application written in PHP, and occasionally need to
> fake the current time for testing purposes. However, PHP doesn't provide
> a standard way to fake the current time, so I've been changing the OS's
> current time to do this, which is quite painful.
>
> This can be avoided by using third-party libraries (such as
> Carbon::setTestNow()). However, it's almost impossible to modify all
> parts of a legacy application that depend on the current time.
>
> Another option is to use libfaketime
> (https://github.com/wolfcw/libfaketime
> <https://github.com/wolfcw/libfaketime>), but this is also quite painful
> to use.
>
> Since this was absolutely necessary for my work, I implemented this
> functionality as a PHP Extension. (Please ignore the dirty
> implementation related to PDO. I'm not planning to propose it this time.)
>
> https://github.com/colopl/php-colopl_timeshifter
> <https://github.com/colopl/php-colopl_timeshifter>
>
> However, this Extension has some problems.
>
> The first is that there's no way to determine whether the format passed
> to the ext-date parser is relative or absolute time, resulting in a
> dirty hack using usleep. The second is that it depends on timelib, so it
> breaks when upstream changes related to timelib are made.
>
> So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)`
> function to ext-date?
>
> This function would treat the current time as shifted by the passed
> DateInterval. Since it's implemented on the ext-date side, there's no
> need for dirty hacks using usleep.
>
> I'd like to hear your opinions. Thank yo

Instead of tweaking the PHP sources and open them up another hellhole of
things that will go south, I'd ratehr use PSR 20[1] for handling times
in application tests.

Also you can already overwrite the `time` function within a namespace
for testing purposes and IIRC there is a way to do that with Carbon.

Just my 0.02 €

Cheers

Andreas

[https://www.php-fig.org/psr/psr-20/]

--
,,,
(o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl                                                       |
| mailto:andreas@heigl.org                  N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org                                           |
+---------------------------------------------------------------------+
| https://hei.gl/appointmentwithandreas                               |
+---------------------------------------------------------------------+
| GPG-Key: https://hei.gl/keyandreasheiglorg                          |
+---------------------------------------------------------------------+

Rob Landers

2 years ago

On Mon, Jul 1, 2024, at 15:07, Go Kudo wrote:

> Hi, Internals.
>
> I've been absent for a long time due to poor health. I'm finally back.
>
> I maintain a legacy application written in PHP, and occasionally need to fake the current time for testing purposes. However, PHP doesn't provide a standard way to fake the current time, so I've been changing the OS's current time to do this, which is quite painful.
>
> This can be avoided by using third-party libraries (such as Carbon::setTestNow()). However, it's almost impossible to modify all parts of a legacy application that depend on the current time.
>
> Another option is to use libfaketime (https://github.com/wolfcw/libfaketime), but this is also quite painful to use.
>
> Since this was absolutely necessary for my work, I implemented this functionality as a PHP Extension. (Please ignore the dirty implementation related to PDO. I'm not planning to propose it this time.)
>
> https://github.com/colopl/php-colopl_timeshifter
>
> However, this Extension has some problems.
>
> The first is that there's no way to determine whether the format passed to the ext-date parser is relative or absolute time, resulting in a dirty hack using usleep. The second is that it depends on timelib, so it breaks when upstream changes related to timelib are made.
>
> So, how about adding a `date_set_test_now(\DateInterval $shiftInterval)` function to ext-date?
>
> This function would treat the current time as shifted by the passed DateInterval. Since it's implemented on the ext-date side, there's no need for dirty hacks using usleep.
>
> I'd like to hear your opinions. Thank you.
>
> Best Regards,
> Go Kudo

Welcome back!

I think this is an interesting proposal and very useful. I think many people may be concerned about it being used in a non-testing context, but I don’t think we should stop people from being creative. If people find a use for it outside of testing, that would be interesting to see. The fact they can already do this today via carbon just makes a point that this is valuable to those of us who don’t like adding dependencies.

Ps. Anytime someone mentions PSR, have your favorite drink. It’ll help take the edge off. :) In all seriousness, PSR doesn’t apply universally and shouldn’t ever be a valid argument on this list. That’s my 2¢ anyway, best of luck to you.

— Rob

Derick Rethans

2 years ago

On Mon, 1 Jul 2024, Go Kudo wrote:

> So, how about adding a `date_set_test_now(\DateInterval
> $shiftInterval)` function to ext-date?
>
> This function would treat the current time as shifted by the passed
> DateInterval. Since it's implemented on the ext-date side, there's no
> need for dirty hacks using usleep.

It's not that easy, as maney calculations are done in "timelib".

Especially for shifting times with relative times, the parser will need
to understand that too I suppose.

I am not keen on adding this global override to this library.

cheers,
Derick