Merge remote-tracking branch 'upstream/1.x' into track-cron-hostname

Ethan3600 · May 17, 2020 · 20ec364 · 20ec364
2 parents 29ca0d1 + e234bed
commit 20ec364
Show file tree

Hide file tree

Showing 20 changed files with 156 additions and 33 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,6 +1,8 @@
 sudo: required
 dist: trusty
 group: edge
+services:
+  - elasticsearch
 addons:
   apt:
     packages:
@@ -33,14 +35,6 @@ matrix:
       env:
        - MAGENTO_VERSION=2.3
        - TEST_SUITE=unit
-    - php: 7.2
-      env:
-       - MAGENTO_VERSION=2.4-develop
-       - TEST_SUITE=integration
-    - php: 7.2
-      env:
-       - MAGENTO_VERSION=2.4-develop
-       - TEST_SUITE=unit
     - php: 7.2
       env:
        - MAGENTO_VERSION=2.3

diff --git a/.travis/before_script.sh b/.travis/before_script.sh
@@ -16,6 +16,9 @@ phpenv rehash;
 
 composer selfupdate
 
+# Allow elasticsearch service to start. See https://docs.travis-ci.com/user/database-setup/#elasticsearch
+sleep 10
+
 # clone main magento github repository
 git clone --branch $MAGENTO_VERSION --depth=1 https://github.com/magento/magento2
 

diff --git a/Api/ScheduleManagementInterface.php b/Api/ScheduleManagementInterface.php
@@ -4,6 +4,7 @@
 
 use Magento\Cron\Model\Schedule;
 use Magento\Framework\Exception\CouldNotSaveException;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Exception\NoSuchEntityException;
 
 interface ScheduleManagementInterface
@@ -26,7 +27,8 @@ public function listJobs(): array;
     /**
      * @param string $jobCode
      * @param string[]|null $groups
-     * @return string|null
+     * @return string
+     * @throws LocalizedException
      */
     public function getGroupId(string $jobCode, $groups = null);
 

diff --git a/Model/ScheduleManagement.php b/Model/ScheduleManagement.php
@@ -9,6 +9,7 @@
 use EthanYehuda\CronjobManager\Api\JobManagementInterface;
 use Magento\Cron\Model\ScheduleFactory;
 use Magento\Cron\Model\Schedule;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Stdlib\DateTime\DateTime;
 
 class ScheduleManagement implements ScheduleManagementInterface
@@ -109,7 +110,10 @@ public function getGroupId(string $jobCode, $groups = null): string
             }
         }
 
-        return null;
+        throw new LocalizedException(__(
+            'No such job: %jobCode',
+            ['jobCode' => $jobCode]
+        ));
     }
 
     public function flush(): bool

diff --git a/Plugin/Cron/Model/SchedulePlugin.php b/Plugin/Cron/Model/SchedulePlugin.php
@@ -4,14 +4,26 @@
 namespace EthanYehuda\CronjobManager\Plugin\Cron\Model;
 
 use Magento\Cron\Model\Schedule;
+use Magento\Cron\Model\ResourceModel\Schedule as ScheduleResource;
 
 class SchedulePlugin
 {
+    /**
+     * @var ScheduleResource
+     */
+    protected $scheduleResource;
+
+    public function __construct(ScheduleResource $scheduleResource)
+    {
+        $this->scheduleResource = $scheduleResource;
+    }
+
     public function afterTryLockJob(Schedule $subject, bool $result)
     {
         if ($result) {
             $subject->setData('hostname', \gethostname());
             $subject->setData('pid', \getmypid());
+            $this->scheduleResource->save($subject);
         }
         return $result;
     }

diff --git a/Test/Integration/CleanRunningJobsTest.php b/Test/Integration/CleanRunningJobsTest.php
@@ -25,10 +25,12 @@ class CleanRunningJobsTest extends TestCase
      * @var ObjectManager
      */
     private $objectManager;
+
     /**
      * @var Event\ManagerInterface
      */
     private $eventManager;
+
     /**
      * @var FakeClock
      */

diff --git a/Test/Integration/Controller/Adminhtml/Manage/CreateTest.php b/Test/Integration/Controller/Adminhtml/Manage/CreateTest.php
@@ -15,6 +15,10 @@ public function testEditAction()
         $this->dispatch($this->uri);
         $result = $this->getResponse()->getBody();
 
-        $this->assertContains('<title>Create Cron Job / Tools / System / Magento Admin</title>', $result);
+        if (\method_exists($this, 'assertStringContainsString')) {
+            $this->assertStringContainsString('<title>Create Cron Job / Tools / System / Magento Admin</title>', $result);
+        } else {
+            $this->assertContains('<title>Create Cron Job / Tools / System / Magento Admin</title>', $result);
+        }
     }
 }
diff --git a/Test/Integration/Controller/Adminhtml/Manage/EditTest.php b/Test/Integration/Controller/Adminhtml/Manage/EditTest.php
@@ -15,6 +15,10 @@ public function testEditAction()
         $this->dispatch($this->uri);
         $result = $this->getResponse()->getBody();
 
