For missing properties, the key for that error in set to "" #154
Conversation
|
@samkeen good fix. Would you mind including a test for this? |
|
added tests. Let me know if they are adequate. I can also rebase to a single commit if you prefer. |
|
@samkeen thank you for the update! I think a squash would be great. I have restarted the Travis runs to see if the failures clear up. |
samkeen
force-pushed
the
fix_empty_property_keyname_when_property_is_missing
branch
from
745cd08 to
c3ec089
Compare
|
rebased to current upstream/master, + 2 commits squashed to 1 |
|
+1 looks good. @alecsammon @Maks3w would you mind to help review? |
|
|
||
| $validator->check($document, $schema); | ||
| $error = $validator->getErrors(); | ||
| $this->assertEquals('foo', $error[0]['property']); |
There was a problem hiding this comment.
If $error is not well formatted, i.e. there is no index 0 or 'property` key does not exists test run will stop without an useful message.
Could you try to compare the whole $error variable or assert 0 and property exists?
|
@bighappyface, @samkeen has reveal invalid data provider is not enough without to check invalid data fails with the expected message. i.e. invalid data providers should be refactored for to check the error message too. |
samkeen
force-pushed
the
fix_empty_property_keyname_when_property_is_missing
branch
from
c3ec089 to
b1c2f65
Compare
|
|
||
| $validator->check($document, $schema); | ||
| $error = $validator->getErrors(); | ||
| if(!isset($error[0]['property'])) { |
There was a problem hiding this comment.
PHPUnit provides assertArrayHasKey and assertArraySubset try to use one of them.
Also this does not check if index 0 is set.
There was a problem hiding this comment.
Sorry assertArraySubset requires phpunit ^4.4.0 /cc @bighappyface
There was a problem hiding this comment.
ah, sorry been away from PHP for a while. Let me know if latest push works, not the most elegant code but I think it covers any malformed input with a helpful fail message.
- Expected bahavior is that the key would be set to the poperty name. - added tests to ensure the "property" is populated regardless of required attribute is completely missing or has empty value.
samkeen
force-pushed
the
fix_empty_property_keyname_when_property_is_missing
branch
from
b1c2f65 to
a71086d
Compare
|
sorry @Maks3w , i realized i didn't use |
|
@Maks3w any last thoughts on this PR? |
|
Can you merge guys we are actively waiting for this PR making our tests failed. |
…property_is_missing For missing properties, the key for that error in set to ""
Expected bahavior is that the key would be set to the poperty name.
This fix addressed the same issue PR'd by @primitive-type but is
rebased to current master