Make it easier to configure new tasks

phpro · Dec 1, 2015 · 8d3934a · 8d3934a
1 parent 1b31559
commit 8d3934a
Show file tree

Hide file tree

Showing 39 changed files with 440 additions and 394 deletions.
diff --git a/README.md b/README.md
@@ -119,7 +119,7 @@ parameters:
         succeeded: grumphp-happy.txt
     tasks:
         behat: ~
-        blacklist: ~
+        git_blacklist: ~
         git_commit_message: ~
         phpcsfixer: ~
         phpcs:
@@ -184,7 +184,7 @@ To activate a task, it is sufficient to add an empty task configuration:
 parameters:
     tasks:
         behat: ~
-        blacklist: ~
+        git_blacklist: ~
         phpcsfixer: ~
         phpcs: ~
         phpspec: ~
@@ -228,10 +228,10 @@ If you want to run a particular suite only, specify it with this option.
 
 When this option is enabled, behat will stop at the first error. This means that it will not run your full test suite when an error occurs.
 
-#### Blacklist
+#### Git Blacklist
 
-The Blacklist task will test your changes for blacklisted keywords, such as `die(`, `var_dump(` etc.
-It lives under the `blacklist` namespace and has following configurable parameters:
+The Git Blacklist task will test your changes for blacklisted keywords, such as `die(`, `var_dump(` etc.
+It lives under the `git_blacklist` namespace and has following configurable parameters:
 
 **keywords**
 
@@ -244,14 +244,14 @@ For example:
 # grumphp.yml
 parameters:
     tasks:
-        blacklist:
+        git_blacklist:
             keywords:
                 - "die("
                 - "var_dump("
                 - "exit;"
 ```
 
-#### Git commit message (git_commit_message)
+#### Git commit message
 
 The git comit message can be used in combination with the git hook `git:commit-msg`.
 It can be used to enforce patterns in a commit message.
@@ -481,7 +481,6 @@ services:
         class: My\Custom\Task
         arguments:
           - @config
-          - "@=parameter('tasks')['myConfigKey'] ? parameter('tasks')['myConfigKey'] : []"
         tags:
           - {name: grumphp.task, config: myConfigKey}
 ```

diff --git a/composer.json b/composer.json
@@ -16,7 +16,8 @@
         "symfony/dependency-injection": "^2.4",
         "symfony/config": "^2.4",
         "doctrine/collections": "~1.2",
