TestingParty08: external URL options need a test

Patch apply's and works but there were coding standard violations. Spaces around concatenation, spaces in front of comments and caps at beginning of comments.

Whoops forgot to change back

I've reviewed this, it's good.

Since this is a testing party patch, I'm going to be very nit-picky in this review. It's only because I care. :)

+class UrlTestCase extends DrupalWebtestCase {

Let's add a PHPdoc block with a quick sentence that summarizes what this test case does.

+      'description' => t("Performs tests on the url() function"),

a) Should be single quotes, not double, since there's no interpolation going on.
b) Should end in a period.

+      'group' => t('System')

Should be a comma at the end of this line. I know it looks like a mistake, but it's the standards. :)

+  /*
+   * Test case
+   * Assert that calling url() with an external URL
+   *   1. containing a fragment works with and without a fragment in $options.
+   *   2. containing or not containing a query works with a query in $options.
+   */

a) /* needs to be /**
b) Test case isn't very descriptive. I would replace that with a one-sentence summary, like "Test the url() function's $options array." Make sure there's a newline between this sentence and the lines below it with more detail.

+    $this->assertEqual($test_url . '&' . $query2, $result_url, t('External URL with a query'));
...
+    $this->assertEqual($test_url . '?' . $query2, $result_url, t('External URL without a query.'));

The last messages were nice and descriptive as to what was being tested. These two, a little less so...

+    $test_url = 'http://www.drupal.org';

That's weird that the code below passes. Looks like this needs a trailing /.

That's it. :) Thanks a lot!!

Hopefully this one is better.

url('http://drupal.org', array('query' => 'param1');

returns http://drupal.org?param1

So the ending slash is not needed.

Added the test function to the other descriptions.

getInfo() should no longer have PHPdoc, see http://drupal.org/node/325974

Also it looks like the calls to t() could use double quotes to save escaping the single quotes.

Reroll according to #11.

@lilou: link to patch in #12 doesn't seem to be working (directs me to the d.o. home page), and the testbot isn't picking this one up for testing. Could you re-upload?

Now tests may passes.

Moving to the appropriate component since url() is base of the base system.

tests look good, all pass. rerolled against HEAD.

Attached patch remove t() in getinfo() (#500866: [META] remove t() from assert message).

Patch applied without fuzz and tests pass. Not sure why testbot is complaining of a warning in the Javascript test.

As a side note though, we now have 3 tests in the System group that all deal with the url() function:

• Tests for the url() function - Performs tests on the url() function. (this patch)
• URL generation tests - Confirm that url(), drupal_query_string_encode(), and l() work correctly with various input.
• Valid Url - Performs tests on Drupal's valid url function.

Whoops. Sorry this took so long to get back on my radar. :\

Committed to HEAD, thanks!