Skip to content

Commit d1a8c60

Browse files
pyrechpyrech
committed
Apply some more review suggestions
1 parent fc5eeff commit d1a8c60

File tree

3 files changed

+23
-54
lines changed

3 files changed

+23
-54
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/console.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,6 @@
389389

390390
->set('console.command.error_dumper', ErrorDumpCommand::class)
391391
->args([
392-
param('kernel.build_dir').'/error_page',
393392
service('filesystem'),
394393
service('error_renderer.html'),
395394
service(EntrypointLookupInterface::class)->nullOnInvalid(),

‎src/Symfony/Component/ErrorHandler/Command/ErrorDumpCommand.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
final class ErrorDumpCommand extends Command
3737
{
3838
public function __construct(
39-
private readonly string $defaultPath,
4039
private readonly Filesystem $filesystem,
4140
private readonly ErrorRendererInterface $errorRenderer,
4241
private readonly ?EntrypointLookupInterface $entrypointLookup = null,
@@ -47,32 +46,32 @@ public function __construct(
4746
protected function configure(): void
4847
{
4948
$this
50-
->addArgument('status-codes', InputArgument::IS_ARRAY, 'Status codes to dump error pages for')
51-
->addOption('path', null, InputOption::VALUE_REQUIRED, 'Path where to dump the error pages in', $this->defaultPath)
52-
->addOption('clear', null, InputOption::VALUE_NONE, 'Remove the directory before dumping new error pages')
49+
->addArgument('path', InputArgument::REQUIRED, 'Path where to dump the error pages in')
50+
->addArgument('status-codes', InputArgument::IS_ARRAY, 'Status codes to dump error pages for, all of them by default')
51+
->addOption('force', 'f', InputOption::VALUE_NONE, 'Force directory removal before dumping new error pages')
5352
;
5453
}
5554

5655
protected function execute(InputInterface $input, OutputInterface $output): int
5756
{
58-
$path = $input->getOption('path');
57+
$path = $input->getArgument('path');
5958

6059
$io = new SymfonyStyle($input, $output);
6160
$io->title('Dumping error pages');
6261

63-
$this->dump($io, $path, $input->getArgument('status-codes'), (bool) $input->getOption('clear'));
62+
$this->dump($io, $path, $input->getArgument('status-codes'), (bool) $input->getOption('force'));
6463
$io->success(\sprintf('Error pages have been dumped in "%s".', $path));
6564

6665
return Command::SUCCESS;
6766
}
6867

69-
private function dump(SymfonyStyle $io, string $path, array $statusCodes, bool $clear = false): void
68+
private function dump(SymfonyStyle $io, string $path, array $statusCodes, bool $force = false): void
7069
{
7170
if (!$statusCodes) {
7271
$statusCodes = array_filter(array_keys(Response::$statusTexts), fn ($statusCode) => $statusCode >= 400);
7372
}
7473

75-
if ($clear || $io->confirm(\sprintf('Do you want to remove the directory "%s" before dumping new error pages?', $path))) {
74+
if ($force || ($this->filesystem->exists($path) && $io->confirm(\sprintf('The "%s" directory already exists. Do you want to empty it before dumping the error pages?', $path), false))) {
7675
$this->filesystem->remove($path);
7776
}
7877

‎src/Symfony/Component/ErrorHandler/Tests/Command/ErrorDumpCommandTest.php‎

Lines changed: 16 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -38,55 +38,44 @@ protected function setUp(): void
3838

3939
public function testDumpPages()
4040
{
41-
$fs = new Filesystem();
42-
$fs->mkdir($this->tmpDir);
43-
$fs->touch($this->tmpDir.\DIRECTORY_SEPARATOR.'400.html');
44-
4541
$tester = $this->getCommandTester($this->getKernel(), []);
46-
$tester->execute([]);
42+
$tester->execute([
43+
'path' => $this->tmpDir,
44+
]);
4745

48-
$this->assertFileExists($this->tmpDir.\DIRECTORY_SEPARATOR.'400.html');
4946
$this->assertFileExists($this->tmpDir.\DIRECTORY_SEPARATOR.'404.html');
5047
$this->assertStringContainsString('Error 404', file_get_contents($this->tmpDir.\DIRECTORY_SEPARATOR.'404.html'));
5148
}
5249

5350
public function testDumpPagesOnlyForGivenStatusCodes()
5451
{
52+
$fs = new Filesystem();
53+
$fs->mkdir($this->tmpDir);
54+
$fs->touch($this->tmpDir.\DIRECTORY_SEPARATOR.'test.html');
55+
5556
$tester = $this->getCommandTester($this->getKernel());
5657
$tester->execute([
58+
'path' => $this->tmpDir,
5759
'status-codes' => ['400', '500'],
5860
]);
5961

62+
$this->assertFileExists($this->tmpDir.\DIRECTORY_SEPARATOR.'test.html');
6063
$this->assertFileDoesNotExist($this->tmpDir.\DIRECTORY_SEPARATOR.'404.html');
6164

6265
$this->assertFileExists($this->tmpDir.\DIRECTORY_SEPARATOR.'400.html');
6366
$this->assertStringContainsString('Error 400', file_get_contents($this->tmpDir.\DIRECTORY_SEPARATOR.'400.html'));
6467
}
6568

66-
public function testDumpPagesAtCustomPath()
69+
public function testForceRemovalPages()
6770
{
68-
$path = sys_get_temp_dir().'/error_pages_custom';
69-
7071
$fs = new Filesystem();
71-
$fs->remove($path);
72-
73-
$tester = $this->getCommandTester($this->getKernel());
74-
$tester->execute([
75-
'--path' => $path,
76-
]);
77-
78-
$this->assertFileExists($path.\DIRECTORY_SEPARATOR.'400.html');
79-
$this->assertStringContainsString('Error 400', file_get_contents($path.\DIRECTORY_SEPARATOR.'400.html'));
80-
}
81-
82-
public function testClearPages()
83-
{
84-
mkdir($this->tmpDir);
85-
touch($this->tmpDir.\DIRECTORY_SEPARATOR.'test.html');
72+
$fs->mkdir($this->tmpDir);
73+
$fs->touch($this->tmpDir.\DIRECTORY_SEPARATOR.'test.html');
8674

8775
$tester = $this->getCommandTester($this->getKernel());
8876
$tester->execute([
89-
'--clear' => true,
77+
'path' => $this->tmpDir,
78+
'--force' => true,
9079
]);
9180

9281
$this->assertFileDoesNotExist($this->tmpDir.\DIRECTORY_SEPARATOR.'test.html');
@@ -95,27 +84,14 @@ public function testClearPages()
9584

9685
private function getKernel(): MockObject&KernelInterface
9786
{
98-
$kernel = $this->createMock(KernelInterface::class);
99-
$kernel
100-
->expects($this->any())
101-
->method('getContainer')
102-
->willReturn(new Container());
103-
104-
$kernel
105-
->expects($this->once())
106-
->method('getBundles')
107-
->willReturn([]);
108-
109-
return $kernel;
87+
return $this->createMock(KernelInterface::class);
11088
}
11189

11290
private function getCommandTester(KernelInterface $kernel): CommandTester
11391
{
114-
$errorRenderer = $this->createMock(ErrorRendererInterface::class);
92+
$errorRenderer = $this->createStub(ErrorRendererInterface::class);
11593
$errorRenderer
116-
->expects($this->any())
11794
->method('render')
118-
->with($this->isInstanceOf(HttpException::class))
11995
->willReturnCallback(function (HttpException $e) {
12096
$exception = FlattenException::createFromThrowable($e);
12197
$exception->setAsString(\sprintf('<html><body>Error %s</body></html>', $e->getStatusCode()));
@@ -125,14 +101,9 @@ private function getCommandTester(KernelInterface $kernel): CommandTester
125101
;
126102

127103
$entrypointLookup = $this->createMock(EntrypointLookupInterface::class);
128-
$entrypointLookup
129-
->expects($this->any())
130-
->method('reset')
131-
;
132104

133105
$application = new Application($kernel);
134106
$application->add(new ErrorDumpCommand(
135-
$this->tmpDir,
136107
new Filesystem(),
137108
$errorRenderer,
138109
$entrypointLookup,

0 commit comments

Comments
 (0)