Skip to content

Commit 4f5a641

Browse files
coisacoisa
committed
wip: Refactor ReportsCommand
1 parent 8930916 commit 4f5a641

5 files changed

Lines changed: 213 additions & 48 deletions

File tree

‎src/Console/Command/ReportsCommand.php‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818

1919
namespace FastForward\DevTools\Console\Command;
2020

21+
use Composer\Command\BaseCommand;
22+
use Composer\Console\Input\InputOption;
23+
use FastForward\DevTools\Process\ProcessBuilderInterface;
24+
use FastForward\DevTools\Process\ProcessQueueInterface;
2125
use Symfony\Component\Console\Attribute\AsCommand;
2226
use Symfony\Component\Console\Input\InputInterface;
2327
use Symfony\Component\Console\Output\OutputInterface;
@@ -31,8 +35,39 @@
3135
description: 'Generates the frontpage for Fast Forward documentation.',
3236
help: 'This command generates the frontpage for Fast Forward documentation, including links to API documentation and test reports.'
3337
)]
34-
final class ReportsCommand extends AbstractCommand
38+
final class ReportsCommand extends BaseCommand
3539
{
40+
/**
41+
* Initializes the command with required dependencies.
42+
*
43+
* @param ProcessBuilderInterface $processBuilder the builder instance used to construct execution processes
44+
* @param ProcessQueueInterface $processQueue the execution queue mechanism for running sub-processes
45+
*/
46+
public function __construct(
47+
private readonly ProcessBuilderInterface $processBuilder,
48+
private readonly ProcessQueueInterface $processQueue,
49+
) {
50+
parent::__construct();
51+
}
52+
53+
public function configure(): void
54+
{
55+
$this
56+
->addOption(
57+
name: 'target',
58+
mode: InputOption::VALUE_OPTIONAL,
59+
description: 'The target directory for the generated reports.',
60+
default: 'public',
61+
)
62+
->addOption(
63+
name: 'coverage',
64+
shortcut: 'c',
65+
mode: InputOption::VALUE_OPTIONAL,
66+
description: 'The target directory for the generated test coverage report.',
67+
default: 'public/coverage',
68+
);
69+
}
70+
3671
/**
3772
* Executes the generation logic for diverse reports.
3873
*
@@ -48,17 +83,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
4883
{
4984
$output->writeln('<info>Generating frontpage for Fast Forward documentation...</info>');
5085

51-
$docsPath = $this->getAbsolutePath('public');
52-
$coveragePath = $this->getAbsolutePath('public/coverage');
53-
54-
$results = [];
86+
$docs = $this->processBuilder
87+
->withArgument('--ansi')
88+
->withArgument('--target', $input->getOption('target'))
89+
->build('composer dev-tools docs');
5590

56-
$output->writeln('<info>Generating API documentation on path: ' . $docsPath . '</info>');
57-
$results[] = $this->runCommand('docs --target=' . $docsPath, $output);
91+
$coverage = $this->processBuilder
92+
->withArgument('--ansi')
93+
->withArgument('--coverage', $input->getOption('coverage'))
94+
->build('composer dev-tools tests');
5895

59-
$output->writeln('<info>Generating test coverage report on path: ' . $coveragePath . '</info>');
60-
$results[] = $this->runCommand('tests --coverage=' . $coveragePath, $output);
96+
$this->processQueue->add(process: $docs, detached: true);
97+
$this->processQueue->add(process: $coverage, detached: true);
6198

62-
return \in_array(self::FAILURE, $results, true) ? self::FAILURE : self::SUCCESS;
99+
return $this->processQueue->run($output);
63100
}
64101
}

‎src/Process/ProcessQueue.php‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,16 @@
3535
* added but do not block subsequent entries.
3636
*
3737
* A detached process that starts successfully is considered dispatched. Because
38-
* this implementation does not wait for detached processes to finish, their
39-
* eventual runtime exit status cannot be incorporated into the final queue
38+
* this implementation does not wait for detached processes to finish during `run()`,
39+
* their eventual runtime exit status cannot be incorporated into the final queue
4040
* result. However, a detached process that cannot be started at all is treated
4141
* as a startup failure and MAY affect the final status code unless its failure
4242
* is explicitly configured to be ignored.
43+
*
44+
* To ensure detached processes finish gracefully without being killed when the
45+
* main PHP script ends, the queue automatically registers a shutdown handler
46+
* during instantiation that implicitly awaits all detached processes. They can
47+
* also be awaited explicitly via `wait()`.
4348
*/
4449
final class ProcessQueue implements ProcessQueueInterface
4550
{
@@ -58,6 +63,16 @@ final class ProcessQueue implements ProcessQueueInterface
5863
*/
5964
private array $runningDetachedProcesses = [];
6065

66+
/**
67+
* Initializes the queue and secures child processes from early termination.
68+
*/
69+
public function __construct()
70+
{
71+
\register_shutdown_function(function (): void {
72+
$this->wait();
73+
});
74+
}
75+
6176
/**
6277
* Adds a process to the queue.
6378
*
@@ -86,8 +101,8 @@ public function add(Process $process, bool $ignoreFailure = false, bool $detache
86101
* The returned status code represents the first non-zero exit code observed
87102
* among non-ignored blocking processes, or among non-ignored detached
88103
* processes that fail to start. Detached processes that start successfully
89-
* are not awaited and therefore do not contribute their eventual runtime
90-
* exit code to the returned result.
104+
* are not awaited iteratively inside run() and therefore do not contribute
105+
* their eventual runtime exit code to the returned result.
91106
*
92107
* @param OutputInterface $output the output used during execution
93108
*
@@ -135,6 +150,23 @@ public function run(?OutputInterface $output = new NullOutput()): int
135150
return $statusCode;
136151
}
137152

153+
/**
154+
* Waits for all detached processes to finish execution.
155+
*
156+
* @param ?OutputInterface $output the output interface to which process output and diagnostics MAY be written
157+
*/
158+
public function wait(?OutputInterface $output = new NullOutput()): void
159+
{
160+
$output = $output ?? new NullOutput();
161+
162+
while ([] !== $this->runningDetachedProcesses) {
163+
$this->drainDetachedProcessesOutput($output, true);
164+
if ([] !== $this->runningDetachedProcesses) {
165+
\usleep(10000);
166+
}
167+
}
168+
}
169+
138170
/**
139171
* Starts a process in detached mode without waiting for completion.
140172
*

‎src/Process/ProcessQueueInterface.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,4 +98,16 @@ public function add(Process $process, bool $ignoreFailure = false, bool $detache
9898
* @return int the final exit status code produced by the queue execution
9999
*/
100100
public function run(?OutputInterface $output = null): int;
101+
102+
/**
103+
* Waits for all detached processes to finish execution.
104+
*
105+
* Implementations MUST block the execution thread until all previously
106+
* started detached processes complete. This ensures the main process
107+
* does not exit prematurely, preventing detached children from being
108+
* abruptly terminated.
109+
*
110+
* @param ?OutputInterface $output the output interface to which process output and diagnostics MAY be written
111+
*/
112+
public function wait(?OutputInterface $output = null): void;
101113
}

‎tests/Console/Command/ReportsCommandTest.php‎

Lines changed: 96 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,61 +19,124 @@
1919
namespace FastForward\DevTools\Tests\Console\Command;
2020

2121
use FastForward\DevTools\Console\Command\ReportsCommand;
22+
use FastForward\DevTools\Process\ProcessBuilderInterface;
23+
use FastForward\DevTools\Process\ProcessQueueInterface;
2224
use PHPUnit\Framework\Attributes\CoversClass;
2325
use PHPUnit\Framework\Attributes\Test;
26+
use PHPUnit\Framework\TestCase;
2427
use Prophecy\Argument;
2528
use Prophecy\PhpUnit\ProphecyTrait;
29+
use Prophecy\Prophecy\ObjectProphecy;
30+
use ReflectionMethod;
31+
use Symfony\Component\Console\Input\InputInterface;
32+
use Symfony\Component\Console\Output\OutputInterface;
33+
use Symfony\Component\Process\Process;
2634

2735
#[CoversClass(ReportsCommand::class)]
28-
final class ReportsCommandTest extends AbstractCommandTestCase
36+
final class ReportsCommandTest extends TestCase
2937
{
3038
use ProphecyTrait;
3139

32-
/**
33-
* @return string
34-
*/
35-
protected function getCommandClass(): string
36-
{
37-
return ReportsCommand::class;
38-
}
40+
/** @var ObjectProphecy<ProcessBuilderInterface> */
41+
private ObjectProphecy $processBuilder;
42+
43+
/** @var ObjectProphecy<ProcessQueueInterface> */
44+
private ObjectProphecy $processQueue;
45+
46+
/** @var ObjectProphecy<InputInterface> */
47+
private ObjectProphecy $input;
48+
49+
/** @var ObjectProphecy<OutputInterface> */
50+
private ObjectProphecy $output;
51+
52+
/** @var ObjectProphecy<Process> */
53+
private ObjectProphecy $docsProcess;
54+
55+
/** @var ObjectProphecy<Process> */
56+
private ObjectProphecy $testsProcess;
3957

40-
/**
41-
* @return string
42-
*/
43-
protected function getCommandName(): string
58+
private ReportsCommand $command;
59+
60+
protected function setUp(): void
4461
{
45-
return 'reports';
62+
$this->processBuilder = $this->prophesize(ProcessBuilderInterface::class);
63+
$this->processQueue = $this->prophesize(ProcessQueueInterface::class);
64+
$this->input = $this->prophesize(InputInterface::class);
65+
$this->output = $this->prophesize(OutputInterface::class);
66+
$this->docsProcess = $this->prophesize(Process::class);
67+
$this->testsProcess = $this->prophesize(Process::class);
68+
69+
$this->input->getOption('target')->willReturn('public');
70+
$this->input->getOption('coverage')->willReturn('public/coverage');
71+
72+
$this->processBuilder->withArgument(Argument::cetera())
73+
->willReturn($this->processBuilder->reveal());
74+
75+
$this->processBuilder->build('composer dev-tools docs')
76+
->willReturn($this->docsProcess->reveal());
77+
78+
$this->processBuilder->build('composer dev-tools tests')
79+
->willReturn($this->testsProcess->reveal());
80+
81+
$this->processQueue->run($this->output->reveal())
82+
->willReturn(ReportsCommand::SUCCESS);
83+
84+
$this->command = new ReportsCommand(
85+
$this->processBuilder->reveal(),
86+
$this->processQueue->reveal()
87+
);
4688
}
4789

48-
/**
49-
* @return string
50-
*/
51-
protected function getCommandDescription(): string
90+
#[Test]
91+
public function commandWillSetExpectedNameDescriptionAndHelp(): void
5292
{
53-
return 'Generates the frontpage for Fast Forward documentation.';
93+
self::assertSame('reports', $this->command->getName());
94+
self::assertSame('Generates the frontpage for Fast Forward documentation.', $this->command->getDescription());
95+
self::assertSame('This command generates the frontpage for Fast Forward documentation, including links to API documentation and test reports.', $this->command->getHelp());
5496
}
5597

56-
/**
57-
* @return string
58-
*/
59-
protected function getCommandHelp(): string
98+
#[Test]
99+
public function commandWillHaveExpectedOptions(): void
60100
{
61-
return 'This command generates the frontpage for Fast Forward documentation, including links to API documentation and test reports.';
101+
$definition = $this->command->getDefinition();
102+
103+
self::assertTrue($definition->hasOption('target'));
104+
self::assertTrue($definition->hasOption('coverage'));
62105
}
63106

64-
/**
65-
* @return void
66-
*/
67107
#[Test]
68-
public function executeWillRunDocsAndTestsCommand(): void
108+
public function executeWillRunDocsAndTestsCommandAsDetachedProcesses(): void
69109
{
70110
$this->output->writeln('<info>Generating frontpage for Fast Forward documentation...</info>')
71-
->shouldBeCalled();
72-
$this->output->writeln(Argument::containingString('Generating API documentation on path:'))
73-
->shouldBeCalled();
74-
$this->output->writeln(Argument::containingString('Generating test coverage report on path:'))
75-
->shouldBeCalled();
111+
->shouldBeCalledOnce();
112+
113+
$this->processBuilder->withArgument('--ansi')
114+
->shouldBeCalled()
115+
->willReturn($this->processBuilder->reveal());
116+
117+
$this->processBuilder->withArgument('--target', 'public')
118+
->shouldBeCalledOnce()
119+
->willReturn($this->processBuilder->reveal());
120+
121+
$this->processBuilder->withArgument('--coverage', 'public/coverage')
122+
->shouldBeCalledOnce()
123+
->willReturn($this->processBuilder->reveal());
124+
125+
$this->processQueue->add($this->docsProcess->reveal(), false, true)
126+
->shouldBeCalledOnce();
127+
128+
$this->processQueue->add($this->testsProcess->reveal(), false, true)
129+
->shouldBeCalledOnce();
130+
131+
$result = $this->executeCommand();
132+
133+
self::assertSame(ReportsCommand::SUCCESS, $result);
134+
}
135+
136+
private function executeCommand(): int
137+
{
138+
$reflectionMethod = new ReflectionMethod($this->command, 'execute');
76139

77-
self::assertSame(ReportsCommand::SUCCESS, $this->invokeExecute());
140+
return $reflectionMethod->invoke($this->command, $this->input->reveal(), $this->output->reveal());
78141
}
79142
}

‎tests/Process/ProcessQueueTest.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,8 @@ public function runWithFailingProcessAndIgnoreFailureReturnsSuccess(): void
203203
#[Test]
204204
public function runDetachedProcessStartsWithoutBlocking(): void
205205
{
206-
$detachedProcess = $this->createDetachedProcessMock(isRunning: true);
206+
$detachedProcess = $this->createDetachedProcessMock();
207+
$detachedProcess->isRunning()->willReturn(true, false);
207208
$blockingProcess = $this->createProcessMock();
208209

209210
$this->queue->add($detachedProcess->reveal(), false, true);
@@ -351,4 +352,24 @@ public function runWillWriteProcessOutputToOutputInterface(): void
351352

352353
$this->queue->run($this->output->reveal());
353354
}
355+
356+
/**
357+
* @return void
358+
*/
359+
#[Test]
360+
public function waitWillBlockUntilDetachedProcessesFinish(): void
361+
{
362+
$detachedProcess = $this->createDetachedProcessMock();
363+
$detachedProcess->isRunning()->willReturn(true, false);
364+
365+
$this->queue->add($detachedProcess->reveal(), false, true);
366+
$this->queue->run($this->output->reveal());
367+
368+
// Call wait explicitly. It should retrieve the process from tracking
369+
// and loop exactly once before it exits because isRunning returns false.
370+
$this->queue->wait($this->output->reveal());
371+
372+
// The assertion simply verifies the test completes and doesn't run infinitely.
373+
self::assertTrue(true);
374+
}
354375
}

0 commit comments

Comments
 (0)