Skip to content

Commit

Permalink
Fix the bug where the second compilation for Artisan commands fails
Browse files Browse the repository at this point in the history
This is because Artisan commands are loaded when Laravel starts up,
causing Ray.Aop's compiler to skip compilation.
  • Loading branch information
ngmy committed Sep 18, 2024
1 parent 509a85b commit fdc0ca5
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 41 deletions.
26 changes: 26 additions & 0 deletions src/ServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ public function register(): void
->giveConfig('aop.watcher.paths')
;

if ($this->runInCompileCommand() || $this->runInWatchCommand()) {
return;
}

ServiceLocator::setReader(new AttributeReader());

$registrar = $this->app->make(ServiceRegistrar::class);
Expand All @@ -54,4 +58,26 @@ public function boot(): void
]);
}
}

/**
* Check if the command is running in the compile command.
*
* @return bool True if the command is running in the compile command, false otherwise
*/
private function runInCompileCommand(): bool
{
return isset($_SERVER['argv'][0]) && 'artisan' === $_SERVER['argv'][0]
&& isset($_SERVER['argv'][1]) && 'aop:compile' === $_SERVER['argv'][1];
}

/**
* Check if the command is running in the watch command.
*
* @return bool True if the command is running in the watch command, false otherwise
*/
private function runInWatchCommand(): bool
{
return isset($_SERVER['argv'][0]) && 'artisan' === $_SERVER['argv'][0]
&& isset($_SERVER['argv'][1]) && 'aop:watch' === $_SERVER['argv'][1];
}
}
43 changes: 14 additions & 29 deletions tests/Feature/AopTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Ngmy\LaravelAop\Tests\Feature;

use Illuminate\Support\Facades\File;
use Illuminate\Testing\PendingCommand;
use Ngmy\LaravelAop\Services\ServiceRegistrar;
use Ngmy\LaravelAop\Tests\Feature\stubs\Attributes\TestAttribute1;
use Ngmy\LaravelAop\Tests\Feature\stubs\Attributes\TestAttribute2;
Expand All @@ -17,7 +16,9 @@
use Ngmy\LaravelAop\Tests\Feature\stubs\Interceptors\TestInterceptor3;
use Ngmy\LaravelAop\Tests\Feature\stubs\Targets\TestTarget1;
use Ngmy\LaravelAop\Tests\TestCase;
use Ngmy\LaravelAop\Tests\utils\SpyLogger;
use Ngmy\LaravelAop\Tests\utils\Attributes\DoesNotDeleteCompiledDirectoryAfter;
use Ngmy\LaravelAop\Tests\utils\Attributes\DoesNotDeleteCompiledDirectoryBefore;
use Ngmy\LaravelAop\Tests\utils\Spies\SpyLogger;
use Psr\Log\LogLevel;

/**
Expand All @@ -43,17 +44,6 @@ final class AopTest extends TestCase
{
protected bool $compileAopClasses = false;

private string $compiledPath;

protected function setUp(): void
{
parent::setUp();

/** @var string $compiledPath */
$compiledPath = config('aop.compiled');
$this->compiledPath = $compiledPath;
}

