From 38cf1c0a84c063c35e6c4bde14f819d988ff4689 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 15:19:23 -0500 Subject: [PATCH 01/10] Add scheduler test for callback events --- .../Console/CallbackSchedulingTest.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/Integration/Console/CallbackSchedulingTest.php diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php new file mode 100644 index 000000000000..a16e9a5f76fb --- /dev/null +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -0,0 +1,56 @@ +app->make(Schedule::class) + ->call($this->logger('call')) + ->after($this->logger('after 1')) + ->before($this->logger('before 1')) + ->after($this->logger('after 2')) + ->before($this->logger('before 2')); + + if ($background) { + $event->runInBackground(); + } + + $this->artisan('schedule:run'); + + $this->assertLogged('before 1', 'before 2', 'call', 'after 1', 'after 2'); + } + + public function executionProvider() + { + return [ + 'Foreground' => [false], + 'Background' => [true], + ]; + } + + protected function logger($message) + { + return function () use ($message) { + $this->log[] = $message; + }; + } + + protected function assertLogged(...$message) + { + $this->assertEquals($message, $this->log); + } +} From ce39e31bfda148b2bff018460b7c366891118e6c Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 15:36:05 -0500 Subject: [PATCH 02/10] Add exception handling test --- .../Console/CallbackSchedulingTest.php | 52 +++++++++++++++++-- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index a16e9a5f76fb..327662008181 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -2,12 +2,11 @@ namespace Illuminate\Tests\Integration\Console; -use Illuminate\Bus\Queueable; +use Illuminate\Console\Events\ScheduledTaskFailed; use Illuminate\Console\Scheduling\Schedule; -use Illuminate\Contracts\Queue\ShouldQueue; -use Illuminate\Queue\InteractsWithQueue; -use Illuminate\Support\Facades\Queue; +use Illuminate\Contracts\Events\Dispatcher; use Orchestra\Testbench\TestCase; +use RuntimeException; class CallbackSchedulingTest extends TestCase { @@ -34,6 +33,51 @@ public function testExecutionOrder($background) $this->assertLogged('before 1', 'before 2', 'call', 'after 1', 'after 2'); } + public function testExceptionHandlingInCallback() + { + $event = $this->app->make(Schedule::class) + ->call($this->logger('call')) + ->name('test-event') + ->withoutOverlapping(); + + // Set up "before" and "after" hooks to ensure they're called + $event->before($this->logger('before'))->after($this->logger('after')); + + // Register a hook to validate that the mutex was initially created + $mutexWasCreated = false; + $event->before(function () use (&$mutexWasCreated, $event) { + $mutexWasCreated = $event->mutex->exists($event); + }); + + // We'll trigger an exception in an "after" hook to test exception handling + $event->after(function () { + throw new RuntimeException; + }); + + // Because exceptions are caught by the ScheduleRunCommand, we need to listen for + // the "failed" event to check whether our exception was actually thrown + $failed = false; + $this->app->make(Dispatcher::class) + ->listen(ScheduledTaskFailed::class, function (ScheduledTaskFailed $failure) use (&$failed, $event) { + if ($failure->task === $event) { + $failed = true; + } + }); + + $this->artisan('schedule:run'); + + // Hooks and execution should happn in correct order + $this->assertLogged('before', 'call', 'after'); + + // Our exception should have resulted in a failure event + $this->assertTrue($failed); + + // Validate that the mutex was originally created, but that it's since + // been removed (even though an exception was thrown) + $this->assertTrue($mutexWasCreated); + $this->assertFalse($event->mutex->exists($event)); + } + public function executionProvider() { return [ From 4e460f8a849a12f815eb13e771f0fbd1b79a50dc Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 20:27:35 -0500 Subject: [PATCH 03/10] Added integration test for command scheduler --- .../Console/CommandSchedulingTest.php | 225 ++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 tests/Integration/Console/CommandSchedulingTest.php diff --git a/tests/Integration/Console/CommandSchedulingTest.php b/tests/Integration/Console/CommandSchedulingTest.php new file mode 100644 index 000000000000..f2f5546f30d8 --- /dev/null +++ b/tests/Integration/Console/CommandSchedulingTest.php @@ -0,0 +1,225 @@ +fs = new Filesystem; + + $this->id = Str::random(); + $this->logFile = storage_path("logs/command_scheduling_test_{$this->id}.log"); + $this->commandFile = app_path("Console/Commands/CommandSchedulingTestCommand_{$this->id}.php"); + + // We always need to clean this up in case of an Exception in a previous run, + // since Test Bench will automatically include it when we execute a command. + $this->fs->delete($this->app->basePath('routes/console.php')); + + $this->writeTestStubs(); + } + + protected function tearDown(): void + { + $this->fs->delete($this->logFile); + $this->fs->delete(base_path('artisan')); + $this->fs->delete(base_path('routes/console.php')); + $this->fs->delete($this->commandFile); + + parent::tearDown(); + } + + /** + * @dataProvider executionProvider + */ + public function testExecutionOrder($background) + { + $event = $this->app->make(Schedule::class) + ->command("test:{$this->id}") + ->onOneServer() + ->after(function () { + $this->fs->append($this->logFile, "after\n"); + }) + ->before(function () { + $this->fs->append($this->logFile, "before\n"); + }); + + if ($background) { + $event->runInBackground(); + } + + // We'll trigger the scheduler three times to simulate multiple servers + $this->artisan('schedule:run'); + $this->artisan('schedule:run'); + $this->artisan('schedule:run'); + + if ($background) { + // Since our command is running in a separate process, we need to wait + // until it has finished executing before running our assertions. + $this->waitForLogMessages('before', 'handled', 'after'); + } + + $this->assertLogged('before', 'handled', 'after'); + } + + public function executionProvider() + { + return [ + 'Foreground' => [false], + 'Background' => [true], + ]; + } + + protected function waitForLogMessages(...$messages) + { + $tries = 0; + $sleep = 100000; // 100K microseconds = 0.1 second + $limit = 50; // 0.1s * 50 = 5 second wait limit + + do { + $log = $this->fs->get($this->logFile); + + if (Str::containsAll($log, $messages)) { + return; + } + + $tries++; + usleep($sleep); + } while ($tries < $limit); + } + + protected function assertLogged(...$messages) + { + $log = trim($this->fs->get($this->logFile)); + + $this->assertEquals(implode("\n", $messages), $log); + } + + protected function writeTestStubs() + { + // Make sure our target directories exist + $this->fs->ensureDirectoryExists(app_path('Console/Commands')); + $this->fs->ensureDirectoryExists(base_path('routes')); + + // Get PHP-ready representation of our paths + $logFile = var_export($this->logFile, true); + $commandFile = var_export($this->commandFile, true); + + // Testbench doesn't ship with an artisan script, so we need to add one to the project + // so that we can execute commands in a separate process + $artisan = <<make(Illuminate\Contracts\Console\Kernel::class); + +\$status = \$kernel->handle( + \$input = new Symfony\Component\Console\Input\ArgvInput, + new Symfony\Component\Console\Output\ConsoleOutput +); + +\$kernel->terminate(\$input, \$status); + +exit(\$status); + +PHP; + + // Each time we run the test we need to write to a unique log file to ensure that separate + // test runs don't pollute the results of each other + $command = <<id} extends Command +{ + protected \$signature = 'test:{$this->id}'; + + public function handle() + { + \$logfile = {$logFile}; + (new Filesystem)->append(\$logfile, "handled\\n"); + } +} +PHP; + + // Testbench automatically loads `routes/console.php` on initialization of the console + // Kernel, so this is a convenient place to register out command and set up the scheduler + $route = <<add(new App\Console\Commands\CommandSchedulingTestCommand_{$this->id}); +}); + +// Add command to scheduler so that the after() callback is trigger in our spawned process +Illuminate\Foundation\Application::getInstance() + ->booted(function (\$app) { + \$app->resolving(Illuminate\Console\Scheduling\Schedule::class, function(\$schedule) { + \$fs = new Illuminate\Filesystem\Filesystem; + \$schedule->command("test:{$this->id}") + ->after(function() use (\$fs) { + \$logfile = {$logFile}; + \$fs->append(\$logfile, "after\\n"); + }) + ->before(function() use (\$fs) { + \$logfile = {$logFile}; + \$fs->append(\$logfile, "before\\n"); + }); + }); + }); + +PHP; + + $this->fs->put($this->commandFile, $command); + $this->fs->put(base_path('routes/console.php'), $route); + $this->fs->put(base_path('artisan'), $artisan); + } +} From 575fc8cfc3f457261efc06d918b23fcf34bdede3 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 20:51:38 -0500 Subject: [PATCH 04/10] Move everything to our `artisan` script --- .../Console/CommandSchedulingTest.php | 102 ++++++------------ 1 file changed, 34 insertions(+), 68 deletions(-) diff --git a/tests/Integration/Console/CommandSchedulingTest.php b/tests/Integration/Console/CommandSchedulingTest.php index f2f5546f30d8..95cba0012d49 100644 --- a/tests/Integration/Console/CommandSchedulingTest.php +++ b/tests/Integration/Console/CommandSchedulingTest.php @@ -22,14 +22,7 @@ class CommandSchedulingTest extends TestCase * * @var string */ - protected $logFile; - - /** - * The path to the custom command that will be written for this test. - * - * @var string - */ - protected $commandFile; + protected $logfile; /** * The Filesystem instance for writing stubs and logs. @@ -45,22 +38,15 @@ protected function setUp(): void $this->fs = new Filesystem; $this->id = Str::random(); - $this->logFile = storage_path("logs/command_scheduling_test_{$this->id}.log"); - $this->commandFile = app_path("Console/Commands/CommandSchedulingTestCommand_{$this->id}.php"); + $this->logfile = storage_path("logs/command_scheduling_test_{$this->id}.log"); - // We always need to clean this up in case of an Exception in a previous run, - // since Test Bench will automatically include it when we execute a command. - $this->fs->delete($this->app->basePath('routes/console.php')); - - $this->writeTestStubs(); + $this->writeArtisanScript(); } protected function tearDown(): void { - $this->fs->delete($this->logFile); + $this->fs->delete($this->logfile); $this->fs->delete(base_path('artisan')); - $this->fs->delete(base_path('routes/console.php')); - $this->fs->delete($this->commandFile); parent::tearDown(); } @@ -74,10 +60,10 @@ public function testExecutionOrder($background) ->command("test:{$this->id}") ->onOneServer() ->after(function () { - $this->fs->append($this->logFile, "after\n"); + $this->fs->append($this->logfile, "after\n"); }) ->before(function () { - $this->fs->append($this->logFile, "before\n"); + $this->fs->append($this->logfile, "before\n"); }); if ($background) { @@ -113,7 +99,7 @@ protected function waitForLogMessages(...$messages) $limit = 50; // 0.1s * 50 = 5 second wait limit do { - $log = $this->fs->get($this->logFile); + $log = $this->fs->get($this->logfile); if (Str::containsAll($log, $messages)) { return; @@ -126,27 +112,28 @@ protected function waitForLogMessages(...$messages) protected function assertLogged(...$messages) { - $log = trim($this->fs->get($this->logFile)); + $log = trim($this->fs->get($this->logfile)); $this->assertEquals(implode("\n", $messages), $log); } - protected function writeTestStubs() + protected function writeArtisanScript() { - // Make sure our target directories exist - $this->fs->ensureDirectoryExists(app_path('Console/Commands')); - $this->fs->ensureDirectoryExists(base_path('routes')); - - // Get PHP-ready representation of our paths - $logFile = var_export($this->logFile, true); - $commandFile = var_export($this->commandFile, true); + $thisFile = __FILE__; + $logFile = var_export($this->logfile, true); - // Testbench doesn't ship with an artisan script, so we need to add one to the project - // so that we can execute commands in a separate process - $artisan = <<make(Illuminate\Contracts\Console\Kernel::class); -\$status = \$kernel->handle( - \$input = new Symfony\Component\Console\Input\ArgvInput, - new Symfony\Component\Console\Output\ConsoleOutput -); - -\$kernel->terminate(\$input, \$status); - -exit(\$status); - -PHP; - - // Each time we run the test we need to write to a unique log file to ensure that separate - // test runs don't pollute the results of each other - $command = <<id} extends Command +// Here is our custom command for the test +class CommandSchedulingTestCommand_{$this->id} extends Illuminate\Console\Command { protected \$signature = 'test:{$this->id}'; public function handle() { \$logfile = {$logFile}; - (new Filesystem)->append(\$logfile, "handled\\n"); + (new Illuminate\Filesystem\Filesystem)->append(\$logfile, "handled\\n"); } } -PHP; - - // Testbench automatically loads `routes/console.php` on initialization of the console - // Kernel, so this is a convenient place to register out command and set up the scheduler - $route = <<add(new App\Console\Commands\CommandSchedulingTestCommand_{$this->id}); + \$artisan->add(new CommandSchedulingTestCommand_{$this->id}); }); // Add command to scheduler so that the after() callback is trigger in our spawned process @@ -216,10 +175,17 @@ public function handle() }); }); +\$status = \$kernel->handle( + \$input = new Symfony\Component\Console\Input\ArgvInput, + new Symfony\Component\Console\Output\ConsoleOutput +); + +\$kernel->terminate(\$input, \$status); + +exit(\$status); + PHP; - $this->fs->put($this->commandFile, $command); - $this->fs->put(base_path('routes/console.php'), $route); - $this->fs->put(base_path('artisan'), $artisan); + $this->fs->put(base_path('artisan'), $script); } } From d5c7d6fff89f2e3751dd64d63a818645132a1ca8 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:05:53 -0500 Subject: [PATCH 05/10] Add a cache implementation --- .../Console/CallbackSchedulingTest.php | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index 327662008181..a084dfded243 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -2,8 +2,15 @@ namespace Illuminate\Tests\Integration\Console; +use Illuminate\Cache\ArrayStore; +use Illuminate\Cache\Repository; use Illuminate\Console\Events\ScheduledTaskFailed; +use Illuminate\Console\Scheduling\CacheEventMutex; +use Illuminate\Console\Scheduling\CacheSchedulingMutex; +use Illuminate\Console\Scheduling\EventMutex; use Illuminate\Console\Scheduling\Schedule; +use Illuminate\Console\Scheduling\SchedulingMutex; +use Illuminate\Contracts\Cache\Factory; use Illuminate\Contracts\Events\Dispatcher; use Orchestra\Testbench\TestCase; use RuntimeException; @@ -12,6 +19,28 @@ class CallbackSchedulingTest extends TestCase { protected $log = []; + protected function setUp(): void + { + parent::setUp(); + + $cache = new class implements Factory { + public $store; + + public function __construct() + { + $this->store = new Repository(new ArrayStore()); + } + + public function store($name = null) + { + return $this->store; + } + }; + + $this->app->instance(EventMutex::class, new CacheEventMutex($cache)); + $this->app->instance(SchedulingMutex::class, new CacheSchedulingMutex($cache)); + } + /** * @dataProvider executionProvider */ From 6a0f9f19d3a8c0f807eb34c6291f02f00548a271 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:11:32 -0500 Subject: [PATCH 06/10] Adjust container call --- tests/Integration/Console/CallbackSchedulingTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index a084dfded243..a346c73f9791 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -10,6 +10,7 @@ use Illuminate\Console\Scheduling\EventMutex; use Illuminate\Console\Scheduling\Schedule; use Illuminate\Console\Scheduling\SchedulingMutex; +use Illuminate\Container\Container; use Illuminate\Contracts\Cache\Factory; use Illuminate\Contracts\Events\Dispatcher; use Orchestra\Testbench\TestCase; @@ -28,7 +29,7 @@ protected function setUp(): void public function __construct() { - $this->store = new Repository(new ArrayStore()); + $this->store = new Repository(new ArrayStore(true)); } public function store($name = null) @@ -37,8 +38,10 @@ public function store($name = null) } }; - $this->app->instance(EventMutex::class, new CacheEventMutex($cache)); - $this->app->instance(SchedulingMutex::class, new CacheSchedulingMutex($cache)); + $container = Container::getInstance(); + + $container->instance(EventMutex::class, new CacheEventMutex($cache)); + $container->instance(SchedulingMutex::class, new CacheSchedulingMutex($cache)); } /** From bd52c4c9792c5573ed59828315706f09cd85ae7f Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:16:37 -0500 Subject: [PATCH 07/10] Restore original artisan command just in case --- .../Console/CommandSchedulingTest.php | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/Integration/Console/CommandSchedulingTest.php b/tests/Integration/Console/CommandSchedulingTest.php index 95cba0012d49..d29d2ae14500 100644 --- a/tests/Integration/Console/CommandSchedulingTest.php +++ b/tests/Integration/Console/CommandSchedulingTest.php @@ -24,6 +24,13 @@ class CommandSchedulingTest extends TestCase */ protected $logfile; + /** + * Just in case Testbench starts to ship an `artisan` script, we'll check and save a backup. + * + * @var string|null + */ + protected $originalArtisan; + /** * The Filesystem instance for writing stubs and logs. * @@ -48,6 +55,10 @@ protected function tearDown(): void $this->fs->delete($this->logfile); $this->fs->delete(base_path('artisan')); + if (! is_null($this->originalArtisan)) { + $this->fs->put(base_path('artisan'), $this->originalArtisan); + } + parent::tearDown(); } @@ -119,8 +130,15 @@ protected function assertLogged(...$messages) protected function writeArtisanScript() { + $path = base_path('artisan'); + + // Save existing artisan script if there is one + if ($this->fs->exists($path)) { + $this->originalArtisan = $this->fs->get($path); + } + $thisFile = __FILE__; - $logFile = var_export($this->logfile, true); + $logfile = var_export($this->logfile, true); $script = <<id} extends Illuminate\Console\Comman public function handle() { - \$logfile = {$logFile}; + \$logfile = {$logfile}; (new Illuminate\Filesystem\Filesystem)->append(\$logfile, "handled\\n"); } } @@ -165,11 +183,11 @@ public function handle() \$fs = new Illuminate\Filesystem\Filesystem; \$schedule->command("test:{$this->id}") ->after(function() use (\$fs) { - \$logfile = {$logFile}; + \$logfile = {$logfile}; \$fs->append(\$logfile, "after\\n"); }) ->before(function() use (\$fs) { - \$logfile = {$logFile}; + \$logfile = {$logfile}; \$fs->append(\$logfile, "before\\n"); }); }); @@ -186,6 +204,6 @@ public function handle() PHP; - $this->fs->put(base_path('artisan'), $script); + $this->fs->put($path, $script); } } From 24109b5bbd28194b4bb8ad916f7a1f9feeefc5a7 Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:17:09 -0500 Subject: [PATCH 08/10] Code style --- tests/Integration/Console/CallbackSchedulingTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index a346c73f9791..30f5b085ecd9 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -24,7 +24,8 @@ protected function setUp(): void { parent::setUp(); - $cache = new class implements Factory { + $cache = new class implements Factory + { public $store; public function __construct() From 3b869b859592de019841e6d7cb3f1ad0781d68ba Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:24:58 -0500 Subject: [PATCH 09/10] Clear container during teardown --- tests/Integration/Console/CallbackSchedulingTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index 30f5b085ecd9..c8233e2b5476 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -45,6 +45,13 @@ public function store($name = null) $container->instance(SchedulingMutex::class, new CacheSchedulingMutex($cache)); } + protected function tearDown(): void + { + Container::setInstance(null); + + parent::tearDown(); + } + /** * @dataProvider executionProvider */ From 49cd6c668990f036582ad9653de9c328419152ac Mon Sep 17 00:00:00 2001 From: Chris Morrell Date: Wed, 1 Dec 2021 21:25:56 -0500 Subject: [PATCH 10/10] Formatting --- tests/Integration/Console/CallbackSchedulingTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Console/CallbackSchedulingTest.php b/tests/Integration/Console/CallbackSchedulingTest.php index c8233e2b5476..9ad046b1931b 100644 --- a/tests/Integration/Console/CallbackSchedulingTest.php +++ b/tests/Integration/Console/CallbackSchedulingTest.php @@ -48,7 +48,7 @@ public function store($name = null) protected function tearDown(): void { Container::setInstance(null); - + parent::tearDown(); }