-        "gitonomy/gitlib": "~0.1.7"
+        "gitonomy/gitlib": "~0.1.7",
+        "symfony/options-resolver": "^2.3"
     },
     "require-dev": {
         "composer/composer": "~1.0@dev",

diff --git a/resources/config/services.yml b/resources/config/services.yml
@@ -18,13 +18,14 @@ services:
     filesystem:
         class: Symfony\Component\Filesystem\Filesystem
 
-    process_builder:
-        class: Symfony\Component\Process\ProcessBuilder
-        prototype: true
-
     executable_finder:
         class: Symfony\Component\Process\ExecutableFinder
 
+    process_builder:
+      class: GrumPHP\Process\ProcessBuilder
+      arguments:
+        - @locator.external_command
+
     git.repository:
         class: Gitonomy\Git\Repository
         arguments:
@@ -51,8 +52,6 @@ services:
         class: GrumPHP\Task\Phpcs
         arguments:
           - @config
-          - "@=parameter('tasks')['phpcs'] ? parameter('tasks')['phpcs'] : []"
-          - @locator.external_command
           - @process_builder
         tags:
           - {name: grumphp.task, config: phpcs}
@@ -61,8 +60,6 @@ services:
         class: GrumPHP\Task\Phpcsfixer
         arguments:
           - @config
-          - "@=parameter('tasks')['phpcsfixer'] ? parameter('tasks')['phpcsfixer'] : []"
-          - @locator.external_command
           - @process_builder
         tags:
           - {name: grumphp.task, config: phpcsfixer}
@@ -71,8 +68,6 @@ services:
         class: GrumPHP\Task\Phpspec
         arguments:
           - @config
-          - "@=parameter('tasks')['phpspec'] ? parameter('tasks')['phpspec'] : []"
-          - @locator.external_command
           - @process_builder
         tags:
           - {name: grumphp.task, config: phpspec}
@@ -81,8 +76,6 @@ services:
         class: GrumPHP\Task\Phpunit
         arguments:
             - @config
-            - "@=parameter('tasks')['phpunit'] ? parameter('tasks')['phpunit'] : []"
-            - @locator.external_command
             - @process_builder
         tags:
             - {name: grumphp.task, config: phpunit}
@@ -91,8 +84,6 @@ services:
         class: GrumPHP\Task\Behat
         arguments:
             - @config
-            - "@=parameter('tasks')['behat'] ? parameter('tasks')['behat'] : []"
-            - @locator.external_command
             - @process_builder
         tags:
             - {name: grumphp.task, config: behat}
@@ -101,26 +92,21 @@ services:
         class: GrumPHP\Task\Git\Blacklist
         arguments:
             - @config
-            - "@=parameter('tasks')['blacklist'] ? parameter('tasks')['blacklist'] : []"
-            - @locator.external_command
             - @process_builder
         tags:
-            - {name: grumphp.task, config: blacklist}
+            - {name: grumphp.task, config: git_blacklist}
 
     task.git.commitmessage:
         class: GrumPHP\Task\Git\CommitMessage
         arguments:
             - @config
-            - "@=parameter('tasks')['git_commit_message'] ? parameter('tasks')['git_commit_message'] : []"
         tags:
             - {name: grumphp.task, config: git_commit_message}
 
     task.codeception:
         class: GrumPHP\Task\Codeception
         arguments:
             - @config
-            - "@=parameter('tasks')['codeception'] ? parameter('tasks')['codeception'] : []"
-            - @locator.external_command
             - @process_builder
         tags:
             - {name: grumphp.task, config: codeception}
diff --git a/spec/GrumPHP/Configuration/GrumPHPSpec.php b/spec/GrumPHP/Configuration/GrumPHPSpec.php
@@ -34,18 +34,6 @@ function it_knows_the_git_dir(ContainerInterface $container)
         $this->getGitDir()->shouldReturn('.');
     }
 
-    function it_provides_a_list_of_active_task_configurations(ContainerInterface $container)
-    {
-        $container->getParameter('tasks')->willReturn(array());
-        $this->getTaskConfig()->shouldReturn(array());
-    }
-
-    function it_can_return_a_particular_task_configuration(ContainerInterface $container)
-    {
-        $container->getParameter('tasks')->willReturn(array('name' => array()));
-        $this->getTaskConfig('name')->shouldReturn(array());
-    }
-
     function it_should_return_empty_ascii_location_for_unknown_resources(ContainerInterface $container)
     {
         $container->getParameter('ascii')->willReturn(array());
@@ -58,11 +46,26 @@ function it_should_return_the_ascii_location_for_known_resources(ContainerInterf
         $this->getAsciiContentPath('success')->shouldReturn('success');
     }
 
-    function it_should_load_available_tasks_from_the_service_container(ContainerBuilder $container, TaskCompilerPass $taskCompiler)
+    function it_should_know_all_registered_tasks(ContainerInterface $container)
     {
-        $container = ContainerFactory::buildFromConfiguration(__DIR__.'/../../../resources/config/services.yml');
-        $taskCompiler->process($container);
-        $this->beConstructedWith($container);
-        $this->getTasks()->shouldBeArray();
+        $container->getParameter('grumphp.tasks.registered')->willReturn(array('phpspec'));
+
+        $this->getRegisteredTasks()->shouldBe(array('phpspec'));
+    }
+
+    function it_should_know_task_configuration(ContainerInterface $container)
+    {
+        $container->getParameter('grumphp.tasks.configuration')->willReturn(array('phpspec' => array()));
+
+        $this->getTaskConfiguration('phpspec')->shouldReturn(array());
+        $this->shouldThrow('GrumPHP\Exception\RuntimeException')->duringGetTaskConfiguration('phpunit');
+    }
+
+    function it_should_know_task_metadata(ContainerInterface $container)
+    {
+        $container->getParameter('grumphp.tasks.metadata')->willReturn(array('phpspec' => array()));
+
+        $this->getTaskMetadata('phpspec')->shouldReturn(array());
+        $this->shouldThrow('GrumPHP\Exception\RuntimeException')->duringGetTaskMetadata('phpunit');
     }
 }
