From edc58cb8f29c3588ed4fc833a22e1e13ff107a42 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 3 Sep 2015 16:03:24 -0500 Subject: [PATCH 1/3] Rewrote test from #60 to satisfy reported expectations - `getMessages()` is expected to return the `NotEmpty::IS_EMPTY` message template. --- test/InputTest.php | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/test/InputTest.php b/test/InputTest.php index bd88157c..3a951e2c 100644 --- a/test/InputTest.php +++ b/test/InputTest.php @@ -11,12 +11,14 @@ use PHPUnit_Framework_MockObject_MockObject as MockObject; use PHPUnit_Framework_TestCase as TestCase; +use ReflectionProperty; use stdClass; use Zend\Filter; use Zend\Filter\FilterChain; use Zend\InputFilter\Input; use Zend\InputFilter\InputInterface; use Zend\Validator; +use Zend\Validator\NotEmpty; use Zend\Validator\ValidatorChain; /** @@ -175,17 +177,33 @@ public function testRequiredWithoutFallbackAndValueNotSetThenFail() $this->assertEquals(['Value is required'], $input->getMessages(), 'getMessages() value not match'); } - public function testRequiredWithoutFallbackAndValueNotSetThenFailWithCustomErrorMessage() + /** + * @group 28 + * @group 60 + */ + public function testRequiredWithoutFallbackAndValueNotSetProvidesNotEmptyValidatorIsEmptyErrorMessage() { $input = $this->input; $input->setRequired(true); - $input->setErrorMessage('fooErrorMessage'); $this->assertFalse( $input->isValid(), - 'isValid() should be return always false when no fallback value, is required, and not data is set.' + 'isValid() should always return false when no fallback value is present, ' + . 'the input is required, and no data is set.' + ); + $messages = $input->getMessages(); + $this->assertInternalType('array', $messages); + $this->assertArrayHasKey(NotEmpty::IS_EMPTY, $messages); + + $notEmpty = new NotEmpty(); + $r = new ReflectionProperty($notEmpty, 'messageTemplates'); + $r->setAccessible(true); + $messageTemplates = $r->getValue($notEmpty); + $this->assertEquals( + $messageTemplates[NotEmpty::IS_EMPTY], + $messages[NotEmpty::IS_EMPTY], + 'getMessages() values do not match' ); - $this->assertEquals(['fooErrorMessage'], $input->getMessages(), 'getMessages() value not match'); } public function testNotRequiredWithoutFallbackAndValueNotSetThenIsValid() From ff811865131958411714a928c0576b2b31622cec Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 3 Sep 2015 16:22:52 -0500 Subject: [PATCH 2/3] Fixed validation failure messages for required input - Updates the test case to create an assertion method; the method now uses `getOption('messageTemplates')` instead of reflection to be less brittle, and is used in two separate test methods. - Updated `Input` to add a `prepareRequiredValidationFailureMessage()` method taht creates the required message format. Each of `Input`, `ArrayInput`, and `FileInput` now assign the return of that method to the `$errorMessage` property directly. --- src/ArrayInput.php | 2 +- src/FileInput.php | 2 +- src/Input.php | 16 +++++++++++++++- test/InputTest.php | 35 ++++++++++++++++++++--------------- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/ArrayInput.php b/src/ArrayInput.php index 2ee27809..d23b9d81 100644 --- a/src/ArrayInput.php +++ b/src/ArrayInput.php @@ -71,7 +71,7 @@ public function isValid($context = null) if (! $hasValue && $required) { if ($this->errorMessage === null) { - $this->setErrorMessage('Value is required'); + $this->errorMessage = $this->prepareRequiredValidationFailureMessage(); } return false; } diff --git a/src/FileInput.php b/src/FileInput.php index 48aa010e..b195306a 100644 --- a/src/FileInput.php +++ b/src/FileInput.php @@ -125,7 +125,7 @@ public function isValid($context = null) if (! $hasValue && $required && ! $this->hasFallback()) { if ($this->errorMessage === null) { - $this->setErrorMessage('Value is required'); + $this->errorMessage = $this->prepareRequiredValidationFailureMessage(); } return false; } diff --git a/src/Input.php b/src/Input.php index 50c98fc6..6342040b 100644 --- a/src/Input.php +++ b/src/Input.php @@ -404,7 +404,7 @@ public function isValid($context = null) if (! $hasValue && $required) { if ($this->errorMessage === null) { - $this->setErrorMessage('Value is required'); + $this->errorMessage = $this->prepareRequiredValidationFailureMessage(); } return false; } @@ -483,4 +483,18 @@ protected function injectNotEmptyValidator() $chain->prependValidator(new NotEmpty(), true); } + + /** + * Create and return the validation failure message for required input. + * + * @return string[] + */ + protected function prepareRequiredValidationFailureMessage() + { + $notEmpty = new NotEmpty(); + $templates = $notEmpty->getOption('messageTemplates'); + return [ + NotEmpty::IS_EMPTY => $templates[NotEmpty::IS_EMPTY], + ]; + } } diff --git a/test/InputTest.php b/test/InputTest.php index 3a951e2c..56bd0e71 100644 --- a/test/InputTest.php +++ b/test/InputTest.php @@ -11,7 +11,6 @@ use PHPUnit_Framework_MockObject_MockObject as MockObject; use PHPUnit_Framework_TestCase as TestCase; -use ReflectionProperty; use stdClass; use Zend\Filter; use Zend\Filter\FilterChain; @@ -36,6 +35,24 @@ public function setUp() $this->input = new Input('foo'); } + public function assertRequiredValidationErrorMessage($input, $message = '') + { + $message = $message ?: 'Expected failure message for required input'; + $message .= ';'; + + $messages = $input->getMessages(); + $this->assertInternalType('array', $messages, $message . ' non-array messages array'); + $this->assertArrayHasKey(NotEmpty::IS_EMPTY, $messages, $message . ' missing NotEmpty::IS_EMPTY key'); + + $notEmpty = new NotEmpty(); + $messageTemplates = $notEmpty->getOption('messageTemplates'); + $this->assertEquals( + $messageTemplates[NotEmpty::IS_EMPTY], + $messages[NotEmpty::IS_EMPTY], + $message . ' NotEmpty::IS_EMPTY message differs' + ); + } + public function testConstructorRequiresAName() { $this->assertEquals('foo', $this->input->getName()); @@ -174,7 +191,7 @@ public function testRequiredWithoutFallbackAndValueNotSetThenFail() $input->isValid(), 'isValid() should be return always false when no fallback value, is required, and not data is set.' ); - $this->assertEquals(['Value is required'], $input->getMessages(), 'getMessages() value not match'); + $this->assertRequiredValidationErrorMessage($input); } /** @@ -191,19 +208,7 @@ public function testRequiredWithoutFallbackAndValueNotSetProvidesNotEmptyValidat 'isValid() should always return false when no fallback value is present, ' . 'the input is required, and no data is set.' ); - $messages = $input->getMessages(); - $this->assertInternalType('array', $messages); - $this->assertArrayHasKey(NotEmpty::IS_EMPTY, $messages); - - $notEmpty = new NotEmpty(); - $r = new ReflectionProperty($notEmpty, 'messageTemplates'); - $r->setAccessible(true); - $messageTemplates = $r->getValue($notEmpty); - $this->assertEquals( - $messageTemplates[NotEmpty::IS_EMPTY], - $messages[NotEmpty::IS_EMPTY], - 'getMessages() values do not match' - ); + $this->assertRequiredValidationErrorMessage($input); } public function testNotRequiredWithoutFallbackAndValueNotSetThenIsValid() From ff758954735136b54839d68e5a80f8564fd727d3 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 3 Sep 2015 16:27:44 -0500 Subject: [PATCH 3/3] Better assertion for required message - In each case, this should be the only message; use `assertSame()`. --- test/InputTest.php | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/InputTest.php b/test/InputTest.php index 56bd0e71..2f99d8d8 100644 --- a/test/InputTest.php +++ b/test/InputTest.php @@ -42,15 +42,12 @@ public function assertRequiredValidationErrorMessage($input, $message = '') $messages = $input->getMessages(); $this->assertInternalType('array', $messages, $message . ' non-array messages array'); - $this->assertArrayHasKey(NotEmpty::IS_EMPTY, $messages, $message . ' missing NotEmpty::IS_EMPTY key'); $notEmpty = new NotEmpty(); $messageTemplates = $notEmpty->getOption('messageTemplates'); - $this->assertEquals( - $messageTemplates[NotEmpty::IS_EMPTY], - $messages[NotEmpty::IS_EMPTY], - $message . ' NotEmpty::IS_EMPTY message differs' - ); + $this->assertSame([ + NotEmpty::IS_EMPTY => $messageTemplates[NotEmpty::IS_EMPTY], + ], $messages, $message . ' missing NotEmpty::IS_EMPTY key and/or contains additional messages'); } public function testConstructorRequiresAName()