Compile urls with/without query, with/without fragment, absolute on/off and assert all that works when clean URLs are on and off. That wll be 16 url calls and asserts.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kkaefer’s picture

Title: TestingParty08: url() for internal urls need a through test » TestingParty08: url() for internal urls need a thorough test
swentel’s picture

Is url('cron.php') also considered as a internal url, or external (see issue http://drupal.org/node/296321) ?

Damien Tournoud’s picture

chx’s picture

Internal means a Drupal path handled by the menu system.

R.Muilwijk’s picture

Assigned: Unassigned » R.Muilwijk
R.Muilwijk’s picture

FileSize
3.41 KB

I'm having problems doing this test... the problem is in the code of url:

  static $script;
  static $clean_url;

  if (!isset($script)) {
    // On some web servers, such as IIS, we can't omit "index.php". So, we
    // generate "index.php?q=foo" instead of "?q=foo" on anything that is not
    // Apache.
    $script = (strpos($_SERVER['SERVER_SOFTWARE'], 'Apache') === FALSE) ? 'index.php' : '';
  }

  // Cache the clean_url variable to improve performance.
  if (!isset($clean_url)) {
    $clean_url = (bool)variable_get('clean_url', '0');
  }

as you can see $script and $clean_url are saved in a static which means this function can only be tested in different page loads with fake modules or something.. is that the way to go?

lilou’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

kscheirer’s picture

Component: tests » base system

@lilou: that pastebin is expired, was there anything useful for this patch in there? I'd still like to see some tests for url() go in.

lilou’s picture

@kscheirer : no, it was the list of issues.

kscheirer’s picture

Assigned: R.Muilwijk » kscheirer
Status: Needs work » Needs review
FileSize
3.58 KB

Here's an initial set of 16 tests, hopefully what chx was looking for. R.Muilwijk, I think you were trying to check for too many possible configurations, so I tried to keep it simple. Maybe I have missed some possibilities though? Also moved the test into the same area as the rest of the l() and url() function tests.

pamelad’s picture

Assigned: kscheirer » pamelad

Assigned to myself for review.

pamelad’s picture

FileSize
4.04 KB

Patched looked good. I added two lines to test passing '< front >' as the first parameter (one line each for clean_url on/off).

cwgordon7’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, tests pass, RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Heh. That's a clever way to write this test!

I couldn't find anything to complain about other than the lines are VERY long (but they're also t() strings which are long by nature) and PHPDoc didn't wrap at 80 chars, and... CRAP I meant to fix that before I committed but I forgot. I'll go start a novice issue. :P

Committed to HEAD. ;)

webchick’s picture

Status: Fixed » Needs work
Issue tags: +Novice

Eh. I'll just re-open this one. :)

Hello, Novice! Your task is to wrap the comment at the top of function testUrl() in modules/simpletest/tests/common.test at 80 characters. Thanks! :)

dmitrig01’s picture

Assigned: pamelad » Unassigned
Priority: Critical » Minor
JuliaKM’s picture

Status: Needs work » Needs review
FileSize
784 bytes

Here's a patch to wrap the comments attached to testUrl() at 80 characters.

webchick’s picture

Status: Needs review » Fixed

Thank you Julia, for satisfying my inner neat freak. :)

Committed to HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.