GHOP task #129: Write tests for path module

chx - January 7, 2008 - 08:31
Project:SimpleTest
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:closed
Issue tags:GHOP
Description
  • create an alias for a node.
  • change the alias
  • move the alias to another node by giving it the same alias
  • add, edit, delete aliases from the administration screen

#1

chx - January 7, 2008 - 08:35
Title:Write tests for path module» GHOP task #129: Write tests for path module

#2

aclight - January 7, 2008 - 12:26

The GHOP task associated with this issue can be found at:
http://code.google.com/p/google-highly-open-participation-drupal/issues/...

#3

chx - January 8, 2008 - 08:08
Status:active» needs review

This is the test from http://code.google.com/p/google-highly-open-participation-drupal/issues/... I have just copied over. That issue includes patches against path module, too.

AttachmentSizeStatusTest resultOperations
path_module.test4.45 KBIgnoredNoneNone

#4

chx - January 8, 2008 - 08:20
Status:needs review» needs work

First of all thanks for the speedy work!

You need a drupalModuleEnable to make sure path module is on.

User input is not fed to t() so remove please the t($node1->title) the t() calls. t is used only for constant strings not user input.

You can safely presume that each randomname calls differ so no need for the getNewAlias function.

When checking for alias moves, I would assert the The path is already in use. message.

Core tests file go into the tests subdirectory of the simpletest module, no need for path_simpletest. Just upload the fixed test file.

#5

Rok Žlender - January 8, 2008 - 09:05

Two more things beside chxs' notes:
- I prefer using assertText function when you look for plain text on a page (title,body, anything that is visible), assertWantedRaw is used if you want to assert something in source code i.e. "Page simpletest_XRs4 has been updated."
- after you change the alias, you could also check that old alias produces "page not found" error

#6

boombatower - January 8, 2008 - 21:06

Summary of changes:

  • Added drupalModuleEnable().
  • Removed t().
  • Removed getNewAlias().
  • Checks for: The path is already in use. and The alias %alias is already in use in this language. instead of checking see if alias call would return that page.
  • Changed test location and removed _simpletest() hook.
  • Changed raw tests to text.
  • Checks to make sure that previous alias no longer works.

A few notes:

  • I used the page_creation.test as an example and it appears to use both assertWantedRaw() and t() for the reason that t() adds <em></em> around inserted variables. To implement the check for the message stating that the alias is already in use I have to do the same thing and thus have to leave a assertWantedRaw() in my test case.

    $this->assertWantedRaw(t('Your %post has been created.', array ('%post' => 'Page')), 'Page created');

  • The placement of tests and the simpletest hook are rather confusing since I was pointed to: http://www.lullabot.com/articles/introduction-unit-testing

I believe this test case includes all the requested changes.

AttachmentSizeStatusTest resultOperations
path_module.test_.txt4.76 KBIgnoredNoneNone

#7

Rok Žlender - January 9, 2008 - 08:58

Great work. You have made all the changes we asked for and I consider your task finished. I have two minor suggestions but this is really nitpicking

  1. You can check that when alias is changed you really get page not found with sth like this
    <?php
    $this
    ->assertTitle(new PatternExpectation('/Page not found/'), 'We get page  not found error');
    ?>
  2. as chx said user input is not run through t() function but warnings and notices that drupal outputs usually are i.e. t('The path is already in use.')

I added this small changes to the test so you don't need to bother with them and also committed this test to cvs.

#8

Rok Žlender - January 9, 2008 - 09:01
Status:needs work» fixed

Oh yeah I guess this is fixed now.

Regarding the hook confusion. You only need simpletest hook if you write tests for your own module (non core). Core tests are kept in simpletest module and are searched for in simpletest/tests dir. So any file that ends with ".test" in simpletest/tests is included automatically.

#9

boombatower - January 9, 2008 - 17:30

Thanks for clearing that up.

#10

Anonymous (not verified) - January 23, 2008 - 17:32
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.