TestingParty08: Modules without help

The only modules in core that do not implement hook_help are all in the tests folder of simpletest. If one visits /admin/help/simpletest/tests/hook_menu for instance, the SimpleTest help is displayed. So I wonder how to test with the assertion that a module does not implement hook_help. Any ideas?

Mock modules to the rescue. I used one of the existing mock modules that does not implement hook_help. I tried to test visiting the url directly, but that does not work as expected - i posted a patch suggestion in #340883: No message when no help available

The patch should be in unified diff format, additionally hook_menu.module is now menu_test.module

New patch uses menu_test.module and is in the right format.

Couple more things, sorry for the piecemeal review.

+    //use one of the mock modules that do not implement hook_help. 
+    //note that this test will fail if hook_help is implemented in menu_test.module

These should be capitalised at the beginning, with a space after the //.

Is there an existing core module which doesn't implement hook help we could use instead of menu_test?

No problem. Comments should be nice too. Made a new patch.

There is no core module with no hook_help. And that is good I think :) Do you think I should use one of the other modules that has more code? Here are the ones that does not implement hook_help:

find . -name *.module -print | xargs grep -L 'help()'
./modules/simpletest/tests/database_test.module
./modules/simpletest/tests/file_test.module
./modules/simpletest/tests/menu_access.module
./modules/simpletest/tests/menu_test.module
./modules/simpletest/tests/session_test.module
./modules/simpletest/tests/system_test.module
./modules/simpletest/tests/taxonomy_test.module
./modules/simpletest/tests/xmlrpc_test.module

Oh sorry, I forgot to mention that #340883: No message when no help available describe why it is not possible to test:

If the url is visited directly, the

t("No help is available for module %module.",
array('%module' => $module['name']))

message appears.

If the patch I posted in that issue indeed is desired behaviour, then I'll write some more test for this issue.

Thanks for taking care of this naxoc! Some comments after visually inspecting the patch (haven't applied it yet):

1. +  /**
   +    * Make sure that modules not implementing help does not put a link in the admin/help page
   +    */
   

   minor: proper formatting for Doxygen comments dictates that the '*' would line up underneath the first one.

2. If a mock module is used, I'm wondering if we should not create a mock module specifically for this test (or for the help module). Just thinking that if for some reason someone adds a hook_help implementation to menu_test, this unrelated test will suddenly fail. Don't know what the 'accepted' practice is in this case though...
3. As per Test guidelines, the setUp-method should not be preceded by Doxygen comments since it's inherited.

4. +    return array(
   +      'name' => t('Help functionality for modules with no help'),
   +      'description' => t('Verify behavior when modules do not provide help'),
   +      'group' => t('Help'),
   +      );
   

   According to the same guidelines, description should end with a period. Also, the closing parenthesis is misaligned (see Coding standards). I also think 'name' is too verbose, would be great if we can trim it down a bit more.

5. +    * Make sure that modules not implementing help does not put a link in the admin/help page
   

   +    * Enable a mock module that does not implement hook_help
   

   Comments should end with a period. I also think the first sentence needs to be reworded a bit, I don't think it's grammatically correct.

6. The current behavior when visiting a URL like admin/help/module-that-does-not-implement-help is to display the help overview page. IMO, until #340883: No message when no help available lands, this test should assume that this by design (which it probably is), and test accordingly. When/if the patch mentioned above lands, that patch should change this test.

I seem to have read somewhere that not having tests is a bug, not a task, so I have changed the category accordingly.

Thank you for the review mr.baileys!

I cleaned up the code, but I still did not do anything about the mock module that might implement hook_help one day.
I am not sure what to do...
Maybe a comment in the mock module with a warning? Or should I create a mock module that would not really do much apart from not implementing help? What do you think?

+      'name' => t('Tests modules with no help'),

Shorter: "No help".

+      'description' => t('Verify behavior when modules do not provide help.'),

"Verify no help is displayed for modules not providing any help."

+    // Use one of the mock modules that do not implement hook_help. 
+    // Note that this test will fail if hook_help is implemented in menu_test.module.

Replace "mock modules" with "test modules". Remove the second comment line.

+  /**
+   * Test that links to modules with no implementation of hook_help does not 
+   * appear in the admin/help page.
+   */

PHPDoc summaries should go on 1 line, _not_ wrapped by 80 chars. If additional description is required, add a blank line, followed by the PHPDoc description, wrapped at 80 chars.

In this case though, the summary can be shortened:
"Ensure modules not implementing help do not appear on admin/help."

Corrected comments.

getInfo() now needs to be public static function getInfo()

Reroll

Great.

Setting to previous status - testbot was broken (failed to install).

Now, t() is removed in getinfo() : #500866: [META] remove t() from assert message

per #500866: [META] remove t() from assert message are the t()'s for assertion description being removed too, or is that waiting until later?

http://drupal.org/node/500866#comment-1807364 is commited.

t()'s for assertion description being removed later.

then patch looks good.

Committed to HEAD. Thanks!