Skip to content

[Console] Improve code coverage #19328

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

Closed
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 src/Symfony/Component/Console/Command/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,8 @@ public function getSynopsis($short = false)
* Add a command usage example.
*
* @param string $usage The usage, it'll be prefixed with the command name
*
* @return $this
Copy link
Member

Choose a reason for hiding this comment

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

we are using @return self everywhere else. Not sure which one is the right one, but I didn't find any occurrence of @return $this in Symfony codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

that is because of my suggestion I'm afraid
#19328 (comment)
sorry if misleading the pr!

*/
public function addUsage($usage)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Console/Helper/TableStyle.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class TableStyle
public function setPaddingChar($paddingChar)
{
if (!$paddingChar) {
throw new LogicException('The padding char must not be empty');
throw new LogicException('The padding char must not be empty.');
}

$this->paddingChar = $paddingChar;
Expand Down
22 changes: 22 additions & 0 deletions src/Symfony/Component/Console/Tests/Command/CommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,28 @@ public function testGetSetAliases()
$this->assertEquals(array('name1'), $command->getAliases(), '->setAliases() sets the aliases');
}

/**
* @expectedException \Symfony\Component\Console\Exception\InvalidArgumentException
* @expectedExceptionMessage $aliases must be an array or an instance of \Traversable
*/
public function testSetAliasesThrowsInvalidArgumentException()
{
$command = new \TestCommand();

$command->setAliases('foo');
}

public function testAddUsage()
{
$command = new \TestCommand();

$command
->addUsage('foo')
->addUsage('bar');

$this->assertSame(array('namespace:name foo', 'namespace:name bar'), $command->getUsages());
}