/**
* @return iterable<string, list{class-string, string, ExpectedLogs, bool, bool}> The AOP cases
*/
Expand Down Expand Up @@ -169,6 +159,8 @@ public static function provideAopCases(): iterable
* @param ExpectedLogs $expectedLogs The expected logs
* @param bool $isFirst Whether this is the first case
*/
#[DoesNotDeleteCompiledDirectoryAfter]
#[DoesNotDeleteCompiledDirectoryBefore]
public function testAopWhenCompiledClassesAreLoaded(
string $targetClassName,
string $targetMethodName,
Expand Down Expand Up @@ -201,6 +193,8 @@ public function testAopWhenCompiledClassesAreLoaded(
* @param ExpectedLogs $expectedLogs The expected logs
* @param bool $isLast Whether this is the last case
*/
#[DoesNotDeleteCompiledDirectoryAfter]
#[DoesNotDeleteCompiledDirectoryBefore]
public function testAopWhenCompiledClassesAreNotLoaded(
string $targetClassName,
string $targetMethodName,
Expand All @@ -219,16 +213,20 @@ public function testAopWhenCompiledClassesAreNotLoaded(

public function testCompileCommandWhenCompiledFilesExist(): void
{
File::deleteDirectory($this->compiledPath);

// Create dummy compiled files
File::makeDirectory($this->compiledPath, 0o755, true, true);
File::put($this->compiledPath.'/source_map.ser', '');
File::put($this->compiledPath.'/Ngmy_LaravelAop_Tests_Feature_stubs_Targets_TestTarget1_3064002867.php', '');

$this->assertCompileCommand();
}

public function testBindWhenSourceMapFileDoesNotExist(): void
{
$serviceRegistrar = $this->app->make(ServiceRegistrar::class);
$serviceRegistrar->bind();

File::deleteDirectory($this->compiledPath);
self::assertTrue(true);
}

protected function resolveApplicationConfiguration($app): void
Expand Down Expand Up @@ -279,17 +277,4 @@ private function assertAop(

self::assertLogCalls($expectedLogs, $spyLogger);
}

/**
* Assert the compile command.
*/
private function assertCompileCommand(): void
{
/** @var PendingCommand $command */
$command = $this->artisan('aop:compile');
$command->run();
$command->assertSuccessful();

self::assertDirectoryExists($this->compiledPath);
}
}
2 changes: 1 addition & 1 deletion tests/Feature/Aspects/Retry/RetryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use Ngmy\LaravelAop\Tests\Feature\Aspects\Retry\stubs\Interceptors\TestInterceptor1;
use Ngmy\LaravelAop\Tests\Feature\Aspects\Retry\stubs\Targets\TestTarget1;
use Ngmy\LaravelAop\Tests\TestCase;
use Ngmy\LaravelAop\Tests\utils\SpyLogger;
use Ngmy\LaravelAop\Tests\utils\Spies\SpyLogger;
use Psr\Log\LogLevel;

/**
Expand Down
9 changes: 2 additions & 7 deletions tests/Feature/WatcherTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@

namespace Ngmy\LaravelAop\Tests\Feature;

use Illuminate\Testing\PendingCommand;
use Ngmy\LaravelAop\Tests\TestCase;
use Ngmy\LaravelAop\Tests\utils\SpyLogger;
use Ngmy\LaravelAop\Tests\utils\Spies\SpyLogger;
use Psr\Log\LogLevel;

/**
Expand All @@ -25,11 +24,7 @@ public function testWatchCommand(): void
{
$spyLogger = (new SpyLogger())->use();

/** @var PendingCommand $command */
$command = $this->artisan('aop:watch');
$command->run();
$command->expectsOutput('Watching...');
$command->assertSuccessful();
$this->assertWatchCommand();

$format = '%s [%s].';
$path = app_path('test.php');
Expand Down
33 changes: 31 additions & 2 deletions tests/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,31 @@
namespace Ngmy\LaravelAop\Tests;

use Illuminate\Contracts\Foundation\Application;
use Illuminate\Support\Facades\File;
use Ngmy\LaravelAop\Services\ServiceRegistrar;
use Ngmy\LaravelAop\Tests\utils\SpyLoggerAssertions;
use Ngmy\LaravelAop\Tests\utils\Assertions\ArtisanAssertions;
use Ngmy\LaravelAop\Tests\utils\Assertions\SpyLoggerAssertions;
use Ngmy\LaravelAop\Tests\utils\Attributes\DoesNotDeleteCompiledDirectoryAfter;
use Ngmy\LaravelAop\Tests\utils\Attributes\DoesNotDeleteCompiledDirectoryBefore;
use Ngmy\LaravelAop\Tests\utils\Helpers\AttributeHelpers;
use Orchestra\Testbench\TestCase as BaseTestCase;

/**
* @property Application $app
*/
abstract class TestCase extends BaseTestCase
{
use ArtisanAssertions;
use AttributeHelpers;
use SpyLoggerAssertions;

protected $enablesPackageDiscoveries = true;

/**
* The compiled path.
*/
protected string $compiledPath;

/**
* Whether to compile AOP classes before each test.
*/
Expand All @@ -27,9 +39,26 @@ protected function setUp(): void
{
parent::setUp();

/** @var string $compiledPath */
$compiledPath = config('aop.compiled');
$this->compiledPath = $compiledPath;

if (!$this->hasAttributes(DoesNotDeleteCompiledDirectoryBefore::class)) {
File::deleteDirectory($this->compiledPath);
}

if ($this->compileAopClasses) {
$this->artisan('aop:compile');
$this->assertCompileCommand();
$this->app->make(ServiceRegistrar::class)->bind();
}
}

protected function tearDown(): void
{
if (!$this->hasAttributes(DoesNotDeleteCompiledDirectoryAfter::class)) {
File::deleteDirectory($this->compiledPath);
}

parent::tearDown();
}
}
46 changes: 46 additions & 0 deletions tests/utils/Assertions/ArtisanAssertions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils\Assertions;

use Illuminate\Testing\PendingCommand;
use Ngmy\LaravelAop\Tests\TestCase;

/**
* @require-extends TestCase
*/
trait ArtisanAssertions
{
/**
* Assert the compile command.
*/
protected function assertCompileCommand(): void
{
$_SERVER['argv'] = ['artisan', 'aop:compile'];

/** @var PendingCommand $command */
$command = $this->artisan('aop:compile');
$command->run();
$command->assertSuccessful();

/** @var string $compiledPath */
$compiledPath = config('aop.compiled');

self::assertDirectoryExists($compiledPath);
}

/**
* Assert the watch command.
*/
protected function assertWatchCommand(): void
{
$_SERVER['argv'] = ['artisan', 'aop:watch'];

/** @var PendingCommand $command */
$command = $this->artisan('aop:watch');
$command->run();
$command->expectsOutput('Watching...');
$command->assertSuccessful();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils;
namespace Ngmy\LaravelAop\Tests\utils\Assertions;

use Ngmy\LaravelAop\Tests\TestCase;
use Ngmy\LaravelAop\Tests\utils\Spies\SpyLogger;
use Psr\Log\LogLevel;

/**
* @phpstan-type ExpectedLogs list<list{LogLevel::*, string}>
*
* @require-extends TestCase
*/
trait SpyLoggerAssertions
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils\Attributes;

#[\Attribute(\Attribute::TARGET_METHOD)]
class DoesNotDeleteCompiledDirectoryAfter {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils\Attributes;

#[\Attribute(\Attribute::TARGET_METHOD)]
class DoesNotDeleteCompiledDirectoryBefore {}
67 changes: 67 additions & 0 deletions tests/utils/Helpers/AttributeHelpers.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
<?php

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils\Helpers;

use Ngmy\LaravelAop\Tests\TestCase;
use PHPUnit\Runner\Version;

/**
* @require-extends TestCase
*/
trait AttributeHelpers
{
/**
* Get the attributes of the test method.
*
* @template T of object
*
* @param null|class-string<T> $name The name of the attribute
* @param int $flags The flags to pass to ReflectionFunctionAbstract::getAttributes()
*
* @return list<\ReflectionAttribute<T>> The attributes of the test method
*/
protected function getAttributes(?string $name = null, int $flags = 0): array
{
$reflection = new \ReflectionClass($this);
$method = $reflection->getMethod($this->_getName());

return $method->getAttributes($name, $flags);
}

/**
* Whether the test method has the specified attributes.
*
* @param null|class-string $name The name of the attribute
* @param int $flags The flags to pass to ReflectionFunctionAbstract::getAttributes()
*
* @return bool True if the test method has the specified attributes, false otherwise
*/
protected function hasAttributes(?string $name = null, int $flags = 0): bool
{
return !empty($this->getAttributes($name, $flags));
}

/**
* Get the name of the test method.
*
* @return string The name of the test method
*
* @todo Remove this method when PHPUnit 10 is the minimum required version
*/
private function _getName(): string
{
$version = (int) Version::id();

if ($version >= 10) {
\assert(method_exists($this, 'name'));
$methodName = $this->name();
} else {
\assert(method_exists($this, 'getName'));
$methodName = $this->getName();
}

return $methodName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

declare(strict_types=1);

namespace Ngmy\LaravelAop\Tests\utils;
namespace Ngmy\LaravelAop\Tests\utils\Spies;

use Illuminate\Contracts\Support\Arrayable;
use Illuminate\Contracts\Support\Jsonable;
Expand Down

0 comments on commit fdc0ca5

Please sign in to comment.