Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run PhpCsFixer tasks asynchronously #210

Merged
merged 2 commits into from
Oct 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ parameters:
hooks_preset: local
stop_on_failure: false
ignore_unstaged_changes: false
process_async_limit: 10
process_async_wait: 1000
process_timeout: 60
ascii:
failed: grumphp-grumpy.txt
Expand Down
25 changes: 20 additions & 5 deletions doc/parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ parameters:
hooks_preset: local
stop_on_failure: false
ignore_unstaged_changes: false
process_async_limit: 10
process_async_wait: 1000
process_timeout: 60
ascii:
failed: resource/grumphp-grumpy.txt
Expand Down Expand Up @@ -49,23 +51,23 @@ GrumPHP comes with following presets:
- `vagrant`: All checks will run in your vagrant box.


*Note:*
When using the vagrant preset, you are required to set the vagrant SSH home folder to your working directory.
*Note:*
When using the vagrant preset, you are required to set the vagrant SSH home folder to your working directory.
This can be done by altering the `.bashrc` or `.zshrc` inside your vagrant box:

```sh
echo 'cd /remote/path/to/your/project' >> ~/.bashrc
```

You can also add the `.bashrc` or `.zshrc` to your vagrant provision script.
You can also add the `.bashrc` or `.zshrc` to your vagrant provision script.
This way the home directory will be set for all the people who are using your vagrant box.

**stop_on_failure**

*Default: false*

This parameter will tell GrumPHP to stop running tasks when one of the tasks results in an error.
By default GrumPHP will continue running the configured tasks.
By default GrumPHP will continue running the configured tasks.

**ignore_unstaged_changes**

Expand All @@ -76,6 +78,19 @@ This way the tasks will run with the code that is actually committed without the
Note that during the commit, the unstaged changes will be stored in git stash.
This may mess with your working copy and result in unexpected merge conflicts.

**process_async_limit**

*Default: 10*

This parameter controls how many asynchronous processes GrumPHP will run simultaneously. Please note
that not all external tasks uses asynchronous processes, nor would they necessarily benefit from using it.

**process_async_wait**

*Default: 1000*

This parameter controls how long GrumPHP will wait (in microseconds) before checking the status of all asynchronous processes.

**process_timeout**

*Default: 60*
Expand Down
2 changes: 2 additions & 0 deletions resources/config/parameters.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ parameters:
tasks: []
stop_on_failure: false
ignore_unstaged_changes: false
process_async_limit: 10
process_async_wait: 1000
process_timeout: 60
extensions: []
ascii:
Expand Down
5 changes: 5 additions & 0 deletions resources/config/services.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
services:
async_process_runner:
class: GrumPHP\Process\AsyncProcessRunner
arguments:
- '@config'

config:
class: GrumPHP\Configuration\GrumPHP
arguments:
Expand Down
2 changes: 2 additions & 0 deletions resources/config/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ services:
arguments:
- '@config'
- '@process_builder'
- '@async_process_runner'
- '@formatter.php_cs_fixer'
tags:
- {name: grumphp.task, config: phpcsfixer}
Expand All @@ -171,6 +172,7 @@ services:
arguments:
- '@config'
- '@process_builder'
- '@async_process_runner'
- '@formatter.php_cs_fixer'
tags:
- {name: grumphp.task, config: phpcsfixer2}
Expand Down
12 changes: 12 additions & 0 deletions spec/GrumPHP/Configuration/GrumPHPSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ function it_knows_to_ignore_unstaged_changes(ContainerInterface $container)
$this->ignoreUnstagedChanges()->shouldReturn(true);
}

function it_configures_the_process_async_limit(ContainerInterface $container)
{
$container->getParameter('process_async_limit')->willReturn(5);
$this->getProcessAsyncLimit()->shouldReturn(5);
}

function it_configures_the_process_async_wait_time(ContainerInterface $container)
{
$container->getParameter('process_async_wait')->willReturn(0);
$this->getProcessAsyncWaitTime()->shouldReturn(0);
}

