New task

CommentFileSizeAuthor
#6 node_revisions.txt4.45 KBcwgordon7
#5 node_revisions.zip1.47 KBcwgordon7
#2 node_revisions.zip1.47 KBcwgordon7

Comments

pwolanin’s picture

pwolanin commented
Title: GHOP :Write Simpletest tests to check the node revision system » GHOP #111: Write Simpletest tests to check the node revision system

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

The main work for this task is to write a small suite of tests to check the basic functionality of node revisions.

Suggested tests:

1. node/#/edit: Create a new revision. Enter a log message and ensure that
it shows up at node/#/revisions
2. node/#/revisions: Revert to a previous revision. Confirm that the
reversion occurs.
3. node/#/revisions: Delete a revision. Confirm that it is gone and that
node/#/revisions/#/view is not found

cwgordon7’s picture

cwgordon7 commented
Assigned: Unassigned » cwgordon7
Status: Active » Needs review
StatusFileSize
new node_revisions.zip1.47 KB

.test file here, needs review and/or suggestions. I discovered http://drupal.org/node/202895 when creating this file, so make sure to use the HEAD version drupal 6. It's zipped because drupal won't let me upload it otherwise.

-cwgordon7

pwolanin’s picture

pwolanin commented

I won't have a chance to actually test the code until Monday

Also - it would be preferable to post the test as a patch to simpletest - see "Adding/Deleting files" at http://drupal.org/patch/create

Rok Žlender’s picture

Rok Žlender commented
  • missing a global user in setup function produces a bunch of exceptions
  • it would be better if you use $this->drupalGet function instead of $this->_browser->get. Also simpletest already has a function that checks if certain text is displayed on loaded page $this->assertText. So proper flow would be use drupalGet to load page you want to test and then do a $this->assertText to check for content on that page.
  • you also have some permission problems or Drupal has them but anyway for deleting revision your new test user needs 'delete any page content' and for reverting revisions your user needs 'edit any page content'

These are my comments on your test. If you make the changes test will finish without fails also after the changes I will consider this task completed as you made all tests pwolanin asked for.

cwgordon7’s picture

cwgordon7 commented
StatusFileSize
new node_revisions.zip1.47 KB

Thank you for the review, the revised .test file is attached. I have tested this with my drupal 6 HEAD install, everything checks out fine. If you want to see some fails, try it on one of the Drupal six betas (one probably being optimal). Again, it's zipped, because I'm having trouble trying to add new files through a patch. Sorry for any inconvenience this causes.

-cwgordon7

cwgordon7’s picture

cwgordon7 commented
StatusFileSize
new node_revisions.txt4.45 KB

Attached also as a .txt file for easier review.

dmitrig01’s picture

dmitrig01 commented
Status: Needs review » Reviewed & tested by the community

Reviewed code and functionality. Both look fine

aclight’s picture

aclight commented

Since Rok and dmitrig01 both signed off on this, I'm going to give cwgordon07 credit for this. If pwolian wishes for more work to be done on this, cwgordon07 assures me that he'll finish anything necessary.

Rok Žlender’s picture

Rok Žlender commented
Status: Reviewed & tested by the community » Fixed

committed to HEAD thanks cwgordon7

pwolanin’s picture

pwolanin commented

If Rok is satisfied, I'm not going to hold it back.

Anonymous’s picture

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

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