-        $this->assertContains('<title>Edit Cron Job / Tools / System / Magento Admin</title>', $result);
+        if (\method_exists($this, 'assertStringContainsString')) {
+            $this->assertStringContainsString('<title>Edit Cron Job / Tools / System / Magento Admin</title>', $result);
+        } else {
+            $this->assertContains('<title>Edit Cron Job / Tools / System / Magento Admin</title>', $result);
+        }
     }
 }
diff --git a/Test/Integration/Controller/Adminhtml/Manage/IndexTest.php b/Test/Integration/Controller/Adminhtml/Manage/IndexTest.php
@@ -18,7 +18,11 @@ public function testIndexAction()
         $this->dispatch($this->uri);
         $result = $this->getResponse()->getBody();
 
-        $this->assertContains('<title>Cron Job Dashboard / Tools / System / Magento Admin</title>', $result);
+        if (\method_exists($this, 'assertStringContainsString')) {
+            $this->assertStringContainsString('<title>Cron Job Dashboard / Tools / System / Magento Admin</title>', $result);
+        } else {
+            $this->assertContains('<title>Cron Job Dashboard / Tools / System / Magento Admin</title>', $result);
+        }
     }
 
     public static function loadDataFixtureCron()

diff --git a/Test/Integration/ErrorNotificationEmailTest.php b/Test/Integration/ErrorNotificationEmailTest.php
@@ -22,16 +22,18 @@ class ErrorNotificationEmailTest extends TestCase
      * @var ObjectManager
      */
     private $objectManager;
+
     /**
      * @var ErrorNotificationEmail
      */
     private $errorNotificationEmail;
+
     /**
      * @var \Magento\TestFramework\Mail\Template\TransportBuilderMock
      */
     private $transportBuilder;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->objectManager = Bootstrap::getObjectManager();
         $this->transportBuilder = $this->objectManager->get(TransportBuilderMock::class);
@@ -131,7 +133,11 @@ private function andEmailShouldHaveContents(Message $sentMessage, array $expecte
         $content = $sentMessage->getBody()->getParts()[0]->getContent();
         $content = \Zend_Mime_Decode::decodeQuotedPrintable($content);
         foreach ($expectedContents as $expectedKey => $expectedContent) {
-            $this->assertContains($expectedContent, $content, "Content should contain $expectedKey");
+            if (\method_exists($this, 'assertStringContainsString')) {
+                $this->assertStringContainsString($expectedContent, $content, "Content should contain $expectedKey");
+            } else {
+                $this->assertContains($expectedContent, $content, "Content should contain $expectedKey");
+            }
         }
     }
 }
diff --git a/Test/Integration/ErrorNotificationTest.php b/Test/Integration/ErrorNotificationTest.php
@@ -28,6 +28,7 @@
 class ErrorNotificationTest extends TestCase
 {
     const NOW = '2019-02-09 18:33:00';
+
     /**
      * @var ObjectManager
      */
@@ -37,28 +38,33 @@ class ErrorNotificationTest extends TestCase
      * @var ScheduleManagementInterface
      */
     private $scheduleManagement;
+
     /**
      * @var ScheduleRepositoryInterface
      */
     private $scheduleRepository;
+
     /**
      * @var FakeClock
      */
     private $clock;
+
     /**
      * @var \Magento\Framework\App\CacheInterface
      */
     private $cache;
+
     /**
      * @var ErrorNotification|\PHPUnit_Framework_MockObject_MockObject
      */
     private $errorNotification;
+
     /**
      * @var ProcessCronQueueObserver
      */
     private $processCronQueueObserver;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->objectManager = Bootstrap::getObjectManager();
         $this->objectManager->configure(
@@ -156,4 +162,4 @@ function (Schedule $schedule) use ($expectedMessage) {
             )
         );
     }
-}
+}
diff --git a/Test/Integration/Model/ManagerTest.php b/Test/Integration/Model/ManagerTest.php
@@ -7,6 +7,7 @@
 use Magento\TestFramework\Helper\Bootstrap;
 use EthanYehuda\CronjobManager\Model\Manager;
 use Magento\Cron\Model\ScheduleFactory;
+use Magento\Framework\Exception\NoSuchEntityException;
 
 class ManagerTest extends TestCase
 {
@@ -21,10 +22,10 @@ class ManagerTest extends TestCase
      */
     private $scheduleFactory;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $objectManager = Bootstrap::getObjectManager();
-        
+
         $this->manager = $objectManager->create(Manager::class);
         $this->scheduleFactory = $objectManager->create(ScheduleFactory::class);
     }
@@ -50,11 +51,10 @@ public function testSaveCronJob()
         $this->assertEquals(Schedule::STATUS_SUCCESS, $cron->getStatus());
     }
 
