From cf7b317f4f5e5c6501e8a151437461f8bf9991dd Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 15:52:40 +0200 Subject: [PATCH 01/10] Cleanup test doumentation --- packages/framework/tests/Feature/ConsoleKernelTest.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/framework/tests/Feature/ConsoleKernelTest.php b/packages/framework/tests/Feature/ConsoleKernelTest.php index 0ff66336e10..0523a37fe25 100644 --- a/packages/framework/tests/Feature/ConsoleKernelTest.php +++ b/packages/framework/tests/Feature/ConsoleKernelTest.php @@ -11,7 +11,13 @@ use ReflectionMethod; /** + * This test covers our custom console kernel, which is responsible for registering our custom bootstrappers. + * * @covers \Hyde\Foundation\ConsoleKernel + * + * Our custom bootstrapping system depends on code from Laravel Zero which is marked as internal. + * Sadly, there is no way around working with this private API. Since they may change the API + * at any time, we have tests here to detect if their code changes, so we can catch it early. */ class ConsoleKernelTest extends TestCase { @@ -29,15 +35,11 @@ public function testBootstrappers() { $kernel = app(ConsoleKernel::class); - // Normally, protected code does not need to be unit tested, but since this array is so vital, we want to inspect it. $bootstrappers = (new ReflectionMethod($kernel, 'bootstrappers'))->invoke($kernel); $this->assertIsArray($bootstrappers); $this->assertContains(LoadYamlConfiguration::class, $bootstrappers); - // Another assertion that is usually a no-no, testing vendor code, especially those which are marked as internal. - // We do this here however, so we get a heads-up if the vendor code changes as that could break our code. - $this->assertSame([ 0 => 'LaravelZero\Framework\Bootstrap\CoreBindings', 1 => 'LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables', From 642b04188f84f1bb02e1796c1363055546f4ee62 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 15:58:58 +0200 Subject: [PATCH 02/10] Clean up and split out test with dedicated scope --- .../tests/Feature/ConsoleKernelTest.php | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/packages/framework/tests/Feature/ConsoleKernelTest.php b/packages/framework/tests/Feature/ConsoleKernelTest.php index 0523a37fe25..ed9856efc86 100644 --- a/packages/framework/tests/Feature/ConsoleKernelTest.php +++ b/packages/framework/tests/Feature/ConsoleKernelTest.php @@ -4,6 +4,7 @@ namespace Hyde\Framework\Testing\Feature; +use LaravelZero\Framework\Kernel as LaravelZeroKernel; use Hyde\Foundation\Internal\LoadYamlConfiguration; use Illuminate\Contracts\Console\Kernel; use Hyde\Foundation\ConsoleKernel; @@ -31,24 +32,38 @@ public function testClassImplementsKernelInterface() $this->assertInstanceOf(Kernel::class, app(ConsoleKernel::class)); } - public function testBootstrappers() + public function testLaravelZeroBootstrappersHaveNotChanged() { - $kernel = app(ConsoleKernel::class); + $bootstrappers = (new ReflectionMethod(app(LaravelZeroKernel::class), 'bootstrappers'))->invoke(app(LaravelZeroKernel::class)); - $bootstrappers = (new ReflectionMethod($kernel, 'bootstrappers'))->invoke($kernel); + $this->assertSame([ + \LaravelZero\Framework\Bootstrap\CoreBindings::class, + \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class, + \LaravelZero\Framework\Bootstrap\LoadConfiguration::class, + \Illuminate\Foundation\Bootstrap\HandleExceptions::class, + \LaravelZero\Framework\Bootstrap\RegisterFacades::class, + \LaravelZero\Framework\Bootstrap\RegisterProviders::class, + \Illuminate\Foundation\Bootstrap\BootProviders::class, + ], $bootstrappers); + } + + public function testHydeBootstrapperInjections() + { + $bootstrappers = (new ReflectionMethod(app(ConsoleKernel::class), 'bootstrappers'))->invoke(app(ConsoleKernel::class)); $this->assertIsArray($bootstrappers); $this->assertContains(LoadYamlConfiguration::class, $bootstrappers); + $this->assertSame(range(0, count($bootstrappers) - 1), array_keys($bootstrappers)); $this->assertSame([ - 0 => 'LaravelZero\Framework\Bootstrap\CoreBindings', - 1 => 'LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables', - 2 => 'Hyde\Foundation\Internal\LoadConfiguration', - 3 => 'Illuminate\Foundation\Bootstrap\HandleExceptions', - 4 => 'LaravelZero\Framework\Bootstrap\RegisterFacades', - 5 => 'Hyde\Foundation\Internal\LoadYamlConfiguration', - 6 => 'LaravelZero\Framework\Bootstrap\RegisterProviders', - 7 => 'Illuminate\Foundation\Bootstrap\BootProviders', + \LaravelZero\Framework\Bootstrap\CoreBindings::class, + \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class, + \Hyde\Foundation\Internal\LoadConfiguration::class, + \Illuminate\Foundation\Bootstrap\HandleExceptions::class, + \LaravelZero\Framework\Bootstrap\RegisterFacades::class, + \Hyde\Foundation\Internal\LoadYamlConfiguration::class, + \LaravelZero\Framework\Bootstrap\RegisterProviders::class, + \Illuminate\Foundation\Bootstrap\BootProviders::class, ], $bootstrappers); } } From 387d59e6b9da264e10231df13b7395fb77f86470 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:01:14 +0200 Subject: [PATCH 03/10] Extract helper method --- .../framework/tests/Feature/ConsoleKernelTest.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/framework/tests/Feature/ConsoleKernelTest.php b/packages/framework/tests/Feature/ConsoleKernelTest.php index ed9856efc86..7d574186c10 100644 --- a/packages/framework/tests/Feature/ConsoleKernelTest.php +++ b/packages/framework/tests/Feature/ConsoleKernelTest.php @@ -34,7 +34,8 @@ public function testClassImplementsKernelInterface() public function testLaravelZeroBootstrappersHaveNotChanged() { - $bootstrappers = (new ReflectionMethod(app(LaravelZeroKernel::class), 'bootstrappers'))->invoke(app(LaravelZeroKernel::class)); + $kernel = app(LaravelZeroKernel::class); + $bootstrappers = $this->getBootstrappersFromKernel($kernel); $this->assertSame([ \LaravelZero\Framework\Bootstrap\CoreBindings::class, @@ -49,7 +50,8 @@ public function testLaravelZeroBootstrappersHaveNotChanged() public function testHydeBootstrapperInjections() { - $bootstrappers = (new ReflectionMethod(app(ConsoleKernel::class), 'bootstrappers'))->invoke(app(ConsoleKernel::class)); + $kernel = app(ConsoleKernel::class); + $bootstrappers = $this->getBootstrappersFromKernel($kernel); $this->assertIsArray($bootstrappers); $this->assertContains(LoadYamlConfiguration::class, $bootstrappers); @@ -66,4 +68,9 @@ public function testHydeBootstrapperInjections() \Illuminate\Foundation\Bootstrap\BootProviders::class, ], $bootstrappers); } + + protected function getBootstrappersFromKernel(\Illuminate\Foundation\Console\Kernel $kernel): array + { + return (new ReflectionMethod($kernel, 'bootstrappers'))->invoke($kernel); + } } From 3363a58e9cfad4db19ddcc235ebbd4aa4d4c5246 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:01:34 +0200 Subject: [PATCH 04/10] Inline local variables --- packages/framework/tests/Feature/ConsoleKernelTest.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/framework/tests/Feature/ConsoleKernelTest.php b/packages/framework/tests/Feature/ConsoleKernelTest.php index 7d574186c10..0e6fcc6281f 100644 --- a/packages/framework/tests/Feature/ConsoleKernelTest.php +++ b/packages/framework/tests/Feature/ConsoleKernelTest.php @@ -34,9 +34,6 @@ public function testClassImplementsKernelInterface() public function testLaravelZeroBootstrappersHaveNotChanged() { - $kernel = app(LaravelZeroKernel::class); - $bootstrappers = $this->getBootstrappersFromKernel($kernel); - $this->assertSame([ \LaravelZero\Framework\Bootstrap\CoreBindings::class, \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class, @@ -45,13 +42,12 @@ public function testLaravelZeroBootstrappersHaveNotChanged() \LaravelZero\Framework\Bootstrap\RegisterFacades::class, \LaravelZero\Framework\Bootstrap\RegisterProviders::class, \Illuminate\Foundation\Bootstrap\BootProviders::class, - ], $bootstrappers); + ], $this->getBootstrappersFromKernel(app(LaravelZeroKernel::class))); } public function testHydeBootstrapperInjections() { - $kernel = app(ConsoleKernel::class); - $bootstrappers = $this->getBootstrappersFromKernel($kernel); + $bootstrappers = $this->getBootstrappersFromKernel(app(ConsoleKernel::class)); $this->assertIsArray($bootstrappers); $this->assertContains(LoadYamlConfiguration::class, $bootstrappers); From b67df7c01609a67ef6fd87cedd82a396129f99f9 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:10:44 +0200 Subject: [PATCH 05/10] Restructure code documentation --- packages/framework/src/Foundation/ConsoleKernel.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/framework/src/Foundation/ConsoleKernel.php b/packages/framework/src/Foundation/ConsoleKernel.php index fdad24e0ee1..be45e81efa3 100644 --- a/packages/framework/src/Foundation/ConsoleKernel.php +++ b/packages/framework/src/Foundation/ConsoleKernel.php @@ -21,12 +21,13 @@ protected function bootstrappers(): array { $bootstrappers = $this->bootstrappers; - // Insert our bootstrapper between load configuration and register provider bootstrappers. - array_splice($bootstrappers, 5, 0, LoadYamlConfiguration::class); - // Since we store our application config in `app/config.php`, we need to replace // the default LoadConfiguration bootstrapper class with our implementation. // We do this by swapping out the LoadConfiguration class with our own. + // We also inject our Yaml configuration loading bootstrapper. + + // Insert our bootstrapper between load configuration and register provider bootstrappers. + array_splice($bootstrappers, 5, 0, LoadYamlConfiguration::class); return array_values((array) tap(array_combine($bootstrappers, $bootstrappers), function (array &$array): void { $array[\LaravelZero\Framework\Bootstrap\LoadConfiguration::class] = \Hyde\Foundation\Internal\LoadConfiguration::class; From b57c7c59000b3236793b8591c445e9b282b0ea4e Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:10:50 +0200 Subject: [PATCH 06/10] Add type annotation --- packages/framework/src/Foundation/ConsoleKernel.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/framework/src/Foundation/ConsoleKernel.php b/packages/framework/src/Foundation/ConsoleKernel.php index be45e81efa3..194d8867ba6 100644 --- a/packages/framework/src/Foundation/ConsoleKernel.php +++ b/packages/framework/src/Foundation/ConsoleKernel.php @@ -19,6 +19,7 @@ class ConsoleKernel extends Kernel */ protected function bootstrappers(): array { + /** @var array $bootstrappers */ $bootstrappers = $this->bootstrappers; // Since we store our application config in `app/config.php`, we need to replace From 6b7641401ed453b34f8ff88e32c0e07176e2af34 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:11:22 +0200 Subject: [PATCH 07/10] Move up documentation block --- packages/framework/src/Foundation/ConsoleKernel.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/framework/src/Foundation/ConsoleKernel.php b/packages/framework/src/Foundation/ConsoleKernel.php index 194d8867ba6..a22b5a125f4 100644 --- a/packages/framework/src/Foundation/ConsoleKernel.php +++ b/packages/framework/src/Foundation/ConsoleKernel.php @@ -19,14 +19,14 @@ class ConsoleKernel extends Kernel */ protected function bootstrappers(): array { - /** @var array $bootstrappers */ - $bootstrappers = $this->bootstrappers; - // Since we store our application config in `app/config.php`, we need to replace // the default LoadConfiguration bootstrapper class with our implementation. // We do this by swapping out the LoadConfiguration class with our own. // We also inject our Yaml configuration loading bootstrapper. + /** @var array $bootstrappers */ + $bootstrappers = $this->bootstrappers; + // Insert our bootstrapper between load configuration and register provider bootstrappers. array_splice($bootstrappers, 5, 0, LoadYamlConfiguration::class); From 931af5f63378e9c5156942c69273061e3052ee77 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:15:40 +0200 Subject: [PATCH 08/10] Begin refactoring bootstrapper array manipulations to be clearer --- .../framework/src/Foundation/ConsoleKernel.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/framework/src/Foundation/ConsoleKernel.php b/packages/framework/src/Foundation/ConsoleKernel.php index a22b5a125f4..aacbd7654e0 100644 --- a/packages/framework/src/Foundation/ConsoleKernel.php +++ b/packages/framework/src/Foundation/ConsoleKernel.php @@ -27,11 +27,16 @@ protected function bootstrappers(): array /** @var array $bootstrappers */ $bootstrappers = $this->bootstrappers; - // Insert our bootstrapper between load configuration and register provider bootstrappers. - array_splice($bootstrappers, 5, 0, LoadYamlConfiguration::class); + // First we key the array by the class name so we can easily manipulate it. + $bootstrappers = array_combine($bootstrappers, $bootstrappers); - return array_values((array) tap(array_combine($bootstrappers, $bootstrappers), function (array &$array): void { - $array[\LaravelZero\Framework\Bootstrap\LoadConfiguration::class] = \Hyde\Foundation\Internal\LoadConfiguration::class; - })); + // Remove the Laravel Zero LoadConfiguration bootstrapper + unset($bootstrappers[\LaravelZero\Framework\Bootstrap\LoadConfiguration::class]); + + // Inject our custom LoadConfiguration bootstrapper + $bootstrappers[\Hyde\Foundation\Internal\LoadConfiguration::class] = \Hyde\Foundation\Internal\LoadConfiguration::class; + + // Now we return the bootstrappers as a numerically indexed array, like it was before. + return array_values($bootstrappers); } } From ce1ea528b8e94e5e6c8f66557956e42ef9dd9234 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:18:37 +0200 Subject: [PATCH 09/10] Directly return the intended array This is clearer, and our test ensure we have the right array contents, and this doesn't even necessarily change any outcomes. --- .../src/Foundation/ConsoleKernel.php | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/packages/framework/src/Foundation/ConsoleKernel.php b/packages/framework/src/Foundation/ConsoleKernel.php index aacbd7654e0..a4fee7af77e 100644 --- a/packages/framework/src/Foundation/ConsoleKernel.php +++ b/packages/framework/src/Foundation/ConsoleKernel.php @@ -5,12 +5,6 @@ namespace Hyde\Foundation; use LaravelZero\Framework\Kernel; -use Hyde\Foundation\Internal\LoadYamlConfiguration; - -use function array_combine; -use function array_splice; -use function array_values; -use function tap; class ConsoleKernel extends Kernel { @@ -24,19 +18,15 @@ protected function bootstrappers(): array // We do this by swapping out the LoadConfiguration class with our own. // We also inject our Yaml configuration loading bootstrapper. - /** @var array $bootstrappers */ - $bootstrappers = $this->bootstrappers; - - // First we key the array by the class name so we can easily manipulate it. - $bootstrappers = array_combine($bootstrappers, $bootstrappers); - - // Remove the Laravel Zero LoadConfiguration bootstrapper - unset($bootstrappers[\LaravelZero\Framework\Bootstrap\LoadConfiguration::class]); - - // Inject our custom LoadConfiguration bootstrapper - $bootstrappers[\Hyde\Foundation\Internal\LoadConfiguration::class] = \Hyde\Foundation\Internal\LoadConfiguration::class; - - // Now we return the bootstrappers as a numerically indexed array, like it was before. - return array_values($bootstrappers); + return [ + \LaravelZero\Framework\Bootstrap\CoreBindings::class, + \LaravelZero\Framework\Bootstrap\LoadEnvironmentVariables::class, + \Hyde\Foundation\Internal\LoadConfiguration::class, + \Illuminate\Foundation\Bootstrap\HandleExceptions::class, + \LaravelZero\Framework\Bootstrap\RegisterFacades::class, + \Hyde\Foundation\Internal\LoadYamlConfiguration::class, + \LaravelZero\Framework\Bootstrap\RegisterProviders::class, + \Illuminate\Foundation\Bootstrap\BootProviders::class, + ]; } } From d9080ccc2277a4e904bd6a84183ef836a84fee02 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Mon, 1 Jul 2024 16:21:26 +0200 Subject: [PATCH 10/10] Clean up test --- packages/framework/tests/Feature/ConsoleKernelTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/framework/tests/Feature/ConsoleKernelTest.php b/packages/framework/tests/Feature/ConsoleKernelTest.php index 0e6fcc6281f..6c433bdb842 100644 --- a/packages/framework/tests/Feature/ConsoleKernelTest.php +++ b/packages/framework/tests/Feature/ConsoleKernelTest.php @@ -51,7 +51,6 @@ public function testHydeBootstrapperInjections() $this->assertIsArray($bootstrappers); $this->assertContains(LoadYamlConfiguration::class, $bootstrappers); - $this->assertSame(range(0, count($bootstrappers) - 1), array_keys($bootstrappers)); $this->assertSame([ \LaravelZero\Framework\Bootstrap\CoreBindings::class,