Test and do not add leading slash when there is one already #5473 #5511
Conversation
|
I put together some test cases. Below is the result of this PR test cases execution on codeception-2.6, which explain the change. Test cases ($configUrl, $requestUrl, $expectedFullUrl): ` [Seed] 1411564475 Unit Tests (11) ------------------------
|
src/Codeception/Module/REST.php
Outdated
| $url = '/' . $url; | ||
| } | ||
| if (strpos($url, '://') === false && $this->config['url']) { | ||
| $url = rtrim($this->config['url'], '/') . ($url && $this->config['url'] ? '/' : '') . ltrim($url, '/'); |
There was a problem hiding this comment.
There is no need to check $this->config['url'] in ternary operator, it was checked in a previous line.
Another edge case to test is when there is a trailing slash in config['url'] - array('http://v1/', '', 'http://v1/'),
removing the slash could be a wrong thing to do, right?
There was a problem hiding this comment.
How about handling empty $url as a special case?
if ($url === '') {
$url = $this->config['url'];
} else {
$url = rtrim($this->config['url'], '/') . '/' . ltrim($url, '/')
}…s. Removed autoprefixing of url with slash, as that should be done by users explicitely.
Redo url parts concatenations the right way.
Moved forward slash prefixing out of the branch to the main level of the function as that would be more consistent behaviour between cases when config['url'] set and empty... though I would rather remove that slash prefixing altogether L583-L585.
This is redo of #5474