-    /**
-     * @expectedException \Magento\Framework\Exception\NoSuchEntityException
-     */
     public function testSaveCronInvalidId()
     {
+        $this->expectException(NoSuchEntityException::class);
+        $this->expectExceptionMessage('The Schedule with the "99999" ID doesn\'t exist');
         $this->manager->saveCronJob(99999);
     }
 
@@ -69,11 +69,10 @@ public function testDeleteCronJob()
         $this->assertNull($cron->getScheduleId());
     }
 
-    /**
-     * @expectedException \Magento\Framework\Exception\NoSuchEntityException
-     */
     public function testDeleteInvalidId()
     {
+        $this->expectException(NoSuchEntityException::class);
+        $this->expectExceptionMessage('The Schedule with the "99999" ID doesn\'t exist');
         $this->manager->deleteCronJob(99999);
     }
 

diff --git a/Test/Integration/Model/ScheduleManagementTest.php b/Test/Integration/Model/ScheduleManagementTest.php
@@ -5,6 +5,7 @@
 
 use EthanYehuda\CronjobManager\Api\ScheduleManagementInterface;
 use Magento\Cron\Model\Schedule;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\TestFramework\Helper\Bootstrap;
 use Magento\TestFramework\ObjectManager;
 use PHPUnit\Framework\TestCase;
@@ -26,11 +27,25 @@ class ScheduleManagementTest extends TestCase
 
     const NOW = '2019-02-09 18:33:00';
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->objectManager = Bootstrap::getObjectManager();
         $this->scheduleManagement = $this->objectManager->get(ScheduleManagementInterface::class);
     }
+
+    public function testGetGroupIdWithValidJobCode()
+    {
+        $groupId = $this->scheduleManagement->getGroupId('backend_clean_cache');
+        $this->assertSame('default', $groupId);
+    }
+
+    public function testGetGroupIdWithInvalidJobCode()
+    {
+        $this->expectException(LocalizedException::class);
+        $this->expectExceptionMessage('No such job: not_valid');
+        $this->scheduleManagement->getGroupId('not_valid');
+    }
+
     public function testKillRequestForRunningJobSucceeds()
     {
         $this->givenRunningSchedule($schedule);

diff --git a/Test/Integration/ProcessIdTest.php b/Test/Integration/ProcessIdTest.php
@@ -18,17 +18,19 @@ class ProcessIdTest extends TestCase
      */
     private $objectManager;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->objectManager = Bootstrap::getObjectManager();
     }
+
     public function testProcessIdSavedOnStart()
     {
         $this->givenPid($pid);
         $this->givenPendingSchedule($schedule);
         $this->whenTryLockJob($schedule);
         $this->thenScheduleIsSavedWithPid($schedule, $pid);
     }
+
     public function testProcessIdMaintainedAfterSuccesfulRun()
     {
         $this->givenPid($pid);

diff --git a/Test/Integration/ProcessKillRequestsTest.php b/Test/Integration/ProcessKillRequestsTest.php
@@ -27,22 +27,27 @@ class ProcessKillRequestsTest extends TestCase
      * @var int
      */
     private $childPid = 0;
+
     /**
      * @var ObjectManager
      */
     private $objectManager;
+
     /**
      * @var Event\ManagerInterface
      */
     private $eventManager;
+
     /**
      * @var ScheduleManagementInterface
      */
     private $scheduleManagement;
+
     /**
      * @var ProcessManagement
      */
     private $processManagement;
+
     /**
      * @var FakeClock
      */
@@ -89,7 +94,6 @@ public function testDeadRunningJobsOnAnotherHostAreNotCleaned()
 
     private function givenRunningScheduleWithKillRequest(&$schedule, int $timestamp)
     {
-
         /** @var Schedule $schedule */
         $schedule = $this->objectManager->create(Schedule::class);
         $schedule->setStatus(Schedule::STATUS_RUNNING);

diff --git a/Test/Integration/phpunit.xml.dist b/Test/Integration/phpunit.xml.dist
@@ -14,7 +14,7 @@
     </testsuites>
     <!-- Code coverage filters -->
     <filter>
-        <whitelist addUncoveredFilesFromWhiteList="true">
+        <whitelist>
             <directory suffix=".php">../../../vendor/ethanyehuda/magento2-cronjobmanager</directory>
             <exclude>
                 <directory>../../../vendor/ethanyehuda/magento2-cronjobmanager/Test</directory>

diff --git a/Test/Unit/Console/Command/KillJobTest.php b/Test/Unit/Console/Command/KillJobTest.php
@@ -35,7 +35,7 @@ class KillJobTest extends TestCase
     private $mockFilterBuilder;
     private $mockFilterGroupBuilder;
 
-    protected function setUp()
+    protected function setUp(): void
     {
         $this->mockState = $this->getMockBuilder(State::class)->setConstructorArgs([
             $this->createMock(ScopeInterface::class),
@@ -304,4 +304,3 @@ private function mockMultipleSchedules(int $numOfSchedules): array
         return $mockSchedules;
     }
 }
-