Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge validating command features into base command #891

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5edc84b
Move constant to base command
caendesilva Jan 30, 2023
037b997
Move handle method to base command
caendesilva Jan 30, 2023
38012f6
Move safeHandle method to base command
caendesilva Jan 30, 2023
b561422
Move handleException method to base command
caendesilva Jan 30, 2023
5806fc5
Bypass inheritance conflict
caendesilva Jan 30, 2023
bb99a62
Move tests for moved methods
caendesilva Jan 30, 2023
833eda1
Split out test for increased clarity
caendesilva Jan 30, 2023
22a7322
Add PHPDoc method description
caendesilva Jan 30, 2023
23db27f
Add line break
caendesilva Jan 30, 2023
048d9be
Introduce local variable to make unreadable code readable
caendesilva Jan 30, 2023
ddad744
Convert a 'sprintf()' call and concatenation to string interpolation
caendesilva Jan 30, 2023
cde73ae
Inline local variable
caendesilva Jan 30, 2023
5e342ff
Format sprintf arguments
caendesilva Jan 30, 2023
0c1af73
Deprecate arguments only used in tests adding unnecessary complexity
caendesilva Jan 30, 2023
0fc3ad7
Apply fixes from StyleCI
StyleCIBot Jan 30, 2023
a517b32
Create test case helper for setting safeHandle throw state
caendesilva Jan 30, 2023
4246db7
Use the added throw control helper
caendesilva Jan 30, 2023
c3e01c7
Revert "Bypass inheritance conflict"
caendesilva Jan 30, 2023
d1d695b
Rename helper method for conflicting method name
caendesilva Jan 30, 2023
6af9fe2
Remove deprecated arguments only used in test
caendesilva Jan 30, 2023
395996d
Remove tests causing more trouble than they're worth
caendesilva Jan 30, 2023
0a1c4d3
Split string in two strings and concatenation
caendesilva Jan 30, 2023
7cac05c
Introduce local variable
caendesilva Jan 30, 2023
5f9fa62
Extract common parts from if statement
caendesilva Jan 30, 2023
4c05568
Simplify 'if'
caendesilva Jan 30, 2023
984f4c7
Merge concatenated string into sprintf call
caendesilva Jan 30, 2023
986b5c8
Format sprintf call
caendesilva Jan 30, 2023
c27acf2
Apply fixes from StyleCI
StyleCIBot Jan 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function handle(): int
try {
$this->validate();
} catch (Exception $exception) {
return $this->handleException($exception);
return $this->withException($exception);
}