public function testGetSynopsis()
{
$command = new \TestCommand();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ public function testDescribeInputDefinition(InputDefinition $definition, $expect
$this->assertDescription($expectedDescription, $definition);
}

/**
* @expectedException \Symfony\Component\Console\Exception\InvalidArgumentException
* @expectedExceptionMessage Object of type "stdClass" is not describable.
*/
public function testDescribeCommandInvalidObjects()
{
$this->assertDescription(null, new \stdClass());
}

/** @dataProvider getDescribeCommandTestData */
public function testDescribeCommand(Command $command, $expectedDescription)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

Copy link
Contributor

Choose a reason for hiding this comment

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

namespace is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No namespaces were introduced for all command_*.php files so IMHO we should let it this way.

use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

// ensure that all lines are aligned to the begin of the first one and start with '//' in a very long line comment
return function (InputInterface $input, OutputInterface $output) {
$output = new SymfonyStyle($input, $output);
$output->section('Lorem ipsum dolor sit amet');
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

// ensure that all lines are aligned to the begin of the first one and start with '//' in a very long line comment
return function (InputInterface $input, OutputInterface $output) {
$output = new SymfonyStyle($input, $output);

try {
$output->progressFinish();
} catch (RuntimeException $e) {
$output->writeln('OK');

return;
}

$output->writeln('KO');
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

Lorem ipsum dolor sit amet
--------------------------

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
OK
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
0/10 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0%
4/10 [▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░] 40%
10/10 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
0/10 [>---------------------------] 0%
4/10 [===========>----------------] 40%
10/10 [============================] 100%

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Tests\Helper;

use Symfony\Component\Console\Helper\DescriptorHelper;
use Symfony\Component\Console\Output\OutputInterface;

class DescriptorHelperTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \Symfony\Component\Console\Exception\InvalidArgumentException
* @expectedExceptionMessage Unsupported format "invalid".
*/
public function testDescribeWithInvalidFormat()
{
$descriptor = new DescriptorHelper();
$descriptor->describe($this->getMock(OutputInterface::class), null, array('format' => 'invalid'));
}

public function testGetName()
{
$descriptor = new DescriptorHelper();

$this->assertSame('descriptor', $descriptor->getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

How it possible to assertSame $descriptor->getName() == descriptor because here $descriptor is null object.I think you have to setName first.

Copy link
Contributor

@SpacePossum SpacePossum Jul 19, 2016

Choose a reason for hiding this comment

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

is null object

$descriptor = new DescriptorHelper(); so how is that possible?

the name will always be set, so the test is valid to make sure of that to prevent regression
https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Console/Helper/DescriptorHelper.php#L92

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,14 @@ public function testTruncatingMessageWithCustomSuffix()
$this->assertSame('test!', $formatter->truncate($message, 4, '!'));
}

public function testTruncatingMessageWithInvalidEncoding()
{
$formatter = new FormatterHelper();
$message = str_repeat(chr(-1), 4);

$this->assertSame(str_repeat(chr(-1), 2).'!', $formatter->truncate($message, 2, '!'));
}

public function testTruncatingWithLongerLengthThanMessageWithSuffix()
{
$formatter = new FormatterHelper();
Expand Down
26 changes: 26 additions & 0 deletions src/Symfony/Component/Console/Tests/Helper/HelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@

class HelperTest extends \PHPUnit_Framework_TestCase
{
public function formatMemoryProvider()
{
return array(
array(0, '0 B'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this kind of alignment allowed by the CS standards?

Copy link
Contributor

Choose a reason for hiding this comment

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

array_element_white_space_after_comma [symfony]
In array declaration, there MUST be a whitespace after each comma.

(https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/1.11/README.rst)
so no rule about how much whitespace other than something ^_^

Copy link
Contributor

Choose a reason for hiding this comment

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

I know there are many implicit rules that aren't yet declared @SpacePossum, that's why I'm asking. The reasons I have in mind are the very same that moved us to unalign = and => in the past (#14438), given the code maintainability turns harder when any update to this "structures" is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@phansys I formatted the array this way to be consistent with what's already in this file (take a look at the formatTimeProvider function).

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure @geoffrey-brier, thanks for pointing and putting the sight on this.

array(1, '1 B'),
array(1000, '1000 B'),
array(1024, '1 KiB'),
array(1025, '1 KiB'),
array(1048576, '1.0 MiB'),
array(1048577, '1.0 MiB'),
array(1073741824, '1.0 GiB'),
array(1073741825, '1.0 GiB'),
);
}

public function formatTimeProvider()
{
return array(
Expand Down Expand Up @@ -51,4 +66,15 @@ public function testFormatTime($secs, $expectedFormat)
{
$this->assertEquals($expectedFormat, Helper::formatTime($secs));
}

/**
* @dataProvider formatMemoryProvider
*
* @param int $bytes
* @param string $expectedFormat
*/
public function testFormatMemory($bytes, $expectedFormat)
{
$this->assertEquals($expectedFormat, Helper::formatMemory($bytes));
}
}
22 changes: 22 additions & 0 deletions src/Symfony/Component/Console/Tests/Helper/ProcessHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,28 @@ public function testVariousProcessRuns($expected, $cmd, $verbosity, $error)
$this->assertEquals($expected, $this->getOutput($output));
}

public function testMustRun()
{
$helper = new ProcessHelper();
$helper->setHelperSet(new HelperSet(array(new DebugFormatterHelper())));
$output = $this->getOutputStream(StreamOutput::VERBOSITY_VERBOSE);
$helper->mustRun($output, 'php -r "echo 42;"', null);

$this->assertSame('', $this->getOutput($output));
}

/**
* @expectedException \Symfony\Component\Process\Exception\ProcessFailedException
* @expectedExceptionMessage The command "php -r "echo 42"" failed.
*/
public function testMustRunThrowsProcessFailedException()
{
$helper = new ProcessHelper();
$helper->setHelperSet(new HelperSet(array(new DebugFormatterHelper())));
$output = $this->getOutputStream(StreamOutput::VERBOSITY_VERBOSE);
$helper->mustRun($output, 'php -r "echo 42"', null);
}

public function testPassedCallbackIsExecuted()
{
$helper = new ProcessHelper();
Expand Down
24 changes: 22 additions & 2 deletions src/Symfony/Component/Console/Tests/Helper/QuestionHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Console\Tests\Helper;

use Symfony\Component\Console\Exception\InvalidArgumentException;
use Symfony\Component\Console\Formatter\OutputFormatter;
use Symfony\Component\Console\Helper\QuestionHelper;
use Symfony\Component\Console\Helper\HelperSet;
Expand All @@ -34,7 +35,7 @@ public function testAskChoice()

$heroes = array('Superman', 'Batman', 'Spiderman');

$inputStream = $this->getInputStream("\n1\n 1 \nFabien\n1\nFabien\n1\n0,2\n 0 , 2 \n\n\n");
$inputStream = $this->getInputStream("\n1\n 1 \nFabien\n1\nFabien\né\n1\n0,2\n 0 , 2 \n\n\n\n");

$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, '2');
$question->setMaxAttempts(1);
Expand Down Expand Up @@ -64,6 +65,16 @@ public function testAskChoice()
$this->assertEquals('Value "Fabien" is invalid', $e->getMessage());
}

try {
$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, '1');
$question->setMaxAttempts(1);
$question->setMultiselect(true);
$questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $output = $this->createOutputInterface(), $question);
$this->fail();
} catch (InvalidArgumentException $e) {
$this->assertEquals('Value "é" is invalid', $e->getMessage());
}

$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, null);
$question->setMaxAttempts(1);
$question->setMultiselect(true);
Expand All @@ -81,8 +92,10 @@ public function testAskChoice()
$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, ' 0 , 1 ');
$question->setMaxAttempts(1);
$question->setMultiselect(true);
$question->setPrompt('Choose wisely');

$this->assertEquals(array('Superman', 'Batman'), $questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $this->createOutputInterface(), $question));
$this->assertEquals(array('Superman', 'Batman'), $questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $output = $this->createOutputInterface(), $question));
$this->assertOutputContains('Choose wisely', $output);
}

