TestingParty08: url() for internal urls need a thorough test
chx - August 17, 2008 - 08:10
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | closed |
| Issue tags: | Novice |
Description
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.
#1
#2
Is url('cron.php') also considered as a internal url, or external (see issue http://drupal.org/node/296321) ?
#3
See also #296466: TestingParty08: Cron tests should pass without clean-urls enabled.
#4
Internal means a Drupal path handled by the menu system.
#5
#6
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?
#7
#8
The last submitted patch failed testing.
#9
See: #335122: Test clean HEAD after every commit and http://pastebin.ca/1258476
#10
The last submitted patch failed testing.
#11
@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.
#12
@kscheirer : no, it was the list of issues.
#13
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.
#14
Assigned to myself for review.
#15
Patched looked good. I added two lines to test passing '< front >' as the first parameter (one line each for clean_url on/off).
#16
Looks great, tests pass, RTBC.
#17
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. ;)
#18
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! :)
#19
#20
Here's a patch to wrap the comments attached to testUrl() at 80 characters.
#21
Thank you Julia, for satisfying my inner neat freak. :)
Committed to HEAD.
#22
Automatically closed -- issue fixed for 2 weeks with no activity.