This information is from the code coverage report (see http://coverage.cwgordon.com/coverage).

We need to test:

1) Attempting to have no category name for a site-wide contact form category.
2) Attempting to have no recipients for a site-wide contact form category.
3) Updating a site-wide contact form category.

CommentFileSizeAuthor
#11 contact_test.patch2.91 KBandreiashu
#9 contact_test.patch2.44 KBandreiashu
#4 contacttest.patch2.49 KBcatch

Comments

harrisben’s picture

harrisben commented

1) Navigating to /contact or via the contact menu item (after enabling it) without setting up a category presents an error message "The contact form has not been configured. Add one or more categories to the form", which correctly provides a link directly to the settings page to add a contact form category.

2) When adding a contact form category the form correctly requires a valid email-formatted recipient.

Not sure if it's intended, but the contact form settings page doesn't have the same drag-and-drop prioritising that modules such as blocks and taxonomy have. It would be good to have it for consistency.

cwgordon7’s picture

cwgordon7 commented

I mean automated tests. Not manual ones.

harrisben’s picture

harrisben commented

oh, then please disregard my other posts...

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new contacttest.patch2.49 KB

#1 and #2 are dead code - both those fields are '#required' => TRUE which means they never get to validation, so that can never be run.

So this patch adds a test to ensure the requirement is set (so covers the behaviour for #1 and #2), and removes the redundant validation code for empty fields. That'll mean it gets caught if someone tries to removed the '#required' => TRUE later on. Doesn't touch #3 at all.

contact.test has four known failures #253506 which are unaffected by this patch.

Nice side effect of the code coverage reports :)

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Needs work 
+    $this->addCategory($category = '', '', '', TRUE);

I'm not so sure this is gonna work

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

dmitrig01, it's not supposed to. Or did you mean something else?

dries’s picture

dries commented
Status: Needs review » Fixed

I think this patch looks good. Committed to CVS HEAD.

catch’s picture

catch
he/him
Primary language English
commented
Title: Tests needed: contact.admin.inc » Tests needed: contact - updating site wide category
Status: Fixed » Active

Thanks.

We're still missing #3, so markin back to active for that one.

andreiashu’s picture

andreiashu commented
Category: bug » task
Priority: Critical » Normal
Status: Active » Needs work
StatusFileSize
new contact_test.patch2.44 KB

This is my first patch.
Is it enough to test for the text "Category %category has been added." or should i take every filed and test if it has been updated ?

cwgordon7’s picture

cwgordon7 commented

This needs to comply to coding standards, first of all. Please see http://drupal.org/coding-standards.

I'd also add a check for the category updates in the database.

andreiashu’s picture

andreiashu commented
StatusFileSize
new contact_test.patch2.91 KB

cwgordon7, thanks for suggestion. I think now the coding standards should be fine. I also added db category updates check.
Have a look.

cwgordon7’s picture

cwgordon7 commented
Status: Needs work » Reviewed & tested by the community

Looks great. Tests all pass. RTBC.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Back from vacation -- see http://buytaert.net/houffalize. Committed to CVS HEAD. Will be working the patch queue the next couple of days.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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