From 9104ef1d749945c40bbe9dd84e9dbd145cfec0eb Mon Sep 17 00:00:00 2001 From: IceMaD Date: Thu, 11 Jul 2019 21:44:16 +0200 Subject: [PATCH 1/7] Fix multiSelect ChoiceQuestion when answers have spaces --- .../Console/Question/ChoiceQuestion.php | 10 ++- .../Tests/Question/ChoiceQuestionTest.php | 64 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php diff --git a/src/Symfony/Component/Console/Question/ChoiceQuestion.php b/src/Symfony/Component/Console/Question/ChoiceQuestion.php index 5cb5056903..3dca160049 100644 --- a/src/Symfony/Component/Console/Question/ChoiceQuestion.php +++ b/src/Symfony/Component/Console/Question/ChoiceQuestion.php @@ -134,17 +134,15 @@ class ChoiceQuestion extends Question $isAssoc = $this->isAssoc($choices); return function ($selected) use ($choices, $errorMessage, $multiselect, $isAssoc) { - // Collapse all spaces. - $selectedChoices = str_replace(' ', '', $selected); - if ($multiselect) { // Check for a separated comma values - if (!preg_match('/^[^,]+(?:,[^,]+)*$/', $selectedChoices, $matches)) { + if (!preg_match('/^[^,]+(?:,[^,]+)*$/', $selected, $matches)) { throw new InvalidArgumentException(sprintf($errorMessage, $selected)); } - $selectedChoices = explode(',', $selectedChoices); + + $selectedChoices = array_map('trim', explode(',', $selected)); } else { - $selectedChoices = [$selected]; + $selectedChoices = [trim($selected)]; } $multiselectChoices = []; diff --git a/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php new file mode 100644 index 0000000000..5ec7a9cacb --- /dev/null +++ b/src/Symfony/Component/Console/Tests/Question/ChoiceQuestionTest.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Console\Tests\Question; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Console\Question\ChoiceQuestion; + +class ChoiceQuestionTest extends TestCase +{ + /** + * @dataProvider selectUseCases + */ + public function testSelectUseCases($multiSelect, $answers, $expected, $message) + { + $question = new ChoiceQuestion('A question', [ + 'First response', + 'Second response', + 'Third response', + 'Fourth response', + ]); + + $question->setMultiselect($multiSelect); + + foreach ($answers as $answer) { + $validator = $question->getValidator(); + $actual = $validator($answer); + + $this->assertEquals($actual, $expected, $message); + } + } + + public function selectUseCases() + { + return [ + [ + false, + ['First response', 'First response ', ' First response', ' First response '], + 'First response', + 'When passed single answer on singleSelect, the defaultValidator must return this answer as a string', + ], + [ + true, + ['First response', 'First response ', ' First response', ' First response '], + ['First response'], + 'When passed single answer on MultiSelect, the defaultValidator must return this answer as an array', + ], + [ + true, + ['First response,Second response', ' First response , Second response '], + ['First response', 'Second response'], + 'When passed multiple answers on MultiSelect, the defaultValidator must return these answers as an array', + ], + ]; + } +} From ce6187908c5e32a4431f12878c08840e7857bd6b Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 17:40:38 +0200 Subject: [PATCH 2/7] fix expected exception message regex --- .../PropertyAccess/Tests/PropertyAccessorCollectionTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php index ad3c9a5b48..61ce5f1272 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorCollectionTest.php @@ -189,7 +189,7 @@ abstract class PropertyAccessorCollectionTest extends PropertyAccessorArrayAcces /** * @expectedException \Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException - * @expectedExceptionMessageRegExp /Could not determine access type for property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*": The property "axes" in class "Mock_PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis\(\)", "removeAxis\(\)" but the new value must be an array or an instance of \\Traversable, "string" given./ + * @expectedExceptionMessageRegExp /Could not determine access type for property "axes" in class "Symfony\\Component\\PropertyAccess\\Tests\\PropertyAccessorCollectionTest_Car[^"]*": The property "axes" in class "Symfony\\Component\\PropertyAccess\\Tests\\PropertyAccessorCollectionTest_Car[^"]*" can be defined with the methods "addAxis\(\)", "removeAxis\(\)" but the new value must be an array or an instance of \\Traversable, "string" given./ */ public function testSetValueFailsIfAdderAndRemoverExistButValueIsNotTraversable() { From 16fc81fbe55e850af8c162ab75dbb803faef5f37 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 18:24:16 +0200 Subject: [PATCH 3/7] fix test --- .../Component/Messenger/Tests/TraceableMessageBusTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php b/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php index 1b03e0036b..b87fffa82f 100644 --- a/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php +++ b/src/Symfony/Component/Messenger/Tests/TraceableMessageBusTest.php @@ -37,7 +37,7 @@ class TraceableMessageBusTest extends TestCase unset($actualTracedMessage['callTime']); // don't check, too variable $this->assertEquals([ 'message' => $message, - 'stamps' => [[$stamp]], + 'stamps' => [$stamp], 'caller' => [ 'name' => 'TraceableMessageBusTest.php', 'file' => __FILE__, From affede122b2ae57e88208b57aa7dd7c8ae1716ee Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 18:52:34 +0200 Subject: [PATCH 4/7] make tests forward compatible with DI 4.4 --- .../Tests/DependencyInjection/FrameworkExtensionTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 9ffb13232f..ef074bd163 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -1170,6 +1170,7 @@ abstract class FrameworkExtensionTest extends TestCase if ($resetCompilerPasses) { $container->getCompilerPassConfig()->setOptimizationPasses([]); $container->getCompilerPassConfig()->setRemovingPasses([]); + $container->getCompilerPassConfig()->setAfterRemovingPasses([]); } $container->getCompilerPassConfig()->setBeforeRemovingPasses([new AddConstraintValidatorsPass(), new TranslatorPass('translator.default', 'translation.reader')]); $container->getCompilerPassConfig()->setAfterRemovingPasses([new AddAnnotationsCachedReaderPass()]); @@ -1191,6 +1192,7 @@ abstract class FrameworkExtensionTest extends TestCase $container->getCompilerPassConfig()->setOptimizationPasses([]); $container->getCompilerPassConfig()->setRemovingPasses([]); + $container->getCompilerPassConfig()->setAfterRemovingPasses([]); $container->compile(); return $container; From 83e7b45b6d5605d37cde44fcad0984a83c0e7298 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 20:15:30 +0200 Subject: [PATCH 5/7] fix typo --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3a02215951..12d031f64b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -231,7 +231,7 @@ install: composer global require --no-progress --no-scripts --no-plugins symfony/flex dev-master - | - # Legacy tests are skipped when deps=high and when the current branch version has not the same major version number than the next one + # Legacy tests are skipped when deps=high and when the current branch version has not the same major version number as the next one [[ $deps = high && ${SYMFONY_VERSION%.*} != $(git show $(git ls-remote --heads | grep -FA1 /$SYMFONY_VERSION | tail -n 1):composer.json | grep '^ *"dev-master". *"[1-9]' | grep -o '[0-9]*' | head -n 1) ]] && LEGACY=,legacy export COMPOSER_ROOT_VERSION=$SYMFONY_VERSION.x-dev From 3304e57233a3ab7b0ee2d316e546122a289d73b8 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 19:18:37 +0200 Subject: [PATCH 6/7] skip updated ChoiceType tests on older versions --- .../Twig/Tests/Extension/FormExtensionDivLayoutTest.php | 2 +- .../Twig/Tests/Extension/FormExtensionTableLayoutTest.php | 2 +- .../Tests/Templating/Helper/FormHelperDivLayoutTest.php | 2 +- .../Tests/Templating/Helper/FormHelperTableLayoutTest.php | 2 +- src/Symfony/Component/Form/Tests/AbstractLayoutTest.php | 8 ++++++++ 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php index e40e57505a..2758f4aad8 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php @@ -31,7 +31,7 @@ class FormExtensionDivLayoutTest extends AbstractDivLayoutTest */ private $renderer; - protected static $supportedFeatureSetVersion = 403; + protected static $supportedFeatureSetVersion = 404; protected function setUp() { diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php index 9570e03e52..7d0e3637b9 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php @@ -30,7 +30,7 @@ class FormExtensionTableLayoutTest extends AbstractTableLayoutTest */ private $renderer; - protected static $supportedFeatureSetVersion = 403; + protected static $supportedFeatureSetVersion = 404; protected function setUp() { diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperDivLayoutTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperDivLayoutTest.php index 729b01920f..219dc6df83 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperDivLayoutTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperDivLayoutTest.php @@ -30,7 +30,7 @@ class FormHelperDivLayoutTest extends AbstractDivLayoutTest */ protected $engine; - protected static $supportedFeatureSetVersion = 403; + protected static $supportedFeatureSetVersion = 404; protected function getExtensions() { diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperTableLayoutTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperTableLayoutTest.php index 8e335788ea..f47d238f1e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperTableLayoutTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Templating/Helper/FormHelperTableLayoutTest.php @@ -30,7 +30,7 @@ class FormHelperTableLayoutTest extends AbstractTableLayoutTest */ protected $engine; - protected static $supportedFeatureSetVersion = 403; + protected static $supportedFeatureSetVersion = 404; public function testStartTagHasNoActionAttributeWhenActionIsEmpty() { diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 8154893fbf..21a7ddd45f 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -726,6 +726,8 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase public function testSingleChoiceWithPreferred() { + $this->requiresFeatureSet(404); + $form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', '&a', [ 'choices' => ['Choice&A' => '&a', 'Choice&B' => '&b'], 'preferred_choices' => ['&b'], @@ -750,6 +752,8 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase public function testSingleChoiceWithPreferredAndNoSeparator() { + $this->requiresFeatureSet(404); + $form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', '&a', [ 'choices' => ['Choice&A' => '&a', 'Choice&B' => '&b'], 'preferred_choices' => ['&b'], @@ -773,6 +777,8 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase public function testSingleChoiceWithPreferredAndBlankSeparator() { + $this->requiresFeatureSet(404); + $form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', '&a', [ 'choices' => ['Choice&A' => '&a', 'Choice&B' => '&b'], 'preferred_choices' => ['&b'], @@ -797,6 +803,8 @@ abstract class AbstractLayoutTest extends FormIntegrationTestCase public function testChoiceWithOnlyPreferred() { + $this->requiresFeatureSet(404); + $form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\ChoiceType', '&a', [ 'choices' => ['Choice&A' => '&a', 'Choice&B' => '&b'], 'preferred_choices' => ['&a', '&b'], From c0eed67aa87c2d25f62b5637365d3c6e0b79d4fe Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Wed, 24 Jul 2019 21:38:05 +0200 Subject: [PATCH 7/7] relax some assertions to make tests forward compatible --- .../Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php | 6 ++++-- .../Twig/Tests/Extension/AbstractBootstrap3LayoutTest.php | 4 ---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php index 7a806d76cd..83cbbd1426 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/Form/Type/EntityTypeTest.php @@ -848,7 +848,8 @@ class EntityTypeTest extends BaseTypeTest ]); $this->assertEquals([3 => new ChoiceView($entity3, '3', 'Baz'), 2 => new ChoiceView($entity2, '2', 'Bar')], $field->createView()->vars['preferred_choices']); - $this->assertEquals([1 => new ChoiceView($entity1, '1', 'Foo')], $field->createView()->vars['choices']); + $this->assertArrayHasKey(1, $field->createView()->vars['choices']); + $this->assertEquals(new ChoiceView($entity1, '1', 'Foo'), $field->createView()->vars['choices'][1]); } public function testOverrideChoicesWithPreferredChoices() @@ -868,7 +869,8 @@ class EntityTypeTest extends BaseTypeTest ]); $this->assertEquals([3 => new ChoiceView($entity3, '3', 'Baz')], $field->createView()->vars['preferred_choices']); - $this->assertEquals([2 => new ChoiceView($entity2, '2', 'Bar')], $field->createView()->vars['choices']); + $this->assertArrayHasKey(2, $field->createView()->vars['choices']); + $this->assertEquals(new ChoiceView($entity2, '2', 'Bar'), $field->createView()->vars['choices'][2]); } public function testDisallowChoicesThatAreNotIncludedChoicesSingleIdentifier() diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3LayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3LayoutTest.php index 183cd820c3..5763345afd 100644 --- a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3LayoutTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3LayoutTest.php @@ -404,7 +404,6 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest /following-sibling::option[@disabled="disabled"][not(@selected)][.="-- sep --"] /following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"] ] - [count(./option)=3] ' ); } @@ -427,7 +426,6 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest ./option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"] /following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"] ] - [count(./option)=2] ' ); } @@ -451,7 +449,6 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest /following-sibling::option[@disabled="disabled"][not(@selected)][.=""] /following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"] ] - [count(./option)=3] ' ); } @@ -468,7 +465,6 @@ abstract class AbstractBootstrap3LayoutTest extends AbstractLayoutTest $this->assertWidgetMatchesXpath($form->createView(), ['attr' => ['class' => 'my&class']], '/select [@class="my&class form-control"] - [count(./option)=2] ' ); }