• 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
CommentFileSizeAuthor
#6 path_module.test_.txt4.76 KBboombatower
#3 path_module.test4.45 KBchx

Comments

chx’s picture

Title: Write tests for path module » GHOP task #129: Write tests for path module
aclight’s picture

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

chx’s picture

Status: Active » Needs review
StatusFileSize
new4.45 KB

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.

chx’s picture

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.

Rok Žlender’s picture

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

boombatower’s picture

StatusFileSize
new4.76 KB

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.

Rok Žlender’s picture

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
    $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.

Rok Žlender’s picture

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.

boombatower’s picture

Thanks for clearing that up.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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