Integration tests refactoring #26235
Conversation
|
@SergioBertolinSG, thanks for your PR! By analyzing the annotation information on this pull request, we identified @PVince81, @rullzer and @DeepDiver1975 to be potential reviewers |
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
Looks good to me, up to you if you want to act on my comments. 👍
| } else { | ||
| return $this->davPath . '/files/' . $user . '/'; | ||
| } | ||
| }*/ |
There was a problem hiding this comment.
remove commented out code ?
| $fullUrl = substr($this->baseUrl, 0, -4) . $this->davPath . "$path"; | ||
| public function makeDavRequest($user, $method, $path, $headers, $body = null, $type = "files"){ | ||
| if ( $type === "files" ){ | ||
| echo "user: " . $user . " method: " . $method . " path: " . $path . " type: " . $type; |
There was a problem hiding this comment.
did you use echo just for debugging ? maybe remove it
| Given using new dav path | ||
| And As an "admin" | ||
| When Downloading file "welcome.txt" | ||
| When Downloading file "/welcome.txt" |
There was a problem hiding this comment.
I'm not sure we really need to enforce that, unless we want this level of precision.
In PHP it's easy to force add a slash without double by doing something like '/' . ltrim($path, '/')
There was a problem hiding this comment.
I want to have it consistent and use slash before always, having it different is confusing.
|
Looks good, merging |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Refactor of webdav trait to be more readable and clear.
Use of initial slash '/' as standard way for dealing with files and folders in gherkin.
Motivation and Context
There is confusion about how the code works.
#26098 (comment)
How Has This Been Tested?
Running tests locally.
Checklist: