Skip to content

Commit

Permalink
Merge pull request #20 from MontealegreLuis/master
Browse files Browse the repository at this point in the history
Fix validation of 'git' and 'files' options
  • Loading branch information
jmolivas committed Oct 28, 2015
2 parents d206afc + b348a7c commit c981e06
Show file tree
Hide file tree
Showing 6 changed files with 251 additions and 33 deletions.
23 changes: 23 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0" encoding="UTF-8"?>

<phpunit backupGlobals="false"
backupStaticAttributes="false"
colors="true"
convertErrorsToExceptions="true"
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnFailure="false"
syntaxCheck="false"
bootstrap="vendor/autoload.php">

<testsuites>
<testsuite name="Unit tests">
<directory>tests/unit</directory>
</testsuite>
<testsuite name="Integration tests">
<directory>tests/integration</directory>
</testsuite>
</testsuites>

</phpunit>
40 changes: 18 additions & 22 deletions src/Command/AnalyzeCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace JMOlivas\Phpqa\Command;

use Exception;
use JMOlivas\Phpqa\Input\FilesOption;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
Expand Down Expand Up @@ -46,7 +48,7 @@ protected function configure()
'git',
null,
InputOption::VALUE_NONE,
'All files added to git index will be analyze.'
'All files added to git index will be analyzed.'
);
}

Expand All @@ -65,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$config = $application->getConfig();

