CRM_Utils_Time::setTime and getTime() and their use in unit tests
They're used partly in tests but seem prone to rollover issues and the use of an internal $delta offset doesn't make sense (to me) in tests. I traced the history - see farther down - and I think before being used in tests it got co-opted for use in queues, and then when it later got used in tests it was maybe based on how it originally worked and/or a misunderstanding/unknowing that its function had been reworked.
Example:
- phpunit/CRM/Core/BAO/ActionScheduleTest::assertCronRuns() calls CRM_Utils_Time::setTime(). This sets $delta to some number, let's say
-1000
just for something specific. - Call api/v3/Job.php::civicrm_api3_job_send_reminder() with no parameters.
- This calls CRM_Core_BAO_ActionSchedule::processQueue() with NULL for the datetime param.
- processQueue() then ends up calling CRM_Utils_Time::getTime(), which returns
time() + 1000
, which if time() hasn't changed, is the original time we passed into setTime() earlier, but if it has changed, we get a different time.
So this is where I was originally confused. If the intent was to use this as a way to set and get a "fake" time for testing purposes, then why not just have setTime() store the actual fake time that was passed in and then have getTime() return that?
I checked back and they were introduced here: https://github.com/civicrm/civicrm-svn/commit/af28c476c4469e430f365461e65e18146d730c42.
And they did just store the time not a delta!
Then it got reworked here: https://github.com/civicrm/civicrm-svn/commit/923ba87fe13087d220278dd3dea9db68216e56da,
but that was before ActionScheduleTest,
which was added here https://github.com/civicrm/civicrm-svn/blob/e5910b77f4d3e34707c247a2e97b4cec51dfb965/tests/phpunit/CRM/Core/BAO/ActionScheduleTest.php
Thus, flaky test. This particular one doesn't come up much in the PR queues though, probably because it's too fast.