Document all drupal(Post|Get)(*) methods $path parameter

@dawehner,
As per the documentation specified in drupalPostWithFormat in WebTestBase.php the path should be string not object, same in drupalGetWithFormat but as per the current implementation in QuickEditLoadingTest the Url::fromRoute('quickedit.metadata') function returns the object.
Is the documentation outdated ?
Should we need to look the other usages of the drupalPostWithFormat ?

Yes. Please look at all of the functions in WebTestBase that accept a $path parameter. If the function is documented to just accept string but it actually accepts a Url object as well, fix the docs. I'll update the summary to say this explicitly, as it's a bit confusing (to me anyway) as it is now.

Also on the one that was patched here:

-   * @param string $path
-   *   Path to request AJAX from.
+   * @param \Drupal\Core\Url|string $path
+   *   Drupal path or URL to load into internal browser.

These docs were incorrectly changed to say "internal browser" not "AJAX". So just be careful when finding these -- can't just copy/paste the description everywhere, it has to match the method's functionality. [This change was actually copied directly from the issue summary, and I've fixed the summary.]

Thanks!

I've made a patch for this. As well as fixing the documentation of the $path variable, I fixed a few docblocks that had incorrect @return values, or were missing a @throw section. Should I have created a separate issue for those, or maybe changed the issue title?

So...

Could you please split off all the return value stuff into a separate issue? This issue is only about the $path parameter.

That said, I reviewed the entire patch, and I think it needs a bit of work:

1. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1311,7 +1314,7 @@ protected function curlInitialize() {
   +   * @return mixed
       *   The content returned from the call to curl_exec().
   

   I believe this is either returning a string (the content of the page) or FALSE (if it failed. So instead of "mixed", lets' say

   string|false

   and document that it returns FALSE if the reqeust fails.

2. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1546,10 +1552,10 @@ protected function drupalGet($path, array $options = array(), array $headers = a
   -   * Retrieves a Drupal path or an absolute path and JSON decode the result.
   +   * Requests a Drupal path or an absolute path in JSON format, and JSON decodes the response.
   

   First line docs for functions need to be <= 80 character lines... so this needs to be shorter.

   I would suggest leaving the wording as it was, but changing the verbs so they are all ending in s (third person).

3. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1584,7 +1589,17 @@ protected function drupalGetWithFormat($path, $format, array $options = [], arra
   -   * Requests a Drupal path in drupal_ajax format and JSON-decodes the response.
   +   * Requests a Drupal path or an absolute path in drupal_ajax format and JSON-decodes the response.
   

   Again, needs to be <= 80 characters. I think in this line we can just say URL instead of "Drupal path or absolute path", to stay shorter.

4. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1584,7 +1589,17 @@ protected function drupalGetWithFormat($path, $format, array $options = [], arra
   +   * @param \Drupal\Core\Url|string $path
   +   *   Drupal path or URL to request given format from.
   ...
   +   *
   

   "given format" is not appropriate here, as this is for only Ajax.

5. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1584,7 +1589,17 @@ protected function drupalGetWithFormat($path, $format, array $options = [], arra
   +   *   Decoded json.
   

   json -> JSON

6. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1594,7 +1609,17 @@ protected function drupalGetAjax($path, array $options = array(), array $headers
   +   *   Drupal path or URL to request given format from.
   

   Again, for this function I wouldn't say "given format".

7. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1684,6 +1709,11 @@ protected function drupalGetXHR($path, array $options = array(), array $headers
   +   * @throws \Exception If #ajax path is not set when emulating an ajax submission.
   

   Line needs wrapping

   Also ajax => Ajax.

8. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1824,8 +1854,10 @@ protected function drupalPostForm($path, $edit, $submit, array $options = array(
   +   * @return array
   +   *   Decoded json and an array of Ajax commands.
   

   Um. How can it return decoded JSON and an array of Ajax commands? Makes no sense to me.

   Also json -> JSON

9. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -2071,7 +2102,7 @@ protected function drupalProcessAjaxResponse($content, array $ajax_response, arr
   +   * @return mixed
   

   See above comment about this return value...

   So we have some things that are documented to return string, and some mixed. I think probably all of them can return string|false, right? Not sure... but let's document them all the same if so.

10. +++ b/core/modules/simpletest/src/WebTestBase.php
    @@ -2105,7 +2135,7 @@ protected function drupalPost($path, $accept, array $post, $options = array()) {
    +   * @return mixed
        *   The content returned from the call to curl_exec().
    

    HEre's another one.

11. +++ b/core/modules/simpletest/src/WebTestBase.php
    @@ -2193,7 +2223,7 @@ protected function cronRun() {
    +   * @return string|bool
        *   Either the new page content or FALSE.
    

    If TRUE cannot be returned, use

    string|false

    not

    string|bool

12. +++ b/core/modules/simpletest/src/WebTestBase.php
    @@ -2587,7 +2617,7 @@ protected function drupalGetHeaders($all_requests = FALSE) {
    +   * @return string|bool
        *   The HTTP header value or FALSE if not found.
    

    string|false

13. +++ b/core/modules/simpletest/src/WebTestBase.php
    @@ -2893,7 +2923,7 @@ protected function verboseEmail($count = 1) {
    +   * @return Request
        *   The mocked request object.
    

    Needs namespace on the class here.

Thanks for reviewing. OK, I've split out the $path parameter stuff and rolled a patch for this issue.

I'll open a new issue for the rest of the doc fixes.

Looks good! Thanks for keeping this patch to what the issue is actually about.

API docs only.

Committed aeed399 and pushed to 8.0.x. Thanks!