if (!$config->isCustom() && !$project) {
throw new \Exception(
throw new Exception(
sprintf(
'No local phpqa.yml or phpqa.yml.dist at current working directory ' .
'you must provide a valid project value (%s)',
Expand All @@ -75,7 +77,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
}

if (!$config->isCustom() && !in_array($project, $this->projects)) {
throw new \Exception(
throw new Exception(
sprintf(
'You must provide a valid project value (%s)',
implode(',', $this->projects)
Expand All @@ -89,31 +91,25 @@ protected function execute(InputInterface $input, OutputInterface $output)

$output->writeln(sprintf('<question>%s</question>', $application->getName()));

$files = $input->getOption('files');
$filesOption = new FilesOption($input->getOption('files'));
$git = $input->getOption('git');

$git = false;
if ($input->hasOption('git')) {
$git = $input->getOption('git');
if (!$filesOption->isAbsent() && $git) {
throw new Exception('Options `files` and `git` cannot be used in combination.');
}

if ($files && $git) {
throw new \Exception('Options `files` and `git` can not used in combination.');
if ($filesOption->isAbsent() && !$git) {
throw new Exception('You must set `files` or `git` options.');
}

if ($files) {
$files = explode(',', $files[0]);
}

if (!$files[0]) {
$files = [];
}

if (!$files && !$git) {
throw new \Exception('You must set `files` or `git` options.');
if (!$filesOption->isAbsent() && $filesOption->isEmpty()) {
throw new Exception('Options `files` needs at least one file.');
}

if ($git) {
$files = $this->extractCommitedFiles($output, $config);
} else {
$files = $filesOption->normalize();
}

$output->writeln(
Expand Down Expand Up @@ -202,7 +198,7 @@ private function checkComposer($output, $files, $config)

if ($config->get('application.method.composer.exception')) {
if ($composerJsonDetected && !$composerLockDetected) {
throw new \Exception($config->get('application.messages.composer.error'));
throw new Exception($config->get('application.messages.composer.error'));
}

$output->writeln(
Expand Down Expand Up @@ -269,7 +265,7 @@ private function analyzer($output, $analyzer, $files, $config, $project)
}

if ($exception && !$success) {
throw new \Exception($config->get('application.messages.'.$analyzer.'.error'));
throw new Exception($config->get('application.messages.'.$analyzer.'.error'));
}
}

Expand Down Expand Up @@ -312,7 +308,7 @@ public function executeProcess($output, $processArguments, $file, $prefixes, $po
private function validateBinary($binaryFile)
{
if (!file_exists($this->directory.$binaryFile)) {
throw new \Exception(
throw new Exception(
sprintf('%s do not exist!', $binaryFile)
);
}
Expand Down
18 changes: 7 additions & 11 deletions src/Console/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,25 @@ class Application extends BaseApplication
*/
private $config;

/**
* @return \JMOlivas\Phpqa\Config
*/
public function getConfig()
public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
{
return $this->config;
parent::__construct($name, $version);
$this->config = new Config();
}

/**
* {@inheritdoc}
* @return \JMOlivas\Phpqa\Config
*/
public function doRun(InputInterface $input, OutputInterface $output)
public function getConfig()
{
$this->config = new Config();

parent::doRun($input, $output);
return $this->config;
}

/**
* @return string
*/
public function getApplicationDirectory()
{
return __DIR__.'/../../';
return __DIR__ . '/../../';
}
}
58 changes: 58 additions & 0 deletions src/Input/FilesOption.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace JMOlivas\Phpqa\Input;

class FilesOption
{
/** @var array */
private $files;

/**
* @param array $files
*/
public function __construct(array $files)
{
$this->files = $files;
}

/**
* Returns true if this option is provided but has no values
*
* @return bool
*/
public function isEmpty()
{
return count($this->files) === 1 && $this->files[0] === null;
}

/**
* Returns true if this option is not provided
*
* @return bool
*/
public function isAbsent()
{
return empty($this->files);
}

/**
* Normalize the provided values as an array
*
* - If it's either empty or absent, it returns an empty array
* - If it's a single value separated by commas, it converts it to array
* - Otherwise returns the value as is.
*
* @return array
*/
public function normalize()
{
if ($this->isAbsent() || $this->isEmpty()) {
return [];
}
if (count($this->files) === 1 && strpos($this->files[0], ',') !== false) {
return explode(',', $this->files[0]);
}

return $this->files;
}
}
63 changes: 63 additions & 0 deletions tests/integration/Command/AnalyzeCommandTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace JMOlivas\Phpqa\Command;

use JMOlivas\Phpqa\Console\Application;
use PHPUnit_Framework_TestCase as TestCase;
use Symfony\Component\Console\Tester\CommandTester;

class AnalyzeCommandTest extends TestCase
{
/**
* @test
* @expectedException \Exception
* @expectedExceptionMessage You must set `files` or `git` options.
*/
function it_should_throw_exception_if_neither_files_nor_git_options_are_provided()
{
$application = new Application();
$command = new AnalyzeCommand();
$command->setApplication($application);

$tester = new CommandTester($command);

$tester->execute([]);
}

/**
* @test
* @expectedException \Exception
* @expectedExceptionMessage Options `files` and `git` cannot be used in combination.
*/
function it_should_throw_exception_if_both_files_and_git_options_are_provided()
{
$application = new Application();
$command = new AnalyzeCommand();
$command->setApplication($application);

$tester = new CommandTester($command);

$tester->execute([
'--files' => [null],
'--git' => true
]);
}

/**
* @test
* @expectedException \Exception
* @expectedExceptionMessage Options `files` needs at least one file.
*/
function it_should_throw_exception_if_files_is_provided_but_it_is_empty()
{
$application = new Application();
$command = new AnalyzeCommand();
$command->setApplication($application);

$tester = new CommandTester($command);

$tester->execute([
'--files' => [null],
]);
}
}
82 changes: 82 additions & 0 deletions tests/unit/Input/FilesOptionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php

namespace JMOlivas\Phpqa\Input;

use PHPUnit_Framework_TestCase as TestCase;

class FilesOptionTest extends TestCase
{
/** @test */
function it_should_recognize_if_option_is_absent()
{
$absentInput = [];
$files = new FilesOption($absentInput);

$this->assertTrue($files->isAbsent());
}

/** @test */
function it_should_recognize_if_option_is_provided_but_is_empty()
{
$emptyInput = [null];
$files = new FilesOption($emptyInput);

$this->assertTrue($files->isEmpty());
}

/** @test */
function it_should_recognize_if_option_is_provided_correctly()
{
$validInput = ['src/'];
$files = new FilesOption($validInput);

$this->assertFalse($files->isAbsent());
$this->assertFalse($files->isEmpty());
}

/** @test */
function it_should_normalize_input_separated_by_commas()
{
// bin/phpqa analyze --files=src/,test/
$singleInputWithMultipleValues = ['src/,test/'];
$files = new FilesOption($singleInputWithMultipleValues);

$values = $files->normalize();

$this->assertCount(2, $values);
$this->assertEquals('src/', $values[0]);
$this->assertEquals('test/', $values[1]);
}

/** @test */
function it_should_return_multiple_files_input_as_is()
{
// bin/phpqa analyze --files=src/ --files=test/
$singleInputWithMultipleValues = ['src/','test/'];
$files = new FilesOption($singleInputWithMultipleValues);

$values = $files->normalize();

$this->assertCount(2, $values);
$this->assertEquals('src/', $values[0]);
$this->assertEquals('test/', $values[1]);
}

/** @test */
function it_should_return_empty_array_if_input_is_absent()
{
$absentInput = [];
$files = new FilesOption($absentInput);

$this->assertCount(0, $files->normalize());
}

/** @test */
function it_should_return_empty_array_if_input_is_empty()
{
$emptyInput = [null];
$files = new FilesOption($emptyInput);

$this->assertCount(0, $files->normalize());
}
}

0 comments on commit c981e06

Please sign in to comment.