GHOP #122: Write Simpletest tests for the Forum module
| Project: | SimpleTest |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
The main work for this task is to write a small suite of tests to check the basic functionality of the Forum module.
Suggested tests:
1. admin/content/forum/add/container Add a forum container.
2. admin/content/forum/add/forum Add a forum.
3. admin/content/taxonomy/edit/vocabulary/# Edit the forum vocabulary.
4. node/add/forum Create a new forum topic.
5. node/add/forum Attempt to add a forum topic to a container, rather than a forum (this should not work).
6. node/#/edit Move a forum topic from one forum to another.
7. node/#/edit Move a forum topic from one forum to another, leaving a shadow copy (confirm existence of shadow copy).
#1
GHOP issue: http://code.google.com/p/google-highly-open-participation-drupal/issues/...
#2
OK, I'm attaching the code for the tests.
One note: there is a lot of repeated code for, for example, submitting a form and creating forums/containers. The reason is that 4 of the tests require a forum to work, 2 others require a container, and I couldn't find a function that would create the forum/container for me. If I missed the function, I'll be more than happy to take some of the repetition out of my code. :)
#3
Ok - looks like a good start. However - the 7 tests don't actually have to be separate DrupalTestCase objects - you can have all of this together in one "test" as long as these 7 pieces of functionality are tested. Or group them in 2 or 3 tests as seems most logical.
Also, if you still have repeated code, you could play the trick I used here: http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/simpletest/...
The latter tests extend the first object which defines common method in addition to those already in DrupalTestCase.
#4
Ooh, interesting, thanks a lot :)
#5
OK, I've reduced it down to 3 classes, the last of which extends the first. The size has been cut quite a bit as a result. :)
#6
ok - looking better.
A couple minor suggestions to start-
I'd rename AddForumTest to something like DrupalForumTestCase.
This should have no "testX" methods, since I think they will get run twice if a later object extends it. It should only have common methods like your function createForum().
Also, the second object should extend DrupalForumTestCase also, this way you can omit:
// Enable the forum module and its dependencies$this->drupalModuleEnable('taxonomy');
$this->drupalModuleEnable('comment');
$this->drupalModuleEnable('forum');
since the setup function will run.
Finally - minor code style issue - don't have any white space between the function name and parens in the function definition.
#7
I actually don't think the methods would be run twice unless you were to redeclare the function in the subclass and call the parent function in there. Otherwise, simpletest would be showing duplicate tests for you to choose from, which it doesn't.
In any case, here's the code with the changes made. The one thing to note here is that the new class, DrupalForumTestCase, needed a get_info function even though there are no tests in it. So, I just clarified what it is a little bit.
#8
ok- obviously I'm not paying attention. You're right about the tests not running twice. Sorry.
How about merging the first two objects together so there is no get_info() that doesn't run an actual test?
#9
I actually think we're wrong. I just made the changes and suddenly my number of passes jumped. Looking at the list of passes, it seems like the first two tests (the ones in the class that is extended) are being run in every class.
#10
testing this now - it's weird with the menu tests, they only run once, but if I add a trivial additional test it runs multiple times.
function testFoo() {$this->assertFalse(1, 'foo');
}
#11
oh wait - I see - I used the same function name in both classes in menu_module.test. I'm too clever for myself...
#12
Ok, a minor problem with the tests for the expected text like:
$this->assertWantedRaw('Created new forum container <em>'. $title .'</em>.', t('New forum container has been created'));This will break if localization is turned on. You need to write it with t() mimicking the exact form from forum module like from modules/forum/forum.admin.inc line 90:
drupal_set_message(t('Created new @type %term.', array('%term' => $form_state['values']['name'], '@type' => $type)));So your code needs to be like:
$type = t('forum container');$this->assertWantedRaw(t('Created new @type %term.', array('%term' => $title '@type' => $type)), t('New forum container has been created'));
There are several spots with this problem.
#13
Alright, thanks :)
Here's a corrected version
#14
And a new copy to correct a small capitalization error pwolanin found.
#15
Looking better - all tests pass.
#16
some code style issues still - for example an extra space in array declarations like:
array ()which should bearray()as before (http://drupal.org/node/201134#comment-669753) try running coder on this.
also, some code could be made more compact, like this:
<?php
// Loop through to find the newly inserted container
$result = db_query(db_rewrite_sql('SELECT t.tid, t.vid, t.name, t.description, t.weight FROM {term_data} t WHERE t.vid = %d', 't', 'tid'), variable_get('forum_nav_vocabulary', ''));
$term = array();
while ($cur_term = db_fetch_array($result)) {
if ($cur_term['name'] == $title && $cur_term['description'] == $description) {
$term = $cur_term;
break;
}
}
?>
could be rewritten as something much simpler like:
<?php$term = db_fetch_array(db_query("SELECT * FROM {term_data} t WHERE t.vid = %d' AND t.name = '%s' AND t.description = '%s'", variable_get('forum_nav_vocabulary', ''), $title, $description);
?>
There's probably no reason to use db_rewrite_sql for simpletests.
There are a couple cases of this.
Also - you generally want to write all query strings with double quotes ( " not ' ) since you need to wrap single qoues around %s placeholders.
#17
The coder module seems to not like me, it still says the code looks perfectly fine. :/
I've corrected the parenthesis spacing and made those queries more compact.
#18
looks better- though a couple code comments like:
// Loop through to find the newly edited vocabularythat should be changed since there is no looping. Maybe just:
// Find the newly edited vocabulary#19
Whoops, I KNEW there was something I forgot to change :P
I changed it for one of the loops and then didn't for the others. New version, coming right up:
#20
ok - looks very close. A suggestion after reviewing the code again.
This part:
// Restore the original settings, and use it as an opportunity to test the form again
$edit = array(
'name' => $original_settings->name,
'description' => $original_settings->description,
'help' => $original_settings->help,
'weight' => $original_settings->weight
);
// Double check that the page says it has edited the vocabulary
$this->drupalPostRequest('admin/content/taxonomy/edit/vocabulary/'. $vid, $edit, 'Save');
$this->assertWantedRaw(t('Updated vocabulary %name.', array('%name' => $original_settings->name)), t('Vocabulary has been edited'));
could probably be simplified to a single API call like:
$original_settings = (array) $original_settings;taxonomy_save_vocabulary($original_settings);
rather than re-testing the thing we tested before. It doesn't matter much, but as it is now will give a redundant failure message.
#21
Fair enough :)
New version with the change
#22
I don't see any other problems - good job.
#23
Thanks for you great work Smartys and pwolanin. Committed.
#24
Automatically closed -- issue fixed for two weeks with no activity.