diff options
author | Ben Burgess <88810029+bx80@users.noreply.github.com> | 2022-02-02 02:19:23 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-02 02:19:23 +0300 |
commit | f20c4cbed3de7f3fcc49810d5721b257d5a56d43 (patch) | |
tree | da362660970e784654bf9f5ec1d4a55755ed1322 /tests/PHPUnit | |
parent | cbb297de4aed974716dd03acfee8832565ebf133 (diff) |
Retry scheduled tasks on failure (#18335)
* Catch and log errors for scheduled tasks
* Added retry on exception mechanism for scheduled tasks
* Replace error code with custom exception
* Fix/workaround for SchedulerTest mock breaking retry list option loading
* Log warning instead of info if a task has failed three times
* Added basic tests, minor code improvements
* Test fix
* Fix for test
* Added integration test to check that only tasks that throw exceptions are scheduled for retry
Diffstat (limited to 'tests/PHPUnit')
-rw-r--r-- | tests/PHPUnit/Integration/RetryScheduledTaskTest.php | 143 | ||||
-rw-r--r-- | tests/PHPUnit/System/TrackerTest.php | 3 | ||||
-rw-r--r-- | tests/PHPUnit/Unit/Scheduler/TimetableTest.php | 15 |
3 files changed, 160 insertions, 1 deletions
diff --git a/tests/PHPUnit/Integration/RetryScheduledTaskTest.php b/tests/PHPUnit/Integration/RetryScheduledTaskTest.php new file mode 100644 index 0000000000..36e055cd8f --- /dev/null +++ b/tests/PHPUnit/Integration/RetryScheduledTaskTest.php @@ -0,0 +1,143 @@ +<?php +/** + * Matomo - free/libre analytics platform + * + * @link https://matomo.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Tests\Integration; +use Piwik\Scheduler\RetryableException; +use Piwik\Scheduler\Timetable; +use Piwik\Scheduler\Scheduler; +use Piwik\Scheduler\Task; +use Piwik\Tests\Framework\Mock\PiwikOption; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; +use Psr\Log\NullLogger; +use ReflectionProperty; + +/** + * @group Scheduler + * @group SchedulerRetryTaskTest + */ +class RetryScheduledTaskTest extends IntegrationTestCase +{ + + public function testRetryCount() + { + + $timetable = new Timetable(); + + $task1 = 'task1'; + $task2 = 'task2'; + + // Should be zero if no retry entry present + $this->assertEquals(0, $timetable->getRetryCount($task1)); + + // Should increment by one + $timetable->incrementRetryCount($task1); + $this->assertEquals(1, $timetable->getRetryCount($task1)); + + // Should not break if more than one tasks is counting retries + $timetable->incrementRetryCount($task2); + $timetable->incrementRetryCount($task2); + $this->assertEquals(2, $timetable->getRetryCount($task2)); + $timetable->incrementRetryCount($task1); + $this->assertEquals(2, $timetable->getRetryCount($task1)); + + // Should clear retry count without affecting other tasks + $timetable->clearRetryCount($task1); + $this->assertEquals(0, $timetable->getRetryCount($task1)); + $this->assertEquals(2, $timetable->getRetryCount($task2)); + $timetable->clearRetryCount($task2); + $this->assertEquals(0, $timetable->getRetryCount($task1)); + + } + + public function testTaskIsRetriedIfRetryableExcetionIsThrown() + { + + // Mock timetable + $now = time() - 60; + $taskName = 'Piwik\Tests\Integration\RetryScheduledTaskTest.exceptionalTask'; + $timetableData = serialize([$taskName => $now]); + self::getReflectedPiwikOptionInstance()->setValue(new PiwikOption($timetableData)); + + // Create task + $dailySchedule = $this->createPartialMock('Piwik\Scheduler\Schedule\Daily', array('getTime')); + $dailySchedule->expects($this->any()) + ->method('getTime') + ->will($this->returnValue($now)); + + // Setup scheduler + $tasks = [new Task($this, 'exceptionalTask', null, $dailySchedule)]; + $taskLoader = $this->createMock('Piwik\Scheduler\TaskLoader'); + $taskLoader->expects($this->atLeastOnce()) + ->method('loadTasks') + ->willReturn($tasks); + + $scheduler = new Scheduler($taskLoader, new NullLogger()); + + // First run + $scheduler->run(); + $nextRun = $scheduler->getScheduledTimeForMethod('Piwik\Tests\Integration\RetryScheduledTaskTest', 'exceptionalTask', null); + + // Should be rescheduled one hour from now + $this->assertEquals($now+3660, $nextRun); + + self::getReflectedPiwikOptionInstance()->setValue(null); + + } + + public function testTaskIsNotRetriedIfNormalExcetionIsThrown() + { + // Mock timetable + $now = time() - 60; + $taskName = 'Piwik\Tests\Integration\RetryScheduledTaskTest.normalExceptionTask'; + $timetableData = serialize([$taskName => $now]); + self::getReflectedPiwikOptionInstance()->setValue(new PiwikOption($timetableData)); + + // Create task + $specificSchedule = $this->createPartialMock('Piwik\Scheduler\Schedule\SpecificTime', array('getTime')); + $specificSchedule->setScheduledTime($now+50000); + $specificSchedule->expects($this->any()) + ->method('getTime') + ->will($this->returnValue($now)); + + // Setup scheduler + $tasks = [new Task($this, 'normalExceptionTask', null, $specificSchedule)]; + $taskLoader = $this->createMock('Piwik\Scheduler\TaskLoader'); + $taskLoader->expects($this->atLeastOnce()) + ->method('loadTasks') + ->willReturn($tasks); + + $scheduler = new Scheduler($taskLoader, new NullLogger()); + + // First run + $scheduler->run(); + $nextRun = $scheduler->getScheduledTimeForMethod('Piwik\Tests\Integration\RetryScheduledTaskTest', 'normalExceptionTask', null); + + // Should not have scheduled for retry + $this->assertEquals($now+50000, $nextRun); + + self::getReflectedPiwikOptionInstance()->setValue(null); + } + + private static function getReflectedPiwikOptionInstance() + { + $piwikOptionInstance = new ReflectionProperty('Piwik\Option', 'instance'); + $piwikOptionInstance->setAccessible(true); + return $piwikOptionInstance; + } + + public function exceptionalTask() + { + throw new RetryableException('This task fails and should be retried'); + } + + public function normalExceptionTask() + { + throw new \Exception('This task fails and should not be retried'); + } + +} diff --git a/tests/PHPUnit/System/TrackerTest.php b/tests/PHPUnit/System/TrackerTest.php index a675e045d0..592b65c75c 100644 --- a/tests/PHPUnit/System/TrackerTest.php +++ b/tests/PHPUnit/System/TrackerTest.php @@ -45,6 +45,7 @@ class TrackerTest extends IntegrationTestCase Option::delete(self::TASKS_STARTED_OPTION_NAME); Option::delete(self::TASKS_FINISHED_OPTION_NAME); Option::delete(Timetable::TIMETABLE_OPTION_STRING); + Option::delete(Timetable::RETRY_OPTION_STRING); SettingsPiwik::overwritePiwikUrl(self::$fixture->getRootUrl() . "tests/PHPUnit/proxy"); } @@ -399,7 +400,7 @@ class TrackerTest extends IntegrationTestCase $this->assertCustomTasksWereStarted(); Option::clearCachedOption(self::TASKS_FINISHED_OPTION_NAME); - $this->assertEmpty(Option::get(self::TASKS_FINISHED_OPTION_NAME)); + $this->assertFalse(Option::get(self::TASKS_FINISHED_OPTION_NAME)); } private function assertCustomTasksWereStarted() diff --git a/tests/PHPUnit/Unit/Scheduler/TimetableTest.php b/tests/PHPUnit/Unit/Scheduler/TimetableTest.php index 1518d1f833..2d930b0d06 100644 --- a/tests/PHPUnit/Unit/Scheduler/TimetableTest.php +++ b/tests/PHPUnit/Unit/Scheduler/TimetableTest.php @@ -81,6 +81,21 @@ class TimetableTest extends \PHPUnit\Framework\TestCase $this->assertEquals(Date::factory('tomorrow')->getTimeStamp(), $timetable->getTimetable()[$task->getName()]); } + public function testRescheduleTaskAndRunInOneHour() + { + self::stubPiwikOption(serialize([])); + + $timetable = new Timetable(); + $task = $this->getMockBuilder(Task::class) + ->disableOriginalConstructor() + ->getMock(); + $task->method('getName')->willReturn('taskName'); + + $timetable->rescheduleTaskAndRunInOneHour($task); + + $this->assertEquals(Date::factory('now')->addHour(1)->getTimeStamp(), $timetable->getTimetable()[$task->getName()]); + } + /** * Dataprovider for testTaskHasBeenScheduledOnce */ |