diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f0a2e2b..27b10deb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Latest ## Fixes * [#367](https://github.com/cleverage/process-bundle/issues/367) `ProcessLauncherTask` (`process_launcher`) with `json_buffering`: when the sub-process writes nothing into its JSON buffer (no `end_point`, or an output that is not an array), the task outputs nothing again, as before 5.5, instead of the path of an empty file, and the empty buffer is removed. The buffer of a failed sub-process, and those of the still running sub-processes stopped because of it, are removed too (one file was left in the temporary directory for each launch). Update documentation, add tests. * [#368](https://github.com/cleverage/process-bundle/issues/368) `YamlWriterTask` (`yaml_writer`): the parent directory is created for a local path only (no scheme, or `file://`), not for a stream wrapper (`compress.zlib://`, `s3://`...), which failed, or `php://stdout`, which created a `php:` directory. Update documentation, add tests. +* [#369](https://github.com/cleverage/process-bundle/issues/369) `StopTask` (`stop`): a process that has already failed (e.g. after a task initialization failure, going on being deprecated) stays failed instead of being flagged as stopped: the `Process failed` log is written at the `failed_level` and `ProcessEvent::isProcessStopped()` is `false`, as before #337. Update documentation, add tests. v5.5 ----- diff --git a/docs/reference/tasks/stop_task.md b/docs/reference/tasks/stop_task.md index 1876c178..05c90a1c 100644 --- a/docs/reference/tasks/stop_task.md +++ b/docs/reference/tasks/stop_task.md @@ -13,6 +13,10 @@ file does not exist. Unlike [DieTask](die_task.md), the stop is handled by the p * `cleverage:process:execute` exits with code 0, and the task running a sub-process ([ProcessExecutorTask](process_executor_task.md)) goes on. +A process that has already failed (e.g. a task whose initialization failed, see the +[initializable tasks](../../02-task_types.md)) stays **failed**: the `StopTask` still stops the flow of the +tasks, but it does not turn the failure into a stop. + To stop a process because of an error, let the error fail the process instead (error strategy `stop`). Task reference diff --git a/src/Task/StopTask.php b/src/Task/StopTask.php index 06ed1645..cad6fef0 100644 --- a/src/Task/StopTask.php +++ b/src/Task/StopTask.php @@ -26,7 +26,10 @@ class StopTask implements TaskInterface public function execute(ProcessState $state): void { $state->setStopped(true); - $state->getProcessHistory() - ->setStopped(); + $processHistory = $state->getProcessHistory(); + // A process already failed (e.g. by a task initialization failure) stays failed + if (!$processHistory->isFailed()) { + $processHistory->setStopped(); + } } } diff --git a/tests/Manager/ProcessManagerInitializationTest.php b/tests/Manager/ProcessManagerInitializationTest.php index a4dc6f12..ac920cd4 100644 --- a/tests/Manager/ProcessManagerInitializationTest.php +++ b/tests/Manager/ProcessManagerInitializationTest.php @@ -28,6 +28,7 @@ use CleverAge\ProcessBundle\Model\TaskInterface; use CleverAge\ProcessBundle\Registry\ProcessConfigurationRegistry; use CleverAge\ProcessBundle\Registry\TaskRegistry; +use CleverAge\ProcessBundle\Task\StopTask; use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\Component\DependencyInjection\Container; @@ -48,11 +49,15 @@ #[\PHPUnit\Framework\Attributes\UsesClass(ProcessHistory::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessState::class)] #[\PHPUnit\Framework\Attributes\UsesClass(ProcessConfigurationRegistry::class)] +#[\PHPUnit\Framework\Attributes\UsesClass(StopTask::class)] class ProcessManagerInitializationTest extends TestCase { /** @var list */ private array $executedTasks = []; + /** @var list */ + private array $endedEvents = []; + /** @var list */ private array $deprecations = []; @@ -92,6 +97,18 @@ public function testNoDeprecationWithoutInitializationFailure(): void self::assertSame([], $this->deprecations); } + public function testStopTaskDoesNotTurnAFailedProcessIntoAStoppedOne(): void + { + $processManager = $this->createProcessManager(skipBeforeBadTask: false, withStopTask: true); + + $this->execute($processManager); + + self::assertSame(['entry'], $this->executedTasks, 'The stop signal still prevents the next tasks from running'); + self::assertSame([$this->getExpectedDeprecation()], $this->deprecations); + self::assertCount(1, $this->endedEvents); + self::assertFalse($this->endedEvents[0]->isProcessStopped(), 'The failed process must not be flagged as stopped'); + } + private function getExpectedDeprecation(): string { return 'Since cleverage/process-bundle 5.1: The initialization of the task "bad" of the process "test.process" has failed with message "Invalid configuration". Going on with the process after an initialization failure is deprecated: in v6.0, the process will fail before executing any task.'; @@ -111,7 +128,7 @@ private function execute(ProcessManager $processManager): void } } - private function createProcessManager(bool $skipBeforeBadTask, bool $failInitialization = true): ProcessManager + private function createProcessManager(bool $skipBeforeBadTask, bool $failInitialization = true, bool $withStopTask = false): ProcessManager { $recorder = function (string $taskCode): void { $this->executedTasks[] = $taskCode; @@ -159,6 +176,7 @@ public function execute(ProcessState $state): void $container = new Container(); $container->set('test.entry', $entryTask); $container->set('test.bad', $badTask); + $container->set('test.stop', new StopTask()); $registry = new ProcessConfigurationRegistry( [ @@ -170,21 +188,30 @@ public function execute(ProcessState $state): void 'help' => '', 'public' => true, 'tasks' => [ - 'entry' => $this->createTaskConfiguration('@test.entry', ['bad']), + 'entry' => $this->createTaskConfiguration('@test.entry', $withStopTask ? ['stop', 'bad'] : ['bad']), 'bad' => $this->createTaskConfiguration('@test.bad'), + ...($withStopTask ? ['stop' => $this->createTaskConfiguration('@test.stop')] : []), ], ], ], 'stop' ); + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_ENDED, function (ProcessEvent $event): void { + $this->endedEvents[] = $event; + }); + $eventDispatcher->addListener(ProcessEvent::EVENT_PROCESS_FAILED, function (ProcessEvent $event): void { + $this->endedEvents[] = $event; + }); + return new ProcessManager( $container, new ProcessLogger(new NullLogger()), new TaskLogger(new NullLogger()), $registry, new ContextualOptionResolver(), - new EventDispatcher(), + $eventDispatcher, taskRegistry: new TaskRegistry($container), ); } diff --git a/tests/Task/StopTaskTest.php b/tests/Task/StopTaskTest.php index c0ac1680..48cac14a 100644 --- a/tests/Task/StopTaskTest.php +++ b/tests/Task/StopTaskTest.php @@ -44,4 +44,17 @@ public function testProcessIsStoppedAndMarkedAsStopped(): void self::assertNull($state->getOutput()); self::assertNull($state->getException()); } + + public function testFailedProcessStaysFailedButIsStopped(): void + { + $processConfiguration = new ProcessConfiguration('test', []); + $state = new ProcessState($processConfiguration, new ProcessHistory($processConfiguration)); + $state->getProcessHistory()->setFailed(); + + (new StopTask())->execute($state); + + self::assertTrue($state->isStopped()); + self::assertTrue($state->getProcessHistory()->isFailed()); + self::assertFalse($state->getProcessHistory()->isStopped()); + } }