2.3 - Add repeat expander feature #109
Conversation
README.md
Outdated
| @@ -84,6 +84,7 @@ $matcher->getError(); // returns null or error message | |||
| * ``oneOf(...$expanders)`` - example usage ``"@string@.oneOf(contains('foo'), contains('bar'), contains('baz'))"`` | |||
| * ``matchRegex($regex)`` - example usage ``"@string@.matchRegex('/^lorem.+/')"`` | |||
| * ``optional()`` - work's only with ``ArrayMatcher``, ``JsonMatcher`` and ``XmlMatcher`` | |||
| * ``repeat($pattern, $isStrict = true)`` - example usage ``'@array@.repeat({"name" => "foe"})'`` or ``"@array@.repeat('@string@')"`` | |||
There was a problem hiding this comment.
{"name" => "foe"}
I think we should either ["name" => "foe"] or {"name": "foe"} but not a mix of both there.
There was a problem hiding this comment.
Thanks, a little mistake here. Of course, it's JSON. I will fix that ASAP
| { | ||
| $jsonPattern = '{"name": "@string@", "activated": "@boolean@"}'; | ||
|
|
||
| $jsonTest = array( |
There was a problem hiding this comment.
You could use "new" PHP short array syntax [] which is less verbose
There was a problem hiding this comment.
Yes probably, but the whole project seems to use the older array() syntax. In order to stick with the project's coding convention I'd rather not change that for now.
There was a problem hiding this comment.
On master branch we are using [] - 2.3 branch was only created for 5.6 support however I'm not planning to add more new features here without adding them to the master branch first
| * @param Matcher $matcher | ||
| * @return bool | ||
| */ | ||
| private function matchJson(array $values, Matcher $matcher) |
There was a problem hiding this comment.
We should maybe allow this method to be recursive to match nested objects ?
There was a problem hiding this comment.
{
"offers": "@array@.repeat({\"subscription\": \"@array@.repeat(\"date\": \"@string@.isDateTime()\")\"})"
}This case will work with actual implementation ?
There was a problem hiding this comment.
This case should work with the nested array.repeat syntax 'cause I just make a call to php-matcher factory. But you're right, maybe allowing nested objects would be more clever.
There was a problem hiding this comment.
Yes I'm sorry, didn't have the time to do it
| public function __construct($pattern, $isStrict = true) | ||
| { | ||
| if (!is_string($pattern)) { | ||
| throw new \InvalidArgumentException("Repeat pattern must be a string."); |
There was a problem hiding this comment.
This exception is not covered by tests
| private function matchScalar(array $values, Matcher $matcher) | ||
| { | ||
| foreach ($values as $index => $value) { | ||
| $match = $matcher->match($value, $this->pattern); |
There was a problem hiding this comment.
If we only match value, is following pattern failing ?
@array@.repeat({"foo": "@string@"})
[
{
"foo": "bar"
},
{
"test": "bar"
}
]
``There was a problem hiding this comment.
There was a problem hiding this comment.
I'm not sure to understand what you're talking about here ?
There was a problem hiding this comment.
Just ignore this comment
Sure, but first I would like to see that feature on master branch and then into 2.3 or even 2.4 |
Totally agreed with that |
|
Fix readme and add cover for a new exception in test. Also create #110 for 3.0 repeat. |
Add "repeat" requested feature in:
#35
And also in:
#21
Feel free to comment, give advice.
I want to merge on 2.3 because I want to stick with PHP 5.6 not 7.