public function testAsk()
Expand Down Expand Up @@ -774,4 +787,11 @@ private function hasSttyAvailable()

return $exitcode === 0;
}

private function assertOutputContains($expected, StreamOutput $output)
{
rewind($output->getStream());
$stream = stream_get_contents($output->getStream());
$this->assertContains($expected, $stream);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Symfony\Component\Console\Helper\SymfonyQuestionHelper;
use Symfony\Component\Console\Output\StreamOutput;
use Symfony\Component\Console\Question\ChoiceQuestion;
use Symfony\Component\Console\Question\ConfirmationQuestion;

/**
* @group tty
Expand All @@ -22,7 +23,7 @@ public function testAskChoice()

$heroes = array('Superman', 'Batman', 'Spiderman');

$inputStream = $this->getInputStream("\n1\n 1 \nFabien\n1\nFabien\n1\n0,2\n 0 , 2 \n\n\n");
$inputStream = $this->getInputStream("\n1\n 1 \nFabien\n1\nFabien\n1\n0,2\n 0 , 2 \n\n\ny\n");

$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, '2');
$question->setMaxAttempts(1);
Expand Down Expand Up @@ -71,6 +72,34 @@ public function testAskChoice()

$this->assertEquals(array('Superman', 'Batman'), $questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $output = $this->createOutputInterface(), $question));
$this->assertOutputContains('What is your favorite superhero? [Superman, Batman]', $output);

$question = new ConfirmationQuestion('Do you like superheroes ?');
$this->assertTrue($questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $output = $this->createOutputInterface(), $question));
$this->assertOutputContains('Do you like superheroes ? (yes/no)', $output);
}

/**
* @expectedException \Symfony\Component\Console\Exception\LogicException
* @expectedExceptionMessage A value is required.
*/
public function testAskWithInvalidValidatorThrowsLogicException()
{
$questionHelper = new SymfonyQuestionHelper();

$helperSet = new HelperSet(array(new FormatterHelper()));
$questionHelper->setHelperSet($helperSet);

$heroes = array('Superman', 'Batman', 'Spiderman');

$inputStream = $this->getInputStream("\n1\n");

$question = new ChoiceQuestion('What is your favorite superhero?', $heroes, '2');
$question->setMaxAttempts(1);
$question->setValidator(function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to use function. You can write direct $question->setValidator('')

Copy link
Member

Choose a reason for hiding this comment

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

Wrong. The validator is a callable

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

return '';
});

$questionHelper->ask($this->createStreamableInputInterfaceMock($inputStream), $output = $this->createOutputInterface(), $question);
}

protected function getInputStream($input)
Expand Down
26 changes: 26 additions & 0 deletions src/Symfony/Component/Console/Tests/Helper/TableCellTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Console\Tests\Helper;

use Symfony\Component\Console\Helper\TableCell;

class TableCellTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \Symfony\Component\Console\Exception\InvalidArgumentException
* @expectedExceptionMessage The TableCell does not support the following options: 'invalid'.
*/
public function testConstructorWithInvalidOptions()
{
new TableCell('', array('invalid' => 'invalid'));
}
}
12 changes: 11 additions & 1 deletion src/Symfony/Component/Console/Tests/Helper/TableStyleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,22 @@
class TableStyleTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \InvalidArgumentException
* @expectedException \Symfony\Component\Console\Exception\InvalidArgumentException
* @expectedExceptionMessage Invalid padding type. Expected one of (STR_PAD_LEFT, STR_PAD_RIGHT, STR_PAD_BOTH).
*/
public function testSetPadTypeWithInvalidType()
{
$style = new TableStyle();
$style->setPadType('TEST');
}

/**
* @expectedException \Symfony\Component\Console\Exception\LogicException
* @expectedExceptionMessage The padding char must not be empty.
*/
public function testSetPaddingCharWithInvalidType()
{
$style = new TableStyle();
$style->setPaddingChar(null);
}
}
Loading