(new RebuildService($this->path))->execute();
Expand Down Expand Up @@ -102,7 +102,7 @@ public function validate(): void
*
* @return int Error code
*/
public function handleException(Exception $exception): int
public function withException(Exception $exception): int
{
$this->error('Something went wrong!');
$this->warn($exception->getMessage());
Expand Down
55 changes: 55 additions & 0 deletions packages/framework/src/Console/Concerns/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,69 @@

namespace Hyde\Console\Concerns;

use function config;
use Exception;
use Hyde\Hyde;
use LaravelZero\Framework\Commands\Command as BaseCommand;
use function sprintf;

/**
* @see \Hyde\Framework\Testing\Feature\CommandTest
*/
abstract class Command extends BaseCommand
{
public const USER_EXIT = 130;

/**
* The base handle method that can be overridden by child classes.
*
* Alternatively, implement the safeHandle method in your child class
* to utilize the automatic exception handling provided by this method.
*
* @return int The exit code.
*/
public function handle(): int
{
try {
return $this->safeHandle();
} catch (Exception $exception) {
return $this->handleException($exception);
}
}

/**
* This method can be overridden by child classes to provide automatic exception handling.
*
* Existing code can be converted simply by renaming the handle() method to safeHandle().
*
* @return int The exit code.
*/
protected function safeHandle(): int
{
return Command::SUCCESS;
}

/**
* Handle an exception that occurred during command execution.
*
* @return int The exit code
*/
public function handleException(Exception $exception): int
{
// When testing it might be more useful to see the full stack trace, so we have an option to actually throw the exception.
if (config('app.throw_on_console_exception', false)) {
throw $exception;
}

// If the exception was thrown from the same file as a command, then we don't need to show which file it was thrown from.
$location = str_ends_with($exception->getFile(), 'Command.php') ? '' : sprintf(' at %s:%s',
$exception->getFile(), $exception->getLine()
);
$this->error("Error: {$exception->getMessage()}".$location);

return Command::FAILURE;
}

/**
* Create a filepath that can be opened in the browser from a terminal.
*/
Expand Down
63 changes: 63 additions & 0 deletions packages/framework/tests/Feature/CommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use Hyde\Console\Concerns\Command;
use Hyde\Hyde;
use Hyde\Testing\TestCase;
use Mockery;
use RuntimeException;
use Symfony\Component\Console\Style\OutputStyle;

/**
Expand Down Expand Up @@ -122,6 +124,46 @@ public function testIndentedLine()
$command->setMockedOutput($output);
$command->handle();
}

public function testHandleCallsBaseSafeHandle()
{
$this->assertSame(0, (new TestCommand())->handle());
}

public function testHandleCallsChildSafeHandle()
{
$this->assertSame(1, (new SafeHandleTestCommand())->handle());
}

public function testSafeHandleException()
{
$command = new SafeThrowingTestCommand();
$output = Mockery::mock(\Illuminate\Console\OutputStyle::class);
$output->shouldReceive('writeln')->once()->withArgs(function (string $message) {
return str_starts_with($message, '<error>Error: This is a test at '.__FILE__.':');
});
$command->setOutput($output);

$code = $command->handle();

$this->assertSame(1, $code);
}

public function testCanEnableThrowOnException()
{
$this->throwOnConsoleException();
$command = new SafeThrowingTestCommand();

$output = Mockery::mock(\Illuminate\Console\OutputStyle::class);

$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('This is a test');

$command->setOutput($output);
$code = $command->handle();

$this->assertSame(1, $code);
}
}

class MockableTestCommand extends Command
Expand All @@ -140,3 +182,24 @@ public function setMockedOutput($output)
$this->output = $output;
}
}

class TestCommand extends Command
{
//
}

class SafeHandleTestCommand extends Command
{
public function safeHandle(): int
{
return 1;
}
}

class SafeThrowingTestCommand extends Command
{
public function safeHandle(): int
{
throw new RuntimeException('This is a test');
}
}
50 changes: 0 additions & 50 deletions packages/publications/src/Commands/ValidatingCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@

use function __;
use function array_merge;
use Exception;
use Hyde\Console\Concerns\Command;
use Illuminate\Support\Facades\Validator;
use function in_array;
use RuntimeException;
use function str_ends_with;
use function ucfirst;

/**
Expand All @@ -21,34 +19,9 @@
*/
class ValidatingCommand extends Command
{
public const USER_EXIT = 130;

/** @var int How many times can the validation loop run? It is high enough to not affect normal usage. */
protected final const MAX_RETRIES = 30;

/**
* @return int The exit code.
*/
public function handle(): int
{
try {
return $this->safeHandle();
} catch (Exception $exception) {
return $this->handleException($exception);
}
}

/**
* This method can be overridden by child classes to provide automatic exception handling.
* Existing code can be converted simply by renaming the handle() method to safeHandle().
*
* @return int The exit code.
*/
protected function safeHandle(): int
{
return Command::SUCCESS;
}

/**
* Ask for a CLI input value until we pass validation rules.
*
Expand Down Expand Up @@ -94,29 +67,6 @@ public function reloadableChoice(callable $options, string $question, string $re
return $selection;
}

/**
* Handle an exception that occurred during command execution.
*
* @param string|null $file The file where the exception occurred. Leave null to auto-detect.
* @return int The exit code
*/
public function handleException(Exception $exception, ?string $file = null, ?int $line = null): int
{
// When testing it might be more useful to see the full stack trace, so we have an option to actually throw the exception.
if (config('app.throw_on_console_exception', false)) {
throw $exception;
}

// If the exception was thrown from the same file as a command, then we don't need to show which file it was thrown from.
if (str_ends_with($file ?? $exception->getFile(), 'Command.php')) {
$this->error("Error: {$exception->getMessage()}");
} else {
$this->error(sprintf('Error: %s at ', $exception->getMessage()).sprintf('%s:%s', $file ?? $exception->getFile(), $line ?? $exception->getLine()));
}

return Command::FAILURE;
}

protected function translate(string $name, string $error): string
{
return __($error, [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Hyde\Publications\Testing\Feature;

use function array_merge;
use function config;
use function file_get_contents;
use function file_put_contents;
use Hyde\Facades\Filesystem;
Expand All @@ -24,7 +23,7 @@ class MakePublicationCommandTest extends TestCase
protected function setUp(): void
{
parent::setUp();
config(['app.throw_on_console_exception' => true]);
$this->throwOnConsoleException();

Filesystem::makeDirectory('test-publication');
Carbon::setTestNow(Carbon::create(2022));
Expand Down Expand Up @@ -54,7 +53,7 @@ public function test_command_creates_publication()

public function test_command_with_no_publication_types()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->artisan('make:publication')
->expectsOutputToContain('Creating a new publication!')
->expectsOutput('Error: Unable to locate any publication types. Did you create any?')
Expand Down Expand Up @@ -167,7 +166,7 @@ public function test_command_with_publication_type_passed_as_argument()

public function test_command_with_invalid_publication_type_passed_as_argument()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->makeSchemaFile();

$this->artisan('make:publication foo')
Expand Down Expand Up @@ -376,7 +375,7 @@ public function test_command_with_multiple_tag_inputs()

public function test_media_input_with_no_images()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->makeSchemaFile([
'canonicalField' => '__createdAt',
'fields' => [[
Expand Down Expand Up @@ -427,7 +426,7 @@ public function test_media_input_with_no_files_but_skips()

public function test_tag_input_with_no_tags()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->makeSchemaFile([
'canonicalField' => '__createdAt',
'fields' => [[
Expand Down Expand Up @@ -480,7 +479,7 @@ public function test_tag_input_with_no_tags_but_skips()

public function test_tag_input_for_field_without_tagGroup_specified()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->makeSchemaFile([
'canonicalField' => '__createdAt',
'fields' => [[
Expand All @@ -499,7 +498,7 @@ public function test_tag_input_for_field_without_tagGroup_specified()

public function test_handleEmptyOptionsCollection_for_required_field()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->makeSchemaFile([
'canonicalField' => '__createdAt',
'fields' => [[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Hyde\Publications\Testing\Feature;

use function config;
use Hyde\Facades\Filesystem;
use Hyde\Hyde;
use Hyde\Publications\Commands\Helpers\InputStreamHandler;
Expand All @@ -26,7 +25,7 @@ protected function setUp(): void
{
parent::setUp();

config(['app.throw_on_console_exception' => true]);
$this->throwOnConsoleException();
}

protected function tearDown(): void
Expand Down Expand Up @@ -171,7 +170,7 @@ public function test_with_multiple_fields_of_the_same_name()

public function test_with_existing_file_of_the_same_name()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);

$this->file('test-publication');

Expand All @@ -182,7 +181,7 @@ public function test_with_existing_file_of_the_same_name()

public function test_with_existing_publication_of_the_same_name()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);

$this->directory('test-publication');
$this->file('test-publication/foo');
Expand Down Expand Up @@ -242,7 +241,7 @@ public function testWithTagFieldInput()

public function testWithTagFieldInputButNoTags()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);
$this->directory('test-publication');

$this->artisan('make:publicationType "Test Publication"')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected function setUp(): void
{
parent::setUp();

config(['app.throw_on_console_exception' => true]);
$this->throwOnConsoleException();
}

public function testCompilingWithPublicationTypeWithSeededFilesContainingAllFieldTypes()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ protected function setUp(): void
{
parent::setUp();

config(['app.throw_on_console_exception' => true]);
$this->throwOnConsoleException();
}

public function testWithNoPublicationTypes()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);

$this->artisan('validate:publicationTypes')
->expectsOutputToContain('Validating publication schemas!')
Expand All @@ -31,7 +31,7 @@ public function testWithNoPublicationTypes()

public function testWithValidSchemaFile()
{
config(['app.throw_on_console_exception' => false]);
$this->throwOnConsoleException(false);

$this->directory('test-publication');
$publicationType = new PublicationType('test-publication', fields: [
Expand Down
Loading