Need to test menu rebuilding through the variable 'menu_rebuild_needed'.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nielsvm’s picture

Status: Active » Needs review
FileSize
2.05 KB

Harold (MadHarold) and I wrote a simple test for this which should do the job. The test tests if the rebuild is fired by manually deleting a record from menu_router and checking if it returns after a page request.

See patch attached.

mikey_p’s picture

All tests pass.

mikey_p’s picture

FileSize
2.4 KB

Change description of final assertion, change delete query to use DBTNG.

benshell’s picture

Status: Needs review » Reviewed & tested by the community

It works and the code makes sense.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Ok, since this is a TestingParty patch, that means a merciless review since that way you learn best practices. Also, it's fun. ;)

+class MenuRebuildCase extends DrupalWebTestCase {

a) That should be MenuRebuildTestCase.
b) Please include a one-liner of PHPDoc to explain what this test case is for.

+  /**
+   * Test if the 'menu_rebuild_needed' variable triggers
+   * an menu_rebuild() call.
+   */

a) PHPDoc above functions should always attempt to fit in one line at < 80 chars. If more explanation than that is needed, make it a second sentence below.
b) Should be a menu_rebuild() not an menu_rebuild().

+    // Check if 'admin' exists

All comments should end in a period.

+    $adminexists = db_result(db_query("SELECT path from {menu_router} WHERE path='admin'"));

a) We don't smoosh together words, so this should be $admin_exists. Needs changing throughout.

+    $this->drupalGet('<front>');// menu_execute_active_handler() should trigger the rebuild.

Please move this to up above this line instead. Inline inline comments are ok for rare circumstances but are generally discouraged, especially when they're long like this.

Darn! That's all I could find. Great job! :)

mikey_p’s picture

Status: Needs work » Needs review
FileSize
2.26 KB

Fun huh? I can't imagine what you qualify as 'unfun,' but anyway....

Fixed all your points.

nielsvm’s picture

FileSize
1.96 KB

Updated patch to remove the always surprising trailing whitespace.

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

Status: Needs work » Needs review
catch’s picture

Status: Needs review » Needs work

We no longer have phpdoc for setUp(), getInfo() or tearDown()
http://drupal.org/node/325974

mikey_p’s picture

Status: Needs work » Needs review
FileSize
2.65 KB

Something to be thankful for, at a thankful time of year ;)

Cleans up all of modules/simpletest/tests/menu.test.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Patch looks good, all tests pass. RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD with some minor whitespace clean-up. Thanks!! :)

Status: Fixed » Closed (fixed)

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