Skip to content

Commit 2b247df

Browse files
sebastianfeldmannsebastianfeldmann
committed
Handle restricted actions separately
Make sure that actions that are not executed because of hook restrictions are marked as skipped instead of done.
1 parent e55e58e commit 2b247df

File tree

6 files changed

+190
-4
lines changed

6 files changed

+190
-4
lines changed

‎src/Exception/ActionNotApplicable.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CaptainHook
5+
*
6+
* (c) Sebastian Feldmann <sf@sebastian-feldmann.info>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace CaptainHook\App\Exception;
13+
14+
use Exception;
15+
16+
/**
17+
* Class ActionNotApplicable
18+
*
19+
* @package CaptainHook
20+
* @author Sebastian Feldmann <sf@sebastian-feldmann.info>
21+
* @link https://github.com/captainhook-git/captainhook
22+
* @since Class available since Release 5.26.1
23+
*/
24+
class ActionNotApplicable extends Exception implements CaptainHookException
25+
{
26+
}

‎src/Runner/Action/PHP.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use CaptainHook\App\Console\IO;
1616
use CaptainHook\App\Event\Dispatcher;
1717
use CaptainHook\App\Exception\ActionFailed;
18+
use CaptainHook\App\Exception\ActionNotApplicable;
1819
use CaptainHook\App\Hook\Action;
1920
use CaptainHook\App\Hook\Constrained;
2021
use CaptainHook\App\Hook\EventSubscriber;
@@ -69,6 +70,7 @@ public function __construct(string $hook, Dispatcher $dispatcher)
6970
* @param \CaptainHook\App\Config\Action $action
7071
* @return void
7172
* @throws \CaptainHook\App\Exception\ActionFailed
73+
* @throws \CaptainHook\App\Exception\ActionNotApplicable
7274
*/
7375
public function execute(Config $config, IO $io, Repository $repository, Config\Action $action): void
7476
{
@@ -85,8 +87,7 @@ public function execute(Config $config, IO $io, Repository $repository, Config\A
8587
$exe = $this->createAction($class);
8688
// check for any given restrictions
8789
if (!$this->isApplicable($exe)) {
88-
$io->write('Action skipped due to hook constraint', true, IO::VERBOSE);
89-
return;
90+
throw new ActionNotApplicable('Action can\'t be used for this hook');
9091
}
9192

9293
// make sure to collect all event handlers before executing the action
@@ -96,7 +97,7 @@ public function execute(Config $config, IO $io, Repository $repository, Config\A
9697

9798
// no restrictions run it!
9899
$exe->execute($config, $io, $repository, $action);
99-
} catch (ActionFailed $e) {
100+
} catch (ActionNotApplicable | ActionFailed $e) {
100101
throw $e;
101102
} catch (Exception $e) {
102103
throw new ActionFailed(

‎src/Runner/Hook.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use CaptainHook\App\Console\IO;
1616
use CaptainHook\App\Event\Dispatcher;
1717
use CaptainHook\App\Exception\ActionFailed;
18+
use CaptainHook\App\Exception\ActionNotApplicable;
1819
use CaptainHook\App\Hook\Constrained;
1920
use CaptainHook\App\Hook\Template\Inspector;
2021
use CaptainHook\App\Plugin;
@@ -336,7 +337,10 @@ private function handleAction(Config\Action $action): void
336337
$runner = $this->createActionRunner(Util::getExecType($action->getAction()));
337338
$runner->execute($this->config, $io, $this->repository, $action);
338339
$this->printer->actionSucceeded($action);
339-
} catch (Exception $e) {
340+
} catch (ActionNotApplicable $e) {
341+
$this->printer->actionSkipped($action);
342+
return;
343+
} catch (Exception $e) {
340344
$status = ActionLog::ACTION_FAILED;
341345
$this->printer->actionFailed($action);
342346
if (!$action->isFailureAllowed($this->config->isFailureAllowed())) {

‎tests/unit/Console/IO/TestIO.php‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CaptainHook.
5+
*
6+
* (c) Sebastian Feldmann <sf@sebastian-feldmann.info>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace CaptainHook\App\Console\IO;
13+
14+
use CaptainHook\App\Console\IO;
15+
16+
class TestIO implements IO
17+
{
18+
private array $log = [];
19+
private array $err = [];
20+
21+
/**
22+
* @inheritDoc
23+
*/
24+
public function getArguments(): array
25+
{
26+
return [];
27+
}
28+
29+
/**
30+
* @inheritDoc
31+
*/
32+
public function getArgument(string $name, string $default = ''): string
33+
{
34+
return '';
35+
}
36+
37+
/**
38+
* @inheritDoc
39+
*/
40+
public function getStandardInput(): array
41+
{
42+
return [];
43+
}
44+
45+
/**
46+
* @inheritDoc
47+
*/
48+
public function isInteractive()
49+
{
50+
return false;
51+
}
52+
53+
/**
54+
* @inheritDoc
55+
*/
56+
public function isVerbose()
57+
{
58+
return false;
59+
}
60+
61+
/**
62+
* @inheritDoc
63+
*/
64+
public function isVeryVerbose()
65+
{
66+
return false;
67+
}
68+
69+
/**
70+
* @inheritDoc
71+
*/
72+
public function isDebug()
73+
{
74+
return false;
75+
}
76+
77+
/**
78+
* @inheritDoc
79+
*/
80+
public function write($messages, $newline = true, $verbosity = self::NORMAL)
81+
{
82+
$this->log[] = (is_array($messages) ? implode(PHP_EOL, $messages) : $messages) . ($newline ? PHP_EOL : '');
83+
}
84+
85+
public function getLog(): array
86+
{
87+
return $this->log;
88+
}
89+
90+
/**
91+
* @inheritDoc
92+
*/
93+
public function writeError($messages, $newline = true, $verbosity = self::NORMAL)
94+
{
95+
$this->err[] = $messages . ($newline ? PHP_EOL : '');
96+
}
97+
98+
public function getErr(): array
99+
{
100+
return $this->err;
101+
}
102+
103+
/**
104+
* @inheritDoc
105+
*/
106+
public function ask($question, $default = null)
107+
{
108+
return '';
109+
}
110+
111+
/**
112+
* @inheritDoc
113+
*/
114+
public function askConfirmation($question, $default = true)
115+
{
116+
return true;
117+
}
118+
119+
/**
120+
* @inheritDoc
121+
*/
122+
public function askAndValidate($question, $validator, $attempts = null, $default = null)
123+
{
124+
return '';
125+
}
126+
}

‎tests/unit/Runner/Action/PHPTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use CaptainHook\App\Config\Mockery as ConfigMockery;
1515
use CaptainHook\App\Console\IO\Mockery as IOMockery;
1616
use CaptainHook\App\Event\Dispatcher;
17+
use CaptainHook\App\Exception\ActionNotApplicable;
1718
use CaptainHook\App\Mockery as CHMockery;
1819
use Exception;
1920
use PHPUnit\Framework\TestCase;
@@ -71,6 +72,8 @@ public function testExecuteConstraintApplicable(): void
7172

7273
public function testExecuteConstraintNotApplicable(): void
7374
{
75+
$this->expectException(ActionNotApplicable::class);
76+
7477
$config = $this->createConfigMock();
7578
$io = $this->createIOMock();
7679
$repo = $this->createRepositoryMock();

‎tests/unit/Runner/HookTest.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use CaptainHook\App\Config;
1515
use CaptainHook\App\Config\Mockery as ConfigMockery;
1616
use CaptainHook\App\Console\IO\Mockery as IOMockery;
17+
use CaptainHook\App\Console\IO\TestIO;
1718
use CaptainHook\App\Git\DummyRepo;
1819
use CaptainHook\App\Hook\Restriction;
1920
use CaptainHook\App\Hooks;
@@ -117,6 +118,31 @@ public function testShouldSkipActionsCanBeSetToFalse(): void
117118
$this->assertFalse($runner->shouldSkipActions());
118119
}
119120

121+
public function testActionNotApplicable(): void
122+
{
123+
$config = $this->createConfigMock();
124+
$config->method('failOnFirstError')->willReturn(true);
125+
$dummy = new DummyRepo(['hooks' => ['pre-commit' => '# hook script']]);
126+
$repo = $this->createRepositoryMock($dummy->getRoot());
127+
$repo->method('getHooksDir')->willReturn($dummy->getHookDir());
128+
129+
$io = new TestIO();
130+
$hookConfig = $this->createHookConfigMock();
131+
$actionConfig = $this->createActionConfigMock();
132+
$actionConfig->method('getAction')->willReturn("\\CaptainHook\\App\\Hook\\Message\\Action\\Beams");
133+
$hookConfig->method('isEnabled')->willReturn(true);
134+
$hookConfig->expects($this->once())->method('getActions')->willReturn([$actionConfig]);
135+
$config->expects($this->once())->method('getHookConfigToExecute')->willReturn($hookConfig);
136+
$config->expects($this->atLeastOnce())->method('isHookEnabled')->willReturn(true);
137+
138+
$runner = new class ($io, $config, $repo) extends Hook {
139+
protected string $hook = Hooks::PRE_PUSH;
140+
};
141+
$runner->run();
142+
143+
$this->assertStringContainsString('skipped', implode($io->getLog()));
144+
}
145+
120146
public function testRunHookWithPlugins(): void
121147
{
122148
if (defined('PHP_WINDOWS_VERSION_MAJOR')) {

0 commit comments

Comments
 (0)