MAGETWO-71359: Test cron expressions against localized time #10432

 - fixed timezone conversion logic
 - fixed unit tests

Author: Oleksii Korshenko

app/code/Magento/Cron/Model/Schedule.php

@@ -7,6 +7,8 @@
 namespace Magento\Cron\Model;
 
 use Magento\Framework\Exception\CronException;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
 
 /**
  * Crontab schedule model
@@ -45,28 +47,28 @@ class Schedule extends \Magento\Framework\Model\AbstractModel
     const STATUS_ERROR = 'error';
 
     /**
-     * @var \Magento\Framework\Stdlib\DateTime\DateTime
+     * @var TimezoneInterface
      */
-    private $dateTime;
+    private $timezoneConverter;
 
     /**
      * @param \Magento\Framework\Model\Context $context
      * @param \Magento\Framework\Registry $registry
      * @param \Magento\Framework\Model\ResourceModel\AbstractResource $resource
      * @param \Magento\Framework\Data\Collection\AbstractDb $resourceCollection
      * @param array $data
-     * @param \Magento\Framework\Stdlib\DateTime\DateTime|null $dateTime
+     * @param TimezoneInterface $timezoneConverter
      */
     public function __construct(
         \Magento\Framework\Model\Context $context,
         \Magento\Framework\Registry $registry,
         \Magento\Framework\Model\ResourceModel\AbstractResource $resource = null,
         \Magento\Framework\Data\Collection\AbstractDb $resourceCollection = null,
         array $data = [],
-        \Magento\Framework\Stdlib\DateTime\DateTime $dateTime = null
+        TimezoneInterface $timezoneConverter = null
     ) {
         parent::__construct($context, $registry, $resource, $resourceCollection, $data);
-        $this->dateTime = $dateTime;
+        $this->timezoneConverter = $timezoneConverter ?: ObjectManager::getInstance()->get(TimezoneInterface::class);
     }
 
     /**
@@ -102,16 +104,17 @@ public function setCronExpr($expr)
      */
     public function trySchedule()
     {
-        $this->dateTime = $this->dateTime ?: \Magento\Framework\App\ObjectManager::getInstance()
-            ->get(\Magento\Framework\Stdlib\DateTime\DateTime::class);
         $time = $this->getScheduledAt();
         $e = $this->getCronExprArr();
 
         if (!$e || !$time) {
             return false;
         }
         if (!is_numeric($time)) {
-            $time = strtotime($time) + $this->dateTime->getGmtOffset();
+            //convert time from UTC to admin store timezone
+            //we assume that all schedules in configuration (crontab.xml and DB tables) are in admin store timezone
+            $time = $this->timezoneConverter->date($time)->format('Y-m-d H:i');
+            $time = strtotime($time);
         }
         $match = $this->matchCronExpression($e[0], strftime('%M', $time))
             && $this->matchCronExpression($e[1], strftime('%H', $time))

app/code/Magento/Cron/Test/Unit/Model/ScheduleTest.php

@@ -13,6 +13,9 @@
  */
 class ScheduleTest extends \PHPUnit_Framework_TestCase
 {
+    /**
+     * @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
+     */
     protected $helper;
 
     protected $resourceJobMock;
@@ -174,6 +177,36 @@ public function testTrySchedule($scheduledAt, $cronExprArr, $expected)
         $this->assertEquals($expected, $result);
     }
 
+
+    public function testTryScheduleWithConversionToAdminStoreTime()
+    {
+        $scheduledAt = '2011-12-13 14:15:16';
+        $cronExprArr = ['*', '*', '*', '*', '*'];
+
+        // 1. Create mocks
+        $timezoneConverter = $this->getMock(\Magento\Framework\Stdlib\DateTime\TimezoneInterface::class);
+        $timezoneConverter->expects($this->once())
+            ->method('date')
+            ->with($scheduledAt)
+            ->willReturn(new \DateTime($scheduledAt));
+
+        /** @var \Magento\Cron\Model\Schedule $model */
+        $model = $this->helper->getObject(
+            \Magento\Cron\Model\Schedule::class,
+            ['timezoneConverter' => $timezoneConverter]
+        );
+
+        // 2. Set fixtures
+        $model->setScheduledAt($scheduledAt);
+        $model->setCronExprArr($cronExprArr);
+
+        // 3. Run tested method
+        $result = $model->trySchedule();
+
+        // 4. Compare actual result with expected result
+        $this->assertTrue($result);
+    }
+
     /**
      * @return array
      */
@@ -187,7 +220,6 @@ public function tryScheduleDataProvider()
             [$date, [], false],
             [$date, null, false],
             [$date, false, false],
-            [$date, ['*', '*', '*', '*', '*'], true],
             [strtotime($date), ['*', '*', '*', '*', '*'], true],
             [strtotime($date), ['15', '*', '*', '*', '*'], true],
             [strtotime($date), ['*', '14', '*', '*', '*'], true],

app/code/Magento/Cron/Test/Unit/Observer/ProcessCronQueueObserverTest.php

@@ -675,7 +675,7 @@ public function testDispatchRemoveConfigMismatch()
         ]);
 
         $jobs = [];
-        for ($i = 0; $i<20; $i++) {
+        for ($i = 0; $i < 20; $i++) {
             $time = date('Y-m-d H:i:00', strtotime("+$i minutes"));
             $jobs[] = [
                 'age' => $time,
@@ -704,9 +704,20 @@ public function testDispatchRemoveConfigMismatch()
         $this->_collection->addItem($schedule);
 
         $scheduleMock = $this->getMockBuilder(Schedule::class)->disableOriginalConstructor()
-            ->setMethods(['save', 'getCollection', 'getResource'])->getMock();
+            ->setMethods(['save', 'getCollection', 'getResource', 'trySchedule'])->getMock();
+
         $scheduleMock->expects($this->any())->method('getCollection')->will($this->returnValue($this->_collection));
         $scheduleMock->expects($this->any())->method('getResource')->will($this->returnValue($this->scheduleResource));
+
+        $callIndex = 0;
+        $scheduleMock->expects($this->any())->method('trySchedule')->willReturnCallback(
+            function () use (&$callIndex, $jobs) {
+                $output = $jobs[$callIndex]['delete'];
+                $callIndex++;
+                return !$output;
+            }
+        );
+
         $this->_scheduleFactory->expects($this->any())->method('create')->will($this->returnValue($scheduleMock));
 
         $query = [