diff --git a/spec/GrumPHP/Locator/ChangedFilesSpec.php b/spec/GrumPHP/Locator/ChangedFilesSpec.php
@@ -22,11 +22,6 @@ function it_is_initializable()
         $this->shouldHaveType('GrumPHP\Locator\ChangedFiles');
     }
 
-    function it_is_a_grumphp_locator()
-    {
-        $this->shouldHaveType('GrumPHP\Locator\LocatorInterface');
-    }
-
     protected function mockFile($name, $isRename = false, $isDelete = false)
     {
         $prophet = new Prophet();

diff --git a/spec/GrumPHP/Locator/ExternalCommandSpec.php b/spec/GrumPHP/Locator/ExternalCommandSpec.php
@@ -18,11 +18,6 @@ function it_is_initializable()
         $this->shouldHaveType('GrumPHP\Locator\ExternalCommand');
     }
 
-    function it_is_a_grumphp_locator()
-    {
-        $this->shouldHaveType('GrumPHP\Locator\LocatorInterface');
-    }
-
     function it_throws_exception_when_external_command_is_not_found(ExecutableFinder $executableFinder)
     {
         $executableFinder->find('test', null, array('bin'))->willReturn(false);

diff --git a/spec/GrumPHP/Locator/RegisteredFilesSpec.php b/spec/GrumPHP/Locator/RegisteredFilesSpec.php
@@ -18,11 +18,6 @@ function it_is_initializable()
         $this->shouldHaveType('GrumPHP\Locator\RegisteredFiles');
     }
 
