#369 - StopTask: keep a failed process failed

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 <code> failed` log is written at the `failed_level` and `ProcessEvent::isProcessStopped()` is `false`, as before #337. Update documentation, add tests.
 
 v5.5
 -----

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

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();
+        }
     }
 }

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<string> */
     private array $executedTasks = [];
 
+    /** @var list<ProcessEvent> */
+    private array $endedEvents = [];
+
     /** @var list<string> */
     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),
         );
     }

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());
+    }
 }