GHOP #112: Write Simpletest tests for Actions for content

pwolanin - December 15, 2007 - 02:55
Project:SimpleTest
Version:6.x-1.x-dev
Component:Code
Category:task
Priority:normal
Assigned:cwgordon7
Status:closed
Issue tags:GHOP
Description

The main work for this task is to write a small suite of tests to check the basic functionality of Actions associated with changes to site content. This suite should be written as a single .test file.

Suggested tests:

1. admin/build/actions/assign/node Assign an action. Trigger the action and make sure it fires.
2. admin/build/actions/assign/node Attempt to assign the same action twice. Confirm error.
3. admin/build/actions/assign/node Remove an assigned action. Make sure it no longer fires.

#1

pwolanin - December 15, 2007 - 03:02
Title:GHOP : Write Simpletest tests for Actions for content» GHOP #112: Write Simpletest tests for Actions for content

link to task: http://code.google.com/p/google-highly-open-participation-drupal/issues/...

#2

cwgordon7 - December 25, 2007 - 04:50
Assigned to:Anonymous» cwgordon7
Status:active» needs review

File attached as a .txt. If anything fails it's because it's a critical Drupal core issue, not because anything's wrong with this file.

AttachmentSize
content_actions.txt 2.83 KB

#3

snufkin - December 25, 2007 - 06:50

using chx's patch from http://drupal.org/node/203509 and enabling trigger the test runs, without fail. Coding style looks good, strings in t().

Testing an error situation with reverting the patch mentioned above gives me a nice error, so does running it without trigger enabled, so it indeed functions well.

#4

cwgordon7 - December 25, 2007 - 19:31

Thanks for the review, pwolanin. Test rerolled.

AttachmentSize
content_actions.txt 6.03 KB

#5

pwolanin - December 25, 2007 - 19:34

note - in IRC I suggested writing the aid as md5(callback) for readability, testing all 6 actions, and comparing $node->status, etc to an int rather than a string.

#6

pwolanin - December 25, 2007 - 19:36
Status:needs review» needs work

This code comment needs fixing:

// Set action id to "publish post".

Also, there is a random $i in various calls like:

$node->body      = $this->randomName(32 + $i)

#7

cwgordon7 - December 25, 2007 - 19:43
Status:needs work» needs review

Thanks, test file rerolled.

AttachmentSize
content_actions.txt 6.01 KB

#8

pwolanin - December 25, 2007 - 19:49
Status:needs review» needs work

why $action rather than $this->actionName($action)? you shouldn't concat like that in t()
t('Check to make sure the "'. $action .'" action is fired.'));
look at your code below: t('Action %action has been unassigned.', array('%action' => $this->actionName($action)))

that's a better model

#9

cwgordon7 - December 25, 2007 - 20:00

Ok, rerolled. Except with @action.

AttachmentSize
content_actions.txt 6.08 KB

#10

pwolanin - December 25, 2007 - 20:04

      // Action should NOT work.
      $this->assertTrue(!$this->actionWorked($action, $node2)

why not:

     $this->assertFalse($this->actionWorked($action, $node2)

#11

cwgordon7 - December 25, 2007 - 20:09
Status:needs work» needs review

Ok, rerolled.

AttachmentSize
content_actions.txt 6.09 KB

#12

cwgordon7 - December 25, 2007 - 22:31

Ok, rerolled (again).

AttachmentSize
content_actions.txt 6.16 KB

#13

cwgordon7 - December 26, 2007 - 02:02

Ok, following our discussion on IRC, this patch is rerolled & shortened significantly.

AttachmentSize
content_actions.txt 3.57 KB

#14

pwolanin - December 26, 2007 - 02:07

There is something weird going on with trigger/actions when trying to sequentially run several sequentially. Also, we found this bug - need to patch to make this work on PHP4.4: http://drupal.org/node/203846

So - this issue demonstrates the value of this exercise - we turned up 2 core bugs! Thanks cwgordon7

#15

cwgordon7 - December 26, 2007 - 02:41

Rerolled

AttachmentSize
content_actions.txt 3.47 KB

#16

cwgordon7 - December 26, 2007 - 02:59

Whoops. Rerolled.

AttachmentSize
content_actions.txt 3.52 KB

#17

kyl191 - December 26, 2007 - 07:40

Hi,
took a look at the test, and everything seems fine - the test came back with 6 failures, but the failures don't look related to the test.
(Disclaimer: Not only am I a student who is taking part in GHOP, but I am also not an expert in simpletest. But the test still looks ok to me.)

The output of the test is attached if anyone's interested in it.

AttachmentSize
ghop 112.txt 3.09 KB

#18

cwgordon7 - December 26, 2007 - 07:54

Adding on to what kyl191 said: The tests kyl191 performed were without chx's patch, so failures were to be expected.

#19

pwolanin - December 26, 2007 - 16:28

some trivial patches that break the tests

AttachmentSize
break_node.patch 689 bytes
break_reassign.patch 731 bytes
break_assign.patch 648 bytes

#20

pwolanin - December 26, 2007 - 16:38
Status:needs review» reviewed & tested by the community

I did a quick fix on a couple of the error messages, and re-rolled this in patch form (diffed from the simpletest module directory). Looks good to me.

AttachmentSize
content_actions-201139-20.patch 3.95 KB

#21

pwolanin - December 26, 2007 - 16:41

@cwgordon7: please post the final version to the GHOP queue

#22

Rok Žlender - December 27, 2007 - 07:38
Status:reviewed & tested by the community» fixed

Committed thanks.

#23

Anonymous - January 10, 2008 - 07:42
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.