Skip to content

Commit

Permalink
Instantiate the plugins from within the hook runner
Browse files Browse the repository at this point in the history
  • Loading branch information
@ramsey
ramsey committed May 2, 2021
1 parent 9bbc7dc commit 3844b7f
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 78 deletions.
32 changes: 7 additions & 25 deletions src/Config/Plugin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ class Plugin
*
* @var string
*/
private $pluginClass;

/**
* Plugin instance
*
* @var CaptainHook
*/
private $plugin;

/**
Expand All @@ -48,17 +41,16 @@ class Plugin
/**
* Plugin constructor
*
* @param string $pluginClass
* @param string $plugin
* @param array $options
*/
public function __construct(string $pluginClass, array $options = [])
public function __construct(string $plugin, array $options = [])
{
if (!is_a($pluginClass, CaptainHook::class, true)) {
throw new InvalidPlugin("{$pluginClass} is not a valid CaptainHook plugin.");
if (!is_a($plugin, CaptainHook::class, true)) {
throw new InvalidPlugin("{$plugin} is not a valid CaptainHook plugin.");
}

$this->pluginClass = $pluginClass;
$this->plugin = new $pluginClass();
$this->plugin = $plugin;
$this->setupOptions($options);
}

Expand All @@ -77,17 +69,7 @@ private function setupOptions(array $options): void
*
* @return string
*/
public function getPluginClass(): string
{
return $this->pluginClass;
}

/**
* Return the plugin instance
*
* @return CaptainHook
*/
public function getPlugin(): CaptainHook
public function getPlugin(): string
{
return $this->plugin;
}
Expand All @@ -110,7 +92,7 @@ public function getOptions(): Options
public function getJsonData(): array
{
return [
'plugin' => $this->pluginClass,
'plugin' => $this->plugin,
'options' => $this->options->getAll(),
];
}
Expand Down
15 changes: 9 additions & 6 deletions src/Runner/Hook.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,27 +394,30 @@ private function getHookPlugins(): array
$this->hookPlugins = [];

foreach ($this->config->getPlugins() as $pluginConfig) {
$plugin = $pluginConfig->getPlugin();

if (!$plugin instanceof Plugin\Hook) {
$pluginClass = $pluginConfig->getPlugin();
if (!is_a($pluginClass, Plugin\Hook::class, true)) {
continue;
}

$this->io->write(
['', 'Configuring Hook Plugin: <comment>' . $pluginConfig->getPluginClass() . '</comment>'],
['', 'Configuring Hook Plugin: <comment>' . $pluginClass . '</comment>'],
true,
IO::VERBOSE
);

if ($plugin instanceof Constrained && !$plugin->getRestriction()->isApplicableFor($this->hook)) {
if (
is_a($pluginClass, Constrained::class, true)
&& !$pluginClass::getRestriction()->isApplicableFor($this->hook)
) {
$this->io->write(
'Skipped because plugin it is not applicable for hook ' . $this->hook,
'Skipped because plugin is not applicable for hook ' . $this->hook,
true,
IO::VERBOSE
);
continue;
}

$plugin = new $pluginClass();
$plugin->configure($this->config, $this->io, $this->repository, $pluginConfig);

$this->hookPlugins[] = $plugin;
Expand Down
9 changes: 1 addition & 8 deletions tests/CaptainHook/Config/PluginTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,7 @@ public function testGetPluginClass(): void
{
$plugin = new Plugin($this->class);

$this->assertEquals($this->class, $plugin->getPluginClass());
}

public function testGetPlugin(): void
{
$plugin = new Plugin($this->class);

$this->assertInstanceOf($this->class, $plugin->getPlugin());
$this->assertEquals($this->class, $plugin->getPlugin());
}

public function testGetOptions(): void
Expand Down
2 changes: 1 addition & 1 deletion tests/CaptainHook/Plugin/DummyConstrainedHookPlugin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class DummyConstrainedHookPlugin extends DummyHookPlugin implements Constrained
{
/**
* @var Restriction
* @var Restriction|null
*/
public static $restriction;

Expand Down
2 changes: 1 addition & 1 deletion tests/CaptainHook/Plugin/DummyConstrainedHookPluginAlt.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class DummyConstrainedHookPluginAlt extends DummyHookPlugin implements Constrained
{
/**
* @var Restriction
* @var Restriction|null
*/
public static $restriction;

Expand Down
2 changes: 1 addition & 1 deletion tests/CaptainHook/Plugin/DummyConstrainedPlugin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class DummyConstrainedPlugin extends DummyPlugin implements Constrained
{
/**
* @var Restriction
* @var Restriction|null
*/
public static $restriction;

Expand Down
16 changes: 8 additions & 8 deletions tests/CaptainHook/Plugin/DummyHookPlugin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,28 @@

class DummyHookPlugin extends Hook\Base
{
public $beforeHookCalled = 0;
public $beforeActionCalled = 0;
public $afterActionCalled = 0;
public $afterHookCalled = 0;
public static $beforeHookCalled = 0;
public static $beforeActionCalled = 0;
public static $afterActionCalled = 0;
public static $afterHookCalled = 0;

public function beforeHook(RunnerHook $hook): void
{
$this->beforeHookCalled++;
self::$beforeHookCalled++;
}

public function beforeAction(RunnerHook $hook, Config\Action $action): void
{
$this->beforeActionCalled++;
self::$beforeActionCalled++;
}

public function afterAction(RunnerHook $hook, Config\Action $action): void
{
$this->afterActionCalled++;
self::$afterActionCalled++;
}

public function afterHook(RunnerHook $hook): void
{
$this->afterHookCalled++;
self::$afterHookCalled++;
}
}
6 changes: 3 additions & 3 deletions tests/CaptainHook/Plugin/DummyHookPluginSkipsActions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ class DummyHookPluginSkipsActions extends DummyHookPlugin
*
* @var string
*/
public $skipStartIn = 'beforeHook';
public static $skipStartIn = 'beforeHook';

/**
* Start skipping actions after the $skipStartIn method has been
* called this many times.
*
* @var int
*/
public $skipStartAt = 1;
public static $skipStartAt = 1;

public function beforeHook(RunnerHook $hook): void
{
Expand Down Expand Up @@ -51,7 +51,7 @@ private function checkSkipStart(string $method, RunnerHook $hook): void
{
$property = $method . 'Called';

if ($this->skipStartIn === $method && $this->{$property} === $this->skipStartAt) {
if (self::$skipStartIn === $method && self::${$property} === self::$skipStartAt) {
$hook->shouldSkipActions(true);
}
}
Expand Down
70 changes: 45 additions & 25 deletions tests/CaptainHook/Runner/HookTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,34 @@ class HookTest extends TestCase
use IOMockery;
use CHMockery;

protected function setUp(): void
{
// Ensure the static properties on dummy plugins are all set to their defaults.
DummyHookPlugin::$beforeHookCalled = 0;
DummyHookPlugin::$beforeActionCalled = 0;
DummyHookPlugin::$afterActionCalled = 0;
DummyHookPlugin::$afterHookCalled = 0;
DummyHookPluginSkipsActions::$skipStartIn = 'beforeHook';
DummyHookPluginSkipsActions::$skipStartAt = 1;
DummyConstrainedHookPlugin::$restriction = null;
DummyConstrainedHookPluginAlt::$restriction = null;
DummyConstrainedPlugin::$restriction = null;
}

protected function tearDown(): void
{
// Reset the static properties on dummy plugins to their defaults.
DummyHookPlugin::$beforeHookCalled = 0;
DummyHookPlugin::$beforeActionCalled = 0;
DummyHookPlugin::$afterActionCalled = 0;
DummyHookPlugin::$afterHookCalled = 0;
DummyHookPluginSkipsActions::$skipStartIn = 'beforeHook';
DummyHookPluginSkipsActions::$skipStartAt = 1;
DummyConstrainedHookPlugin::$restriction = null;
DummyConstrainedHookPluginAlt::$restriction = null;
DummyConstrainedPlugin::$restriction = null;
}

/**
* Tests Hook::getActionRunner
*/
Expand Down Expand Up @@ -154,26 +182,17 @@ public function testRunHookWithPlugins(): void
};
$runner->run();

$this->assertSame(1, $pluginConfig2->getPlugin()->beforeHookCalled);
$this->assertSame(2, $pluginConfig2->getPlugin()->beforeActionCalled);
$this->assertSame(2, $pluginConfig2->getPlugin()->afterActionCalled);
$this->assertSame(1, $pluginConfig2->getPlugin()->afterHookCalled);

$this->assertSame(1, $pluginConfig4->getPlugin()->beforeHookCalled);
$this->assertSame(2, $pluginConfig4->getPlugin()->beforeActionCalled);
$this->assertSame(2, $pluginConfig4->getPlugin()->afterActionCalled);
$this->assertSame(1, $pluginConfig4->getPlugin()->afterHookCalled);

$this->assertSame(0, $pluginConfig5->getPlugin()->beforeHookCalled);
$this->assertSame(0, $pluginConfig5->getPlugin()->beforeActionCalled);
$this->assertSame(0, $pluginConfig5->getPlugin()->afterActionCalled);
$this->assertSame(0, $pluginConfig5->getPlugin()->afterHookCalled);
$this->assertSame(2, DummyHookPlugin::$beforeHookCalled);
$this->assertSame(4, DummyHookPlugin::$beforeActionCalled);
$this->assertSame(4, DummyHookPlugin::$afterActionCalled);
$this->assertSame(2, DummyHookPlugin::$afterHookCalled);
}

public function testRunHookSkipsActionsFromPluginBeforeHook(): void
{
DummyHookPluginSkipsActions::$skipStartAt = 1;

$pluginConfig = new Config\Plugin(DummyHookPluginSkipsActions::class);
$pluginConfig->getPlugin()->skipStartAt = 1;

$config = $this->createConfigMock();
$config->method('failOnFirstError')->willReturn(true);
Expand All @@ -196,10 +215,10 @@ public function testRunHookSkipsActionsFromPluginBeforeHook(): void
};
$runner->run();

$this->assertSame(1, $pluginConfig->getPlugin()->beforeHookCalled);
$this->assertSame(0, $pluginConfig->getPlugin()->beforeActionCalled);
$this->assertSame(0, $pluginConfig->getPlugin()->afterActionCalled);
$this->assertSame(1, $pluginConfig->getPlugin()->afterHookCalled);
$this->assertSame(1, DummyHookPlugin::$beforeHookCalled);
$this->assertSame(0, DummyHookPlugin::$beforeActionCalled);
$this->assertSame(0, DummyHookPlugin::$afterActionCalled);
$this->assertSame(1, DummyHookPlugin::$afterHookCalled);
}

public function testRunHookSkipsActionsFromPluginBeforeAction(): void
Expand All @@ -208,9 +227,10 @@ public function testRunHookSkipsActionsFromPluginBeforeAction(): void
$this->markTestSkipped('not tested on windows');
}

DummyHookPluginSkipsActions::$skipStartIn = 'beforeAction';
DummyHookPluginSkipsActions::$skipStartAt = 3;

$pluginConfig = new Config\Plugin(DummyHookPluginSkipsActions::class);
$pluginConfig->getPlugin()->skipStartIn = 'beforeAction';
$pluginConfig->getPlugin()->skipStartAt = 3;

$config = $this->createConfigMock();
$config->method('failOnFirstError')->willReturn(true);
Expand All @@ -237,9 +257,9 @@ public function testRunHookSkipsActionsFromPluginBeforeAction(): void
};
$runner->run();

$this->assertSame(1, $pluginConfig->getPlugin()->beforeHookCalled);
$this->assertSame(3, $pluginConfig->getPlugin()->beforeActionCalled);
$this->assertSame(2, $pluginConfig->getPlugin()->afterActionCalled);
$this->assertSame(1, $pluginConfig->getPlugin()->afterHookCalled);
$this->assertSame(1, DummyHookPlugin::$beforeHookCalled);
$this->assertSame(3, DummyHookPlugin::$beforeActionCalled);
$this->assertSame(2, DummyHookPlugin::$afterActionCalled);
$this->assertSame(1, DummyHookPlugin::$afterHookCalled);
}
}

0 comments on commit 3844b7f

Please sign in to comment.