-    function it_is_a_grumphp_locator()
-    {
-        $this->shouldHaveType('GrumPHP\Locator\LocatorInterface');
-    }
-
     function it_will_list_all_diffed_files(Repository $repository)
     {
         $files = array('file1.txt', 'file2.txt');

diff --git a/spec/GrumPHP/Process/ProcessBuilderSpec.php b/spec/GrumPHP/Process/ProcessBuilderSpec.php
@@ -0,0 +1,40 @@
+<?php
+
+namespace spec\GrumPHP\Process;
+
+use GrumPHP\Collection\ProcessArgumentsCollection;
+use GrumPHP\Locator\ExternalCommand;
+use PhpSpec\ObjectBehavior;
+use Prophecy\Argument;
+
+class ProcessBuilderSpec extends ObjectBehavior
+{
+    function let(ExternalCommand $externalCommandLocator)
+    {
+        $this->beConstructedWith($externalCommandLocator);
+    }
+
+    function it_is_initializable()
+    {
+        $this->shouldHaveType('GrumPHP\Process\ProcessBuilder');
+    }
+
+    function it_should_be_able_to_create_process_arguments_based_on_taskname(ExternalCommand $externalCommandLocator)
+    {
+        $externalCommandLocator->locate('grumphp')->willReturn('/usr/bin/grumphp');
+
+        $arguments = $this->createArgumentsForCommand('grumphp');
+        $arguments->shouldHaveType('GrumPHP\Collection\ProcessArgumentsCollection');
+        $arguments[0]->shouldBe('/usr/bin/grumphp');
+        $arguments->count()->shouldBe(1);
+    }
+
+    function it_should_build_process_based_on_process_arguments()
+    {
+        $arguments = new ProcessArgumentsCollection(array('/usr/bin/grumphp'));
+        $process = $this->buildProcess($arguments);
+
+        $process->shouldHaveType('Symfony\Component\Process\Process');
+        $process->getCommandLine()->shouldBe("'/usr/bin/grumphp'");
+    }
+}
diff --git a/spec/GrumPHP/Task/BehatSpec.php b/spec/GrumPHP/Task/BehatSpec.php
@@ -3,38 +3,33 @@
 namespace spec\GrumPHP\Task;
 
 use GrumPHP\Collection\FilesCollection;
+use GrumPHP\Collection\ProcessArgumentsCollection;
 use GrumPHP\Configuration\GrumPHP;
-use GrumPHP\Locator\LocatorInterface;
+use GrumPHP\Process\ProcessBuilder;
 use GrumPHP\Task\Context\ContextInterface;
 use GrumPHP\Task\Context\GitPreCommitContext;
 use GrumPHP\Task\Context\RunContext;
 use PhpSpec\ObjectBehavior;
 use Prophecy\Argument;
 use Symfony\Component\Process\Process;
-use Symfony\Component\Process\ProcessBuilder;
 use SplFileInfo;
 
 class BehatSpec extends ObjectBehavior
 {
-    function let(GrumPHP $grumPHP, LocatorInterface $externalCommandLocator, ProcessBuilder $processBuilder)
+    function let(GrumPHP $grumPHP, ProcessBuilder $processBuilder)
     {
-        $this->beConstructedWith($grumPHP, array(), $externalCommandLocator, $processBuilder);
+        $grumPHP->getTaskConfiguration('behat')->willReturn(array());
+        $this->beConstructedWith($grumPHP, $processBuilder);
     }
 
     function it_is_initializable()
     {
         $this->shouldHaveType('GrumPHP\Task\Behat');
     }
 
-    function it_is_a_grumphp_external_task()
+    function it_should_have_a_name()
     {
-        $this->shouldHaveType('GrumPHP\Task\ExternalTaskInterface');
-    }
-
-    function it_uses_its_external_command_locator_to_find_correct_command(LocatorInterface $externalCommandLocator)
-    {
-        $externalCommandLocator->locate('behat')->shouldBeCalled();
-        $this->getCommandLocation();
+        $this->getName()->shouldBe('behat');
     }
 
     function it_should_run_in_git_pre_commit_context(GitPreCommitContext $context)
@@ -49,18 +44,18 @@ function it_should_run_in_run_context(RunContext $context)
 
     function it_does_not_do_anything_if_there_are_no_files(ProcessBuilder $processBuilder, ContextInterface $context)
     {
-        $processBuilder->add(Argument::any())->shouldNotBeCalled();
-        $processBuilder->setArguments(Argument::any())->shouldNotBeCalled();
-        $processBuilder->getProcess()->shouldNotBeCalled();
+        $processBuilder->createArgumentsForCommand('behat')->shouldNotBeCalled();
+        $processBuilder->buildProcess()->shouldNotBeCalled();
 
         $context->getFiles()->willReturn(new FilesCollection());
         $this->run($context)->shouldBeNull();
     }
 
     function it_runs_the_suite(ProcessBuilder $processBuilder, Process $process, ContextInterface $context)
     {
-        $processBuilder->setArguments(Argument::type('array'))->shouldBeCalled();
-        $processBuilder->getProcess()->willReturn($process);
+        $arguments = new ProcessArgumentsCollection();
+        $processBuilder->createArgumentsForCommand('behat')->willReturn($arguments);
+        $processBuilder->buildProcess($arguments)->willReturn($process);
 
         $process->run()->shouldBeCalled();
         $process->isSuccessful()->willReturn(true);
@@ -76,8 +71,9 @@ function it_throws_exception_if_the_process_fails(
         Process $process,
         ContextInterface $context
     ) {
-        $processBuilder->setArguments(Argument::type('array'))->shouldBeCalled();
-        $processBuilder->getProcess()->willReturn($process);
+        $arguments = new ProcessArgumentsCollection();
+        $processBuilder->createArgumentsForCommand('behat')->willReturn($arguments);
+        $processBuilder->buildProcess($arguments)->willReturn($process);
 
         $process->run()->shouldBeCalled();
         $process->isSuccessful()->willReturn(false);