function it_configures_the_symfony_process_timeout(ContainerInterface $container)
{
$container->getParameter('process_timeout')->willReturn(null);
Expand Down
63 changes: 63 additions & 0 deletions spec/GrumPHP/Process/AsyncProcessRunnerSpec.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace spec\GrumPHP\Process;

use GrumPHP\Configuration\GrumPHP;
use GrumPHP\Process\AsyncProcessRunner;
use PhpSpec\ObjectBehavior;
use Prophecy\Prophet;

/**
* @mixin AsyncProcessRunner
*/
class AsyncProcessRunnerSpec extends ObjectBehavior
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's a good idea to test the async process limit part as well?

{
public function let(GrumPHP $grumPHP) {
$this->beConstructedWith($grumPHP);

$grumPHP->getProcessAsyncWaitTime()->willReturn(0);
$grumPHP->getProcessAsyncLimit()->willReturn(5);
}

function it_is_initializable()
{
$this->shouldHaveType('GrumPHP\Process\AsyncProcessRunner');
}

function it_should_be_able_to_run_processes()
{
$prophet = new Prophet();
$processes = array();

for ($i = 0; $i < 20; $i++) {
$process = $prophet->prophesize('Symfony\Component\Process\Process');

$process->started = false;
$process->terminated = false;

$process->start()->will(function () use ($process) {
$process->started = true;
})->shouldBeCalledTimes(1);

$process->isTerminated()->will(function () use ($process) {
if (!$process->terminated) {
$process->terminated = true;
return false;
}

return true;
})->shouldBeCalledTimes(2);

// The number of times isStarted() is called starts at 3
// and increases by 2 after each chunk of five processes.
$process->isStarted()->will(function () use ($process) {
return $process->started;
})->shouldBeCalledTimes(floor($i / 5) * 2 + 3);

$processes[] = $process->reveal();
}

$this->run($processes);
$prophet->checkPredictions();
}
}
15 changes: 9 additions & 6 deletions spec/GrumPHP/Task/PhpCsFixerSpec.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use GrumPHP\Collection\ProcessArgumentsCollection;
use GrumPHP\Configuration\GrumPHP;
use GrumPHP\Formatter\PhpCsFixerFormatter;
use GrumPHP\Process\AsyncProcessRunner;
use GrumPHP\Process\ProcessBuilder;
use GrumPHP\Runner\TaskResult;
use GrumPHP\Task\Context\ContextInterface;
Expand All @@ -23,15 +24,15 @@
class PhpCsFixerSpec extends ObjectBehavior
{

function let(GrumPHP $grumPHP, ProcessBuilder $processBuilder, PhpCsFixerFormatter $formatter)
function let(GrumPHP $grumPHP, ProcessBuilder $processBuilder, AsyncProcessRunner $processRunner, PhpCsFixerFormatter $formatter)
{
$grumPHP->getTaskConfiguration('phpcsfixer')->willReturn(array());

$formatter->format(Argument::any())->willReturn('');
$formatter->formatSuggestion(Argument::any())->willReturn('');
$formatter->formatErrorMessage(Argument::cetera())->willReturn('');

$this->beConstructedWith($grumPHP, $processBuilder, $formatter);
$this->beConstructedWith($grumPHP, $processBuilder, $processRunner, $formatter);
}

function it_is_initializable()
Expand Down Expand Up @@ -106,6 +107,7 @@ function it_runs_the_suite_for_all_files(

function it_runs_the_suite_for_changed_files(
ProcessBuilder $processBuilder,
AsyncProcessRunner $processRunner,
Process $process,
ContextInterface $context,
PhpCsFixerFormatter $formatter
Expand All @@ -121,8 +123,8 @@ function it_runs_the_suite_for_changed_files(
return $args->contains($file1) || $args->contains($file2);
}))->willReturn($process);

$process->run()->shouldBeCalled();
$process->isSuccessful()->willReturn(true);
$processRunner->run(Argument::type('array'))->shouldBeCalled();
$process->isSuccessful()->willReturn(true);

$result = $this->run($context);
$result->shouldBeAnInstanceOf('GrumPHP\Runner\TaskResultInterface');
Expand All @@ -131,6 +133,7 @@ function it_runs_the_suite_for_changed_files(

function it_throws_exception_if_the_process_fails(
ProcessBuilder $processBuilder,
AsyncProcessRunner $processRunner,
Process $process,
ContextInterface $context,
PhpCsFixerFormatter $formatter
Expand All @@ -141,8 +144,8 @@ function it_throws_exception_if_the_process_fails(
$processBuilder->createArgumentsForCommand('php-cs-fixer')->willReturn($arguments);
$processBuilder->buildProcess(Argument::type('GrumPHP\Collection\ProcessArgumentsCollection'))->willReturn($process);

$process->run()->shouldBeCalled();
$process->isSuccessful()->willReturn(false);
$processRunner->run(Argument::type('array'))->shouldBeCalled();
$process->isSuccessful()->willReturn(false);

$context->getFiles()->willReturn(new FilesCollection(array(
new SplFileInfo('file1.php', '.', 'file1.php'),
Expand Down
15 changes: 9 additions & 6 deletions spec/GrumPHP/Task/PhpCsFixerV2Spec.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use GrumPHP\Collection\ProcessArgumentsCollection;
use GrumPHP\Configuration\GrumPHP;
use GrumPHP\Formatter\PhpCsFixerFormatter;
use GrumPHP\Process\AsyncProcessRunner;
use GrumPHP\Process\ProcessBuilder;
use GrumPHP\Runner\TaskResult;
use GrumPHP\Task\Context\ContextInterface;
Expand All @@ -23,15 +24,15 @@
class PhpCsFixerV2Spec extends ObjectBehavior
{

function let(GrumPHP $grumPHP, ProcessBuilder $processBuilder, PhpCsFixerFormatter $formatter)
function let(GrumPHP $grumPHP, ProcessBuilder $processBuilder, AsyncProcessRunner $processRunner, PhpCsFixerFormatter $formatter)
{
$grumPHP->getTaskConfiguration('phpcsfixer2')->willReturn(array());

$formatter->format(Argument::any())->willReturn('');
$formatter->formatSuggestion(Argument::any())->willReturn('');
$formatter->formatErrorMessage(Argument::cetera())->willReturn('');

$this->beConstructedWith($grumPHP, $processBuilder, $formatter);
$this->beConstructedWith($grumPHP, $processBuilder, $processRunner, $formatter);
}

function it_is_initializable()
Expand Down Expand Up @@ -108,6 +109,7 @@ function it_runs_the_suite_for_all_files(

function it_runs_the_suite_for_changed_files(
ProcessBuilder $processBuilder,
AsyncProcessRunner $processRunner,
Process $process,
ContextInterface $context,
PhpCsFixerFormatter $formatter
Expand All @@ -123,8 +125,8 @@ function it_runs_the_suite_for_changed_files(
return $args->contains($file1) || $args->contains($file2);
}))->willReturn($process);

$process->run()->shouldBeCalled();
$process->isSuccessful()->willReturn(true);
$processRunner->run(Argument::type('array'))->shouldBeCalled();
$process->isSuccessful()->willReturn(true);

$result = $this->run($context);
$result->shouldBeAnInstanceOf('GrumPHP\Runner\TaskResultInterface');
Expand All @@ -133,6 +135,7 @@ function it_runs_the_suite_for_changed_files(

function it_throws_exception_if_the_process_fails(
ProcessBuilder $processBuilder,
AsyncProcessRunner $processRunner,
Process $process,
ContextInterface $context,
PhpCsFixerFormatter $formatter
Expand All @@ -143,8 +146,8 @@ function it_throws_exception_if_the_process_fails(
$processBuilder->createArgumentsForCommand('php-cs-fixer')->willReturn($arguments);
$processBuilder->buildProcess(Argument::type('GrumPHP\Collection\ProcessArgumentsCollection'))->willReturn($process);

$process->run()->shouldBeCalled();
$process->isSuccessful()->willReturn(false);
$processRunner->run(Argument::type('array'))->shouldBeCalled();
$process->isSuccessful()->willReturn(false);

$context->getFiles()->willReturn(new FilesCollection(array(
new SplFileInfo('file1.php', '.', 'file1.php'),
Expand Down
16 changes: 16 additions & 0 deletions src/GrumPHP/Configuration/GrumPHP.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ public function ignoreUnstagedChanges()
return (bool) $this->container->getParameter('ignore_unstaged_changes');
}

/**
* @return int
*/
public function getProcessAsyncLimit()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you ass a spec for this method?

{
return (int) $this->container->getParameter('process_async_limit');
}

/**
* @return int
*/
public function getProcessAsyncWaitTime()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you ass a spec for this method?

{
return (int) $this->container->getParameter('process_async_wait');
}

/**
* @return float|null
*/
